diff --git a/CHANGES.txt b/CHANGES.txt index 637a308577..29261198d9 100644 --- a/CHANGES.txt +++ b/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 diff --git a/src/java/org/apache/hadoop/util/GenericOptionsParser.java b/src/java/org/apache/hadoop/util/GenericOptionsParser.java index c22400704c..1efc767600 100644 --- a/src/java/org/apache/hadoop/util/GenericOptionsParser.java +++ b/src/java/org/apache/hadoop/util/GenericOptionsParser.java @@ -86,8 +86,8 @@ * $ 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 - * list /data directory in dfs with conf specified in hadoop-site.xml + * $ bin/hadoop dfs -conf core-site.xml -conf hdfs-site.xml -ls /data + * 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 @@ private String validateFiles(String files, Configuration conf) /** * 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; } /** diff --git a/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java b/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java index bcabcfa64d..920d9a2c67 100644 --- a/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java +++ b/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java @@ -27,6 +27,9 @@ 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; @@ -76,6 +79,42 @@ public void testFilesOption() throws Exception { files = conf2.get("tmpfiles"); 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 {