You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by kjduling <gi...@git.apache.org> on 2016/05/26 16:48:21 UTC

[GitHub] incubator-geode pull request: GEODE-1331: gfsh.bat on Windows is i...

GitHub user kjduling opened a pull request:

    https://github.com/apache/incubator-geode/pull/149

    GEODE-1331: gfsh.bat on Windows is incorrect

    * Renamed internal variable from CLASSPATH to DEPENDENCIES.
    * Verified @setlocal was not altering the System environment variables for the shell.
    * Launch with -classpath param like the bash script does.
    * Ensured command-line arguments match bash script's order of arguments.

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

    $ git pull https://github.com/kjduling/incubator-geode feature/GEODE-1331

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

    https://github.com/apache/incubator-geode/pull/149.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 #149
    
----
commit 79842cbea7fd2c6c9e4e67d966b11ed379159375
Author: Kevin J. Duling <kd...@pivotal.io>
Date:   2016-05-26T16:46:03Z

    GEODE-1331: gfsh.bat on Windows is incorrect
    
    * Renamed internal variable from CLASSPATH to DEPENDENCIES.
    * Verified @setlocal was not altering the System environment variables for the shell.
    * Launch with -classpath param like the bash script does.
    * Ensured command-line arguments match bash script's order of arguments.

----


---
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.
---

[GitHub] incubator-geode pull request #149: GEODE-1331: gfsh.bat on Windows is incorr...

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

    https://github.com/apache/incubator-geode/pull/149


---
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.
---

[GitHub] incubator-geode pull request #149: GEODE-1331: gfsh.bat on Windows is incorr...

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

    https://github.com/apache/incubator-geode/pull/149#discussion_r65780743
  
    --- Diff: geode-assembly/src/main/dist/bin/gfsh.bat ---
    @@ -74,14 +70,14 @@ REM )
     REM  Consider java is from JDK
     @set TOOLS_JAR=%JAVA_HOME%\lib\tools.jar
     @IF EXIST "%TOOLS_JAR%" (
    -    @set CLASSPATH=%CLASSPATH%;%TOOLS_JAR%
    +    @set DEPENDENCIES=%DEPENDENCIES%;%TOOLS_JAR%
     ) ELSE (
         set TOOLS_JAR=
     )
     
     @set LAUNCHER=com.gemstone.gemfire.management.internal.cli.Launcher
     @if defined JAVA_ARGS (
    -@set JAVA_ARGS="%JAVA_ARGS%"
    +    @set JAVA_ARGS="%JAVA_ARGS%"
     )
    -@"%GF_JAVA%" -Dgfsh=true -Dlog4j.configurationFile=classpath:log4j2-cli.xml %JAVA_ARGS% %LAUNCHER% %*
    --- End diff --
    
    I am not very good at scripting language at all, but could the problem be fixed by just adding 
     -classpath %CLASSPATH% in the final command line? 
    
    It seems that we don't need to introduce DEPENDENCIES variable.


---
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.
---

[GitHub] incubator-geode pull request #149: GEODE-1331: gfsh.bat on Windows is incorr...

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

    https://github.com/apache/incubator-geode/pull/149#discussion_r65782155
  
    --- Diff: geode-assembly/src/main/dist/bin/gfsh.bat ---
    @@ -74,14 +70,14 @@ REM )
     REM  Consider java is from JDK
     @set TOOLS_JAR=%JAVA_HOME%\lib\tools.jar
     @IF EXIST "%TOOLS_JAR%" (
    -    @set CLASSPATH=%CLASSPATH%;%TOOLS_JAR%
    +    @set DEPENDENCIES=%DEPENDENCIES%;%TOOLS_JAR%
     ) ELSE (
         set TOOLS_JAR=
     )
     
     @set LAUNCHER=com.gemstone.gemfire.management.internal.cli.Launcher
     @if defined JAVA_ARGS (
    -@set JAVA_ARGS="%JAVA_ARGS%"
    +    @set JAVA_ARGS="%JAVA_ARGS%"
     )
    -@"%GF_JAVA%" -Dgfsh=true -Dlog4j.configurationFile=classpath:log4j2-cli.xml %JAVA_ARGS% %LAUNCHER% %*
    --- End diff --
    
    It certainly could be done that way.  I think I defined a new variable to be clear that we prepend to an existing CLASSPATH if there is one.  If there isn't, then we only use what we define in the script.


---
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.
---