HADOOP-6482. GenericOptionsParser constructor that takes Options and String[] ignores options. Contributed by Eli Collins.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@986971 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -202,6 +202,9 @@ Trunk (unreleased changes)
     HADOOP-6885. Fix java doc warnings in Groups and RefreshUserMappingsProtocol.
     (Eli Collins via jghoman) 
 
+    HADOOP-6482. GenericOptionsParser constructor that takes Options and 
+    String[] ignores options. (Eli Collins via jghoman)
+
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES

src/java/org/apache/hadoop/util/GenericOptionsParser.java

@@ -86,8 +86,8 @@ import org.apache.hadoop.fs.Path;
  * $ bin/hadoop dfs -D fs.default.name=darwin:8020 -ls /data
  * list /data directory in dfs with namenode darwin:8020
  *     
- * $ bin/hadoop dfs -conf hadoop-site.xml -ls /data
+ * $ bin/hadoop dfs -conf core-site.xml -conf hdfs-site.xml -ls /data
- * list /data directory in dfs with conf specified in hadoop-site.xml
+ * list /data directory in dfs with multiple conf files specified.
  *     
  * $ bin/hadoop job -D mapred.job.tracker=darwin:50020 -submit job.xml
  * submit a job to job tracker darwin:50020
@@ -122,7 +122,7 @@ public class GenericOptionsParser {
    */
   public GenericOptionsParser(Options opts, String[] args) 
       throws IOException {
-    this(new Configuration(), new Options(), args);
+    this(new Configuration(), opts, args);
   }
 
   /**
@@ -400,25 +400,23 @@ public class GenericOptionsParser {
   /**
    * Parse the user-specified options, get the generic options, and modify
    * configuration accordingly
+   * @param opts Options to use for parsing args.
    * @param conf Configuration to be modified
    * @param args User-specified arguments
-   * @return Command-specific arguments
    */
-  private String[] parseGeneralOptions(Options opts, Configuration conf, 
+  private void parseGeneralOptions(Options opts, Configuration conf, 
       String[] args) throws IOException {
     opts = buildGeneralOptions(opts);
     CommandLineParser parser = new GnuParser();
     try {
       commandLine = parser.parse(opts, args, true);
       processGeneralOptions(conf, commandLine);
-      return commandLine.getArgs();
     } catch(ParseException e) {
       LOG.warn("options parsing failed: "+e.getMessage());
 
       HelpFormatter formatter = new HelpFormatter();
       formatter.printHelp("general options are: ", opts);
     }
-    return args;
   }
 
   /**

src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java

@@ -27,6 +27,9 @@ import junit.framework.TestCase;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.OptionBuilder;
+import org.apache.commons.cli.Options;
 
 public class TestGenericOptionsParser extends TestCase {
   File testDir;
@@ -77,6 +80,42 @@ public class TestGenericOptionsParser extends TestCase {
     assertNull("files is not null", files);
   }
 
+  /**
+   * Test that options passed to the constructor are used.
+   */
+  @SuppressWarnings("static-access")
+  public void testCreateWithOptions() throws Exception {
+    // Create new option newOpt
+    Option opt = OptionBuilder.withArgName("int")
+    .hasArg()
+    .withDescription("A new option")
+    .create("newOpt");
+    Options opts = new Options();
+    opts.addOption(opt);
+
+    // Check newOpt is actually used to parse the args
+    String[] args = new String[2];
+    args[0] = "--newOpt";
+    args[1] = "7";
+    GenericOptionsParser g = new GenericOptionsParser(opts, args);
+    assertEquals("New option was ignored",
+      "7", g.getCommandLine().getOptionValues("newOpt")[0]);
+  }
+
+  /**
+   * Test that multiple conf arguments can be used.
+   */
+  public void testConfWithMultipleOpts() throws Exception {
+    String[] args = new String[2];
+    args[0] = "--conf=foo";
+    args[1] = "--conf=bar";
+    GenericOptionsParser g = new GenericOptionsParser(args);
+    assertEquals("1st conf param is incorrect",
+      "foo", g.getCommandLine().getOptionValues("conf")[0]);
+    assertEquals("2st conf param is incorrect",
+      "bar", g.getCommandLine().getOptionValues("conf")[1]);
+  }
+  
   @Override
   protected void setUp() throws Exception {
     super.setUp();