Summary
Sourcing bin/activate.fish destroys a NODE_PATH the user already had
set. deactivate_node then has nothing to restore, so the variable is gone
for good. NPM_CONFIG_PREFIX and npm_config_prefix have the same defect.
Reproduction
$ nodeenv --prebuilt /tmp/ne
$ fish -c 'set -gx NODE_PATH /my/node_path
source /tmp/ne/bin/activate.fish
deactivate_node
echo "NODE_PATH=[$NODE_PATH]"'
NODE_PATH=[]
Expected NODE_PATH=[/my/node_path], which is what the POSIX bin/activate
does under sh, dash and bash.
Cause
activate.fish calls deactivate_node nondestructive near the top, before
the activation code has saved anything:
# unset irrelevant variables
deactivate_node nondestructive
On that pass _OLD_NODE_PATH is empty, so deactivate_node takes its else
branch and erases the variable:
if test -n "$_OLD_NODE_PATH"
set -gx NODE_PATH $_OLD_NODE_PATH
set -e _OLD_NODE_PATH
else
set -e NODE_PATH
end
The activation code that runs afterwards therefore sees set -q NODE_PATH
as false, never records _OLD_NODE_PATH, and the user's value is lost:
if set -q NODE_PATH
set -gx _OLD_NODE_PATH $NODE_PATH
set -gx NODE_PATH "$NODE_VIRTUAL_ENV/lib/node_modules" $NODE_PATH
else
set -gx NODE_PATH "$NODE_VIRTUAL_ENV/lib/node_modules"
end
b042056 ("fix(nodeenv): guard fish npm config restore") already fixed
exactly this disease, but only for the --isolate-npm variables, by
wrapping the restore in a guard:
# Skip the "deactivate_node nondestructive" pass at the top of
# activate.fish: the variables are only saved after it has run
if set -q NODE_VIRTUAL_ENV
NODE_PATH and the NPM_CONFIG_PREFIX / npm_config_prefix pair never got
that guard.
Scope of the fix
All three variable groups, not just NODE_PATH. The prefix pair is affected
identically; it only looks fine today because the test asserts NODE_PATH
first and stops there.
Test coverage
tests/test_activate_shells.py (added in #397) marks this
xfail(strict=True) as FISH_NODE_PATH_CLOBBER on
test_deactivate_restores_env, via the restoring_shell decorator. When the
fix lands, drop the constant and the decorator and put the test back on
@activating_shell.
Summary
Sourcing
bin/activate.fishdestroys aNODE_PATHthe user already hadset.
deactivate_nodethen has nothing to restore, so the variable is gonefor good.
NPM_CONFIG_PREFIXandnpm_config_prefixhave the same defect.Reproduction
Expected
NODE_PATH=[/my/node_path], which is what the POSIXbin/activatedoes under sh, dash and bash.
Cause
activate.fishcallsdeactivate_node nondestructivenear the top, beforethe activation code has saved anything:
# unset irrelevant variables deactivate_node nondestructiveOn that pass
_OLD_NODE_PATHis empty, sodeactivate_nodetakes its elsebranch and erases the variable:
The activation code that runs afterwards therefore sees
set -q NODE_PATHas false, never records
_OLD_NODE_PATH, and the user's value is lost:b042056("fix(nodeenv): guard fish npm config restore") already fixedexactly this disease, but only for the
--isolate-npmvariables, bywrapping the restore in a guard:
NODE_PATHand theNPM_CONFIG_PREFIX/npm_config_prefixpair never gotthat guard.
Scope of the fix
All three variable groups, not just
NODE_PATH. The prefix pair is affectedidentically; it only looks fine today because the test asserts
NODE_PATHfirst and stops there.
Test coverage
tests/test_activate_shells.py(added in #397) marks thisxfail(strict=True)asFISH_NODE_PATH_CLOBBERontest_deactivate_restores_env, via therestoring_shelldecorator. When thefix lands, drop the constant and the decorator and put the test back on
@activating_shell.