[SPARK-6435] spark-shell --jars option does not add all jars to classpath

Modified to accept double-quotated args properly in spark-shell.cmd.

Author: Masayoshi TSUZUKI <tsudukim@oss.nttdata.co.jp>

Closes #5227 from tsudukim/feature/SPARK-6435-2 and squashes the following commits:

ac55787 [Masayoshi TSUZUKI] removed unnecessary argument.
60789a7 [Masayoshi TSUZUKI] Merge branch 'master' of https://github.com/apache/spark into feature/SPARK-6435-2
1fee420 [Masayoshi TSUZUKI] fixed test code for escaping '='.
0d4dc41 [Masayoshi TSUZUKI] - escaped comman and semicolon in CommandBuilderUtils.java - added random string to the temporary filename - double-quotation followed by `cmd /c` did not worked properly - no need to escape `=` by `^` - if double-quoted string ended with `\` like classpath, the last `\` is parsed as the escape charactor and the closing `"` didn't work properly
2a332e5 [Masayoshi TSUZUKI] Merge branch 'master' into feature/SPARK-6435-2
04f4291 [Masayoshi TSUZUKI] [SPARK-6435] spark-shell --jars option does not add all jars to classpath

3 files changed, 9 insertions, 11 deletions

diff --git a/launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java b/launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java
index 8028e42ffb..261402856a 100644
--- a/launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java
+++ b/launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java
@@ -244,7 +244,7 @@ class CommandBuilderUtils {
     boolean needsQuotes = false;
     for (int i = 0; i < arg.length(); i++) {
       int c = arg.codePointAt(i);
-      if (Character.isWhitespace(c) || c == '"' || c == '=') {
+      if (Character.isWhitespace(c) || c == '"' || c == '=' || c == ',' || c == ';') {
         needsQuotes = true;
         break;
       }
@@ -261,15 +261,14 @@ class CommandBuilderUtils {
         quoted.append('"');
         break;
 
-      case '=':
-        quoted.append('^');
-        break;
-
       default:
         break;
       }
       quoted.appendCodePoint(cp);
     }
+    if (arg.codePointAt(arg.length() - 1) == '\\') {
+      quoted.append("\\");
+    }
     quoted.append("\"");
     return quoted.toString();
   }
diff --git a/launcher/src/main/java/org/apache/spark/launcher/Main.java b/launcher/src/main/java/org/apache/spark/launcher/Main.java
index 206acfb514..929b29a49e 100644
--- a/launcher/src/main/java/org/apache/spark/launcher/Main.java
+++ b/launcher/src/main/java/org/apache/spark/launcher/Main.java
@@ -101,12 +101,9 @@ class Main {
    * The method quotes all arguments so that spaces are handled as expected. Quotes within arguments
    * are "double quoted" (which is batch for escaping a quote). This page has more details about
    * quoting and other batch script fun stuff: http://ss64.com/nt/syntax-esc.html
-   *
-   * The command is executed using "cmd /c" and formatted in single line, since that's the
-   * easiest way to consume this from a batch script (see spark-class2.cmd).
    */
   private static String prepareWindowsCommand(List<String> cmd, Map<String, String> childEnv) {
-    StringBuilder cmdline = new StringBuilder("cmd /c \"");
+    StringBuilder cmdline = new StringBuilder();
     for (Map.Entry<String, String> e : childEnv.entrySet()) {
       cmdline.append(String.format("set %s=%s", e.getKey(), e.getValue()));
       cmdline.append(" && ");
@@ -115,7 +112,6 @@ class Main {
       cmdline.append(quoteForBatchScript(arg));
       cmdline.append(" ");
     }
-    cmdline.append("\"");
     return cmdline.toString();
   }
 
diff --git a/launcher/src/test/java/org/apache/spark/launcher/CommandBuilderUtilsSuite.java b/launcher/src/test/java/org/apache/spark/launcher/CommandBuilderUtilsSuite.java
index 1ae42eed8a..bc513ec9b3 100644
--- a/launcher/src/test/java/org/apache/spark/launcher/CommandBuilderUtilsSuite.java
+++ b/launcher/src/test/java/org/apache/spark/launcher/CommandBuilderUtilsSuite.java
@@ -74,7 +74,10 @@ public class CommandBuilderUtilsSuite {
     assertEquals("\"a b c\"", quoteForBatchScript("a b c"));
     assertEquals("\"a \"\"b\"\" c\"", quoteForBatchScript("a \"b\" c"));
     assertEquals("\"a\"\"b\"\"c\"", quoteForBatchScript("a\"b\"c"));
-    assertEquals("\"ab^=\"\"cd\"\"\"", quoteForBatchScript("ab=\"cd\""));
+    assertEquals("\"ab=\"\"cd\"\"\"", quoteForBatchScript("ab=\"cd\""));
+    assertEquals("\"a,b,c\"", quoteForBatchScript("a,b,c"));
+    assertEquals("\"a;b;c\"", quoteForBatchScript("a;b;c"));
+    assertEquals("\"a,b,c\\\\\"", quoteForBatchScript("a,b,c\\"));
   }
 
   @Test