yarnSetup path issue on git bash

[kpage]

Sample project demonstrating the issue with steps to reproduce: https://github.com/kpage/gradle-node-plugin-git-bash-issue

The 'yarnSetup' task fails on Windows 10 when run in git bash due to node not being present on the PATH. Running yarnSetup in cmd.exe works fine. npmSetup and npmInstall both work on either git bash or cmd.exe.

Error is:

:yarnSetup
C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn\yarnpkg -> C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn\node_modules\yarn\bin\yarn.js
                                                                                                                       C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn\yarn -> C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn\node_modules\yarn\bin\yarn.js

> spawn-sync@1.0.15 postinstall C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn\node_modules\yarn\node_modules\spawn-sync
> node postinstall

'node' is not recognized as an internal or external command,
operable program or batch file.
C:\Users\user\repo\gradle-node-plugin-git-bash-issue\.gradle\yarn
`-- (empty)

                                                                                  npm ERR! Windows_NT 10.0.14393
npm ERR! argv "C:\\Users\\user\\repo\\gradle-node-plugin-git-bash-issue\\.gradle\\nodejs\\node-v6.9.1-win-x64\\node.exe" "C:\\Users\\user\\repo\\gradle-node-plugin-git-bash-issue\\.gradle\\nodejs\\node-v6.9.1-win-x64\\node_modules\\npm\\bin\\npm-cli.js" "install" "--prefix" "C:\\Users\\user\\repo\\gradle-node-plugin-git-bash-issue\\.gradle\\yarn" "yarn@0.17.10"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code ELIFECYCLE

npm ERR! spawn-sync@1.0.15 postinstall: `node postinstall`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the spawn-sync@1.0.15 postinstall script 'node postinstall'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the spawn-sync package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node postinstall
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs spawn-sync
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls spawn-sync
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     C:\Users\user\repo\gradle-node-plugin-git-bash-issue\npm-debug.log
npm ERR! code 1
:yarnSetup FAILED

This command should use the node that is auto-downloaded by gradle-node-plugin. But the path is not correctly set in git bash. If you don't have node globally installed, you will see the error.

If you do have node globally installed, it will use the wrong version (not the version required by gradle-node-plugin). This can lead to errors as the wrong version of node can be used to create node_modules, and then the user will get errors when running commands that require node-specific versions of packages.

[kpage]

Hi @kaitoy , thanks for your addition of yarn support to gradle-node-plugin. It seems that npmSetup correctly sets the path for node on git bash, do you think there is some strategy we can take from that to have the path also work correctly for yarnSetup?

Or perhaps it's just a coincidence that npmSetup works because it does not shell out to node? I have also included an example where npm has to install node-sass, which shells out to a node postinstall script during npmInstall. Yarn install will fail on this task.

[kpage]

After further investigation, it looks like this issue is due to a case sensitivity issue of the path environment variable ("Path" vs "PATH"). Probably the reason that cmd.exe has no issue is that environment is not case-sensitive, while git bash is more case-sensitive for some operations.

I don't think this issue is due to the setup of yarn vs npm in gradle-node-plugin at all. It seems actually that gradle-node-plugin is not always setting the path correctly on windows in git bash in general, but that the npm tool run in git bash is less sensitive than yarn when it comes to case sensitivity of the path env variable.

On my Windows system, the path env variable was "Path". gradle-node-plugin has this logic in NodeExecRunner to set the path:

            // Take care of Windows environments that may contain "Path" OR "PATH" - both existing
            // possibly (but not in parallel as of now)
            if ( System.getenv( 'Path' ) != null )
            {
                nodeEnvironment['Path'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'Path' )
            }
            else
            {
                nodeEnvironment['PATH'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'PATH' )
            }

On my system, the first branch run. This means that nodeEnvironment['Path'] was set, but nodeEnvironment['PATH'] was not. Newer versions of npm run in git bash seem fine with this, but yarn does not like it. Perhaps instead we could set both cases like this? This seems to fix the issue for me:

            // Take care of Windows environments that may contain "Path" OR "PATH" - both existing
            // possibly (but not in parallel as of now)
            if ( System.getenv( 'Path' ) != null )
            {
                nodeEnvironment['Path'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'Path' )
            }
            if ( System.getenv( 'PATH' ) != null )
            {
                nodeEnvironment['PATH'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'PATH' )
            }

[kaitoy]

According to the Java API doc, System.getenv( 'Path' ) returns the same value as System.getenv( 'PATH' ).

For example, the expression System.getenv("FOO").equals(System.getenv("foo")) is likely to be true on Microsoft Windows.

So, I think just removing the branch for Path solve the issue:

if ( System.getenv( 'PATH' ) != null )
{
    nodeEnvironment['PATH'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'PATH' )
}

Or more simply, remove if:

nodeEnvironment['PATH'] = nodeBinDirPath + File.pathSeparator + System.getenv( 'PATH' )

[kpage]

Hi @kaitoy , thanks for taking a look. Yes, I agree with you that the System.getenv('Path') and System.getenv('PATH') would probably return the same value on Windows (but not on Linux).

But I think removing the 'Path' version may cause a regression fixed by: #44

I will test some more.

[kpage]

I tested without setting nodeEnvironment['Path'] and had no issues on either git bash or cmd.exe. It may be that this was a problem for older Windows/java/node/npm, but it no longer seems to be an issue. However, we may want to set both nodeEnvironment['Path'] and nodeEnvironment['PATH'] for better backwards compatibility, if it causes no new issue.

My test environment:

• Windows 10
• Git bash and cmd.exe
• java jdk 1.8.0_111 64-bit
• node 6.9.1
• npm 3.10.9
• yarn 0.17.10

[tspaeth]

@kpage as you pinged me directly: I'm currently too busy to follow immediately, but if it is not solved next week, I'll have a look if possible. Thanks.

[kpage]

I did some more testing today, it doesn't look like this is going to be easy to solve. I ran the gradle-node-plugin tests (both master and with the fix that @kaitoy mentioned in #173 (comment)) in both cmd.exe and git bash. I found that the case of the path variable on Windows ("Path" vs "PATH") affects whether the builds work or fail.

• "Path" with fix on cmd.exe: FAILS at NpmRule_integTest "succeeds to run npm module using npm_run_ when shrinkwrap contains local npm". Error is with running npm script: 'npm-run-all' is not recognized...
• "Path" with fix on git bash: WORKS
• "Path" without fix on cmd.exe: WORKS
• "Path" without fix on git bash: yarnSetup FAILS
• "PATH" with fix on cmd.exe: WORKS
• "PATH" with fix on git bash: WORKS
• "PATH" without fix on cmd.exe: yarnSetup FAILS
• "PATH" without fix on git bash: yarnSetup FAILS

The only thing that works on both cmd.exe and git bash is if the path is cased as "PATH" and the fix is made. But this would be a regression for users who already have working builds with "Path" on cmd.exe. "Path" is the default casing of this variable on Windows 10, so this would probably cause issues for most users.

This seems it would be difficult to fix in the general case, as @kaitoy pointed out Java has no information on the case of the Windows path variable, as calls to System.getenv( 'Path' ) and System.getenv( 'PATH' ) would both return the same results. So the gradle build cannot have special logic for one case or the other.

Another option is to set both environment('Path') and environment('PATH') as I was suggesting above. But that also seems to have problems. Here are the results for that test:

• "Path" on git bash: FAILS at YarnRule_integTest “can execute an yarn module using yarn_run_”. Error is with running npm script yarn_run_parent: ‘concurrently’ is not recognized...
• "Path" on cmd.exe: FAILS at NpmRule_integTest "succeeds to run npm module using npm_run_ when shrinkwrap contains local npm". Error is with running npm script: 'npm-run-all' is not recognized...
• "PATH" on git bash: FAILS at YarnRule_integTest “can execute an yarn module using yarn_run_”. Error is with running npm script yarn_run_parent: ‘concurrently’ is not recognized...
• "PATH" on cmd.exe: FAILS at YarnRule_integTest “can execute an yarn module using yarn_run_”. Error is with running npm script yarn_run_parent: ‘concurrently’ is not recognized…

So it looks like setting environment('Path') and environment('PATH') is not a good option.

At this point, there is no clear way forward. Perhaps we could remove the logic to set ‘Path’ instead of ‘PATH’ and document that all Windows users must define their path variable in all uppercase, but this is a breaking change. Maybe it could be done for the next major release?

[s0x]

Please check if this could be resolved by applying MR #179

[ChristianRamseier]

Thanks, the pull request resolves the issue for me.

[srs]

Great. This is fixed in 1.1.+.