diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java index a0cb6cccc1..6048f8e818 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java @@ -25,6 +25,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.apache.commons.cli.CommandLine; @@ -413,7 +414,50 @@ private String validateFiles(String files, Configuration conf) } return StringUtils.arrayToString(finalArr); } - + + /** + * Windows powershell and cmd can parse key=value themselves, because + * /pkey=value is same as /pkey value under windows. However this is not + * compatible with how we get arbitrary key values in -Dkey=value format. + * Under windows -D key=value or -Dkey=value might be passed as + * [-Dkey, value] or [-D key, value]. This method does undo these and + * return a modified args list by manually changing [-D, key, value] + * into [-D, key=value] + * + * @param args command line arguments + * @return fixed command line arguments that GnuParser can parse + */ + private String[] preProcessForWindows(String[] args) { + if (!Shell.WINDOWS) { + return args; + } + List newArgs = new ArrayList(args.length); + for (int i=0; i < args.length; i++) { + String prop = null; + if (args[i].equals("-D")) { + newArgs.add(args[i]); + if (i < args.length - 1) { + prop = args[++i]; + } + } else if (args[i].startsWith("-D")) { + prop = args[i]; + } else { + newArgs.add(args[i]); + } + if (prop != null) { + if (prop.contains("=")) { + // everything good + } else { + if (i < args.length - 1) { + prop += "=" + args[++i]; + } + } + newArgs.add(prop); + } + } + + return newArgs.toArray(new String[newArgs.size()]); + } /** * Parse the user-specified options, get the generic options, and modify @@ -427,7 +471,7 @@ private void parseGeneralOptions(Options opts, Configuration conf, opts = buildGeneralOptions(opts); CommandLineParser parser = new GnuParser(); try { - commandLine = parser.parse(opts, args, true); + commandLine = parser.parse(opts, preProcessForWindows(args), true); processGeneralOptions(conf, commandLine); } catch(ParseException e) { LOG.warn("options parsing failed: "+e.getMessage()); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java index b58bb190b4..87bc83b419 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java @@ -21,6 +21,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.net.URI; +import java.util.Arrays; +import java.util.Map; import junit.framework.TestCase; @@ -35,6 +37,9 @@ import org.apache.commons.cli.Option; import org.apache.commons.cli.OptionBuilder; import org.apache.commons.cli.Options; +import org.junit.Assert; + +import com.google.common.collect.Maps; public class TestGenericOptionsParser extends TestCase { File testDir; @@ -191,4 +196,90 @@ public void testTokenCacheOption() throws IOException { localFs.delete(new Path(testDir.getAbsolutePath()), true); } + + /** Test -D parsing */ + public void testDOptionParsing() throws Exception { + String[] args; + Map expectedMap; + String[] expectedRemainingArgs; + + args = new String[]{}; + expectedRemainingArgs = new String[]{}; + expectedMap = Maps.newHashMap(); + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-Dkey1=value1"}; + expectedRemainingArgs = new String[]{}; + expectedMap = Maps.newHashMap(); + expectedMap.put("key1", "value1"); + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-Dkey1=value1", "arg1"}; + expectedRemainingArgs = new String[]{"arg1"}; + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1=value1", "arg1"}; + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + if (Shell.WINDOWS) { + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1", + "value1", "arg1"}; + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-Dkey1", "value1", "arg1"}; + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1", "value1", + "-fs", "someother", "-D", "key2", "value2", "arg1", "arg2"}; + expectedRemainingArgs = new String[]{"arg1", "arg2"}; + expectedMap = Maps.newHashMap(); + expectedMap.put("key1", "value1"); + expectedMap.put("key2", "value2"); + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1", "value1", + "-fs", "someother", "-D", "key2", "value2"}; + expectedRemainingArgs = new String[]{}; + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1", "value1", + "-fs", "someother", "-D", "key2"}; + expectedMap = Maps.newHashMap(); + expectedMap.put("key1", "value1"); + expectedMap.put("key2", null); // we expect key2 not set + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + } + + args = new String[]{"-fs", "hdfs://somefs/", "-D", "key1=value1", + "-fs", "someother", "-Dkey2"}; + expectedRemainingArgs = new String[]{}; + expectedMap = Maps.newHashMap(); + expectedMap.put("key1", "value1"); + expectedMap.put("key2", null); // we expect key2 not set + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + + args = new String[]{"-fs", "hdfs://somefs/", "-D"}; + expectedMap = Maps.newHashMap(); + assertDOptionParsing(args, expectedMap, expectedRemainingArgs); + } + + private void assertDOptionParsing(String[] args, + Map expectedMap, String[] expectedRemainingArgs) + throws Exception { + for (Map.Entry entry : expectedMap.entrySet()) { + assertNull(conf.get(entry.getKey())); + } + + Configuration conf = new Configuration(); + GenericOptionsParser parser = new GenericOptionsParser(conf, args); + String[] remainingArgs = parser.getRemainingArgs(); + + for (Map.Entry entry : expectedMap.entrySet()) { + assertEquals(entry.getValue(), conf.get(entry.getKey())); + } + + Assert.assertArrayEquals( + Arrays.toString(remainingArgs) + Arrays.toString(expectedRemainingArgs), + expectedRemainingArgs, remainingArgs); + } }