feat(nodeenv): write the posix activate on Windows for git-bash - #409
Merged
Merged
Conversation
On Windows only activate.bat, deactivate.bat and Activate.ps1 were written, so git-bash and the other posix shells there had nothing to source. Python's venv treats its bash activate as "common" and installs it on every platform; do the same. The script itself needs two Windows adjustments: node.exe keeps the global modules next to itself, in Scripts/node_modules rather than lib/node_modules, and it cannot read the /c/... paths a Windows shell hands out, so NODE_PATH and NPM_CONFIG_PREFIX are converted back with cygpath while $PATH stays posix. A new CI job runs the activation under `shell: bash` on a windows runner, which is git-bash. #226
The first CI run of the new job failed with a bare exit 1 and no diagnostic: the probe ran through `bash -c` with the script and the environment path as arguments, so the quoting rules of two command line parsers sat between the test and what the shell ran, and stderr was not captured at all. It now writes the probe to a file, traces it with `set -x` and puts both streams into the assertion message. The direct call guard matches on $0, which a shell reports in the form it was given; the test was passing the native `C:\...` path, which no posix shell would produce.
`bash` on PATH is C:\Windows\System32\bash.exe, the WSL launcher: on a runner without a distribution it answers "Windows Subsystem for Linux has no installed distributions" in UTF-16 and exits 1, which is what the job was reporting. Look git-bash up next to git instead.
Looking git-bash up next to git.exe missed: on PATH inside git-bash git is Git/mingw64/bin/git.exe, and there is no bash.exe beside it. The job already runs in the right shell, so it now passes $BASH to the test, which keeps a PATH lookup that skips the System32 launcher for runs outside CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #226.
On Windows nodeenv wrote only
activate.bat,deactivate.batandActivate.ps1, so git-bash and the other posix shells available there hadnothing to source. Python's
venvkeeps its bashactivateinscripts/common/and installs it on every platform; this does the same.The script needed two Windows adjustments
node.exe, inScripts/node_modules,not in
lib/node_modules, and its prefix is the directory holdingnode.exe- soNODE_PATHandNPM_CONFIG_PREFIXpoint there.node.exeis a native binary and cannot read the/c/...paths a Windowsshell hands out, so both variables are converted back with
cygpath -wwhile
$PATHstays posix, since that one is read by the shell itself.venvsolves the same problem the same way.The "do not call this file directly" guard now matches
*/Scripts/activatetoo, and the
$PATHline builds the.bindirectory from__MOD_NAME__instead of a hardcoded
lib/node_modules- on posix both render exactly asbefore.
Tests
tests/test_install_activate.pygets 8 tests that fakeis_WIN, so theScripts/layout is checked from any host: the set of files written, the$PATHline, the native paths,sh -nover the generated script, the directcall guard, and
-pappending to theScripts/activatethat venv alreadywrote there instead of replacing it.
Behaviour on a real Windows host is covered by
test_smoke_git_bash, run bya new
git-bashCI job:shell: bashon a windows runner is git-bash. Itbuilds an environment, sources
Scripts/activateand asserts thatprocess.execPathandnpm root -gboth point inside it - the second onefails if the
cygpathconversion did not happen.Not in scope
activate.fishis still posix-only, there is no native fish on Windows.BASH_SOURCEfallback inACTIVATE_SHstill embeds the nativeC:\...path, which would break$PATHif a non-bash shell sourced it onWindows. git-bash ships bash as
sh, so the case from the issue is covered.set_predeactivate_hook()writes.bat/.ps1hooks only on Windows; aposix
predeactivatefor git-bash is a separate question.