You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by en...@apache.org on 2013/07/03 01:01:39 UTC
svn commit: r1499144 - in /hadoop/common/branches/branch-1-win/src:
core/org/apache/hadoop/util/GenericOptionsParser.java
test/org/apache/hadoop/util/TestGenericOptionsParser.java
Author: enis
Date: Tue Jul 2 23:01:38 2013
New Revision: 1499144
URL: http://svn.apache.org/r1499144
Log:
HADOOP-9660 [WINDOWS] Powershell / cmd parses -Dkey=value from command line as [-Dkey, value] which breaks GenericsOptionParser (enis)
Modified:
hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/GenericOptionsParser.java
hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/util/TestGenericOptionsParser.java
Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/GenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/GenericOptionsParser.java?rev=1499144&r1=1499143&r2=1499144&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/GenericOptionsParser.java (original)
+++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/GenericOptionsParser.java Tue Jul 2 23:01:38 2013
@@ -25,6 +25,7 @@ import java.net.URISyntaxException;
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;
@@ -395,7 +396,50 @@ public class GenericOptionsParser {
}
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<String> newArgs = new ArrayList<String>(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
@@ -409,7 +453,7 @@ public class GenericOptionsParser {
opts = buildGeneralOptions(opts);
CommandLineParser parser = new GnuParser();
try {
- commandLine = parser.parse(opts, args, true);
+ commandLine = parser.parse(opts, preProcessForWindows(args), true);
processGeneralOptions(conf, commandLine);
return commandLine.getArgs();
} catch(ParseException e) {
Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/util/TestGenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/util/TestGenericOptionsParser.java?rev=1499144&r1=1499143&r2=1499144&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/util/TestGenericOptionsParser.java (original)
+++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/util/TestGenericOptionsParser.java Tue Jul 2 23:01:38 2013
@@ -21,12 +21,17 @@ import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
+import java.util.Arrays;
+import java.util.Map;
import junit.framework.TestCase;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.junit.Assert;
+
+import com.google.common.collect.Maps;
public class TestGenericOptionsParser extends TestCase {
File testDir;
@@ -133,4 +138,89 @@ public class TestGenericOptionsParser ex
assertNull("files is not null", files);
}
+ /** Test -D parsing */
+ public void testDOptionParsing() throws Exception {
+ String[] args;
+ Map<String,String> 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<String,String> expectedMap, String[] expectedRemainingArgs)
+ throws Exception {
+ for (Map.Entry<String, String> 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<String, String> entry : expectedMap.entrySet()) {
+ assertEquals(entry.getValue(), conf.get(entry.getKey()));
+ }
+
+ Assert.assertArrayEquals(
+ Arrays.toString(remainingArgs) + Arrays.toString(expectedRemainingArgs),
+ expectedRemainingArgs, remainingArgs);
+ }
}