You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jarrettmeyer <gi...@git.apache.org> on 2017/05/04 18:53:58 UTC

[GitHub] spark pull request #17861: Remove excess quotes in Windows executable

GitHub user jarrettmeyer opened a pull request:

    https://github.com/apache/spark/pull/17861

    Remove excess quotes in Windows executable

    ## What changes were proposed in this pull request?
    
    Quotes are already added to the RUNNER variable on line 54. There is no need to put quotes on line 67. If you do, you will get an error when launching Spark.
    
    '""C:\Program' is not recognized as an internal or external command, operable program or batch file.
    
    ## How was this patch tested?
    
    Tested manually on Windows 10.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jarrettmeyer/spark fix-windows-cmd

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/17861.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #17861
    
----
commit d784a3101338634d022b32dcd950f78351b69d29
Author: Jarrett Meyer <ja...@gmail.com>
Date:   2017-05-04T18:52:09Z

    Remove excess quotes in Windows executable
    
    Quotes are already added to the RUNNER variable on line 54. There is no need to put quotes on line 67.
    
    If you have 2 sets of quotes, you receive the following error:
    
    '""C:\Program' is not recognized as an internal or external command, operable program or batch file.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17861: [SPARK-20613] Remove excess quotes in Windows exe...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17861#discussion_r115001274
  
    --- Diff: bin/spark-class2.cmd ---
    @@ -64,7 +64,7 @@ if not "x%JAVA_HOME%"=="x" (
     rem The launcher library prints the command to be executed in a single line suitable for being
     rem executed by the batch interpreter. So read all the output of the launcher into a variable.
     set LAUNCHER_OUTPUT=%temp%\spark-class-launcher-output-%RANDOM%.txt
    -"%RUNNER%" -Xmx128m -cp "%LAUNCH_CLASSPATH%" org.apache.spark.launcher.Main %* > %LAUNCHER_OUTPUT%
    --- End diff --
    
    FWIW, it looks has been fine in AppVeyor as below:
    
    ```cmd
    echo %JAVA_HOME%
    C:\Progra~1\Java\jdk1.8.0
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: Remove excess quotes in Windows executable

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    **[Test build #3690 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3690/testReport)** for PR 17861 at commit [`d784a31`](https://github.com/apache/spark/commit/d784a3101338634d022b32dcd950f78351b69d29).
     * This patch **fails from timeout after a configured wait of \`250m\`**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: Remove excess quotes in Windows executable

Posted by jarrettmeyer <gi...@git.apache.org>.
Github user jarrettmeyer commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    I have created a Jira issue [SPARK-20613](https://issues.apache.org/jira/browse/SPARK-20613) and updated the title.
    
    Yes, fixing in either place will work. You need quotes on the line where the `RUNNER` is invoked, or you need quotes on the line where the `RUNNER` is defined. You can't have both. That is invalid syntax.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: [SPARK-20613] Remove excess quotes in Windows executable

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    If either place works identical, let's go back to the original one rather then trying new one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17861: [SPARK-20613] Remove excess quotes in Windows exe...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/17861


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: [SPARK-20613] Remove excess quotes in Windows executable

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    ouch - my bad.
    thanks @jarrettmeyer for the fix and @HyukjinKwon for review and testing
    
    merged to master/2.2


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: Remove excess quotes in Windows executable

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: Remove excess quotes in Windows executable

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    **[Test build #3690 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3690/testReport)** for PR 17861 at commit [`d784a31`](https://github.com/apache/spark/commit/d784a3101338634d022b32dcd950f78351b69d29).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17861: Remove excess quotes in Windows executable

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17861#discussion_r114934079
  
    --- Diff: bin/spark-class2.cmd ---
    @@ -64,7 +64,7 @@ if not "x%JAVA_HOME%"=="x" (
     rem The launcher library prints the command to be executed in a single line suitable for being
     rem executed by the batch interpreter. So read all the output of the launcher into a variable.
     set LAUNCHER_OUTPUT=%temp%\spark-class-launcher-output-%RANDOM%.txt
    -"%RUNNER%" -Xmx128m -cp "%LAUNCH_CLASSPATH%" org.apache.spark.launcher.Main %* > %LAUNCHER_OUTPUT%
    --- End diff --
    
    I found this problem when I tested some cases on Windows before but I just thought it was my wrong environmental setup.
    
    I think we should change `set RUNNER="%JAVA_HOME%\bin\java"` to `set RUNNER=%JAVA_HOME%\bin\java`. cc @felixcheung and @shivaram, I just realised I was cc'ed in the PR (https://github.com/apache/spark/pull/16596) but it looks I missed this as well ...
    
    I can reproduce this problem as below:
    
    ```cmd
    C:\...\spark>set JAVA_HOME
    JAVA_HOME=C:\Program Files\Java\jdk1.8.0_121
    
    C:\...\spark>.\bin\spark-shell
    '""C:\Program' is not recognized as an internal or external command,
    operable program or batch file.
    ```
    
    ```cmd
    echo "%RUNNER%"
    ```
    
    prints
    
    ```cmd
    ""C:\Program Files\Java\jdk1.8.0_121\bin\java""
    ```
    
    It looks cmd does not recognise the space.
    
    
    To double check, I copied the jdk into `C:\Java` and then ran the commands as below:
    
    ```cmd
    C:\...\spark>set JAVA_HOME=C:\Java\jdk1.8.0_121
    
    C:\...\spark>.\bin\spark-shell
    
    ...
    
    Spark context Web UI available at http://10.0.2.15:4040
    Spark context available as 'sc' (master = local[*], app id = local-1493961061248).
    Spark session available as 'spark'.
    Welcome to
          ____              __
         / __/__  ___ _____/ /__
        _\ \/ _ \/ _ `/ __/  '_/
       /___/ .__/\_,_/_/ /_/\_\   version 2.3.0-SNAPSHOT
          /_/
    
    Using Scala version 2.11.8 (Java HotSpot(TM) Client VM, Java 1.8.0_121)
    Type in expressions to have them evaluated.
    Type :help for more information.
    ...
    ```
    
    
    ```cmd
    echo "%RUNNER%"
    ```
    
    prints 
    
    ```cmd
    ""C:\Java\jdk1.8.0_121\bin\java""
    ```
    
    
    **After fixing the line I suggested**
    
    ```diff
     if not "x%JAVA_HOME%"=="x" (
    -   set RUNNER="%JAVA_HOME%\bin\java"
    +   set RUNNER=%JAVA_HOME%\bin\java
     ) else (
    ```
    
    
    ```cmd
    C:\...\spark>set JAVA_HOME
    JAVA_HOME=C:\Program Files\Java\jdk1.8.0_121
    
    C:\...\spark>.\bin\spark-shell
    ...
    Spark context Web UI available at http://10.0.2.15:4040
    Spark context available as 'sc' (master = local[*], app id = local-1493962115332).
    Spark session available as 'spark'.
    Welcome to
          ____              __
         / __/__  ___ _____/ /__
        _\ \/ _ \/ _ `/ __/  '_/
       /___/ .__/\_,_/_/ /_/\_\   version 2.3.0-SNAPSHOT
          /_/
    
    Using Scala version 2.11.8 (Java HotSpot(TM) Client VM, Java 1.8.0_121)
    Type in expressions to have them evaluated.
    Type :help for more information.
    ...
    ```
    
    
    ```cmd
    echo "%RUNNER%"
    ```
    
    prints 
    
    ```cmd
    "C:\Program Files\Java\jdk1.8.0_121\bin\java"
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17861: Remove excess quotes in Windows executable

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/17861
  
    @jarrettmeyer I think we should create a JIRA for this as it does look non-trivial fix although the line diff is single. Please refer http://spark.apache.org/contributing.html.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org