Repository navigation
🐛 fix(batch): remember an undefined PATH so deactivate can restore it - #3493
Merged
Merged
Conversation
When PATH is undefined at activation, for example in a build script that runs `set PATH=` first, activate.bat has nothing to save. `set "_OLD_VIRTUAL_PATH=%PATH%"` expands to an empty value, which cmd treats as deleting the variable, and deactivate.bat only restores PATH when _OLD_VIRTUAL_PATH is defined, so it left the environment's Scripts folder on PATH. The re-activation guard from pypa#3245 checks the same variable, so a second activate.bat did not deactivate first. It saved the active PATH as the one to restore and added Scripts a second time, and PKG_CONFIG_PATH got a second entry the same way. activate.bat now sets _OLD_VIRTUAL_PATH_UNSET when PATH is undefined, and the guard deactivates when that marker is set too. deactivate.bat undefines PATH when it finds the marker and then clears it.
darrenhuai
marked this pull request as ready for review
October 30, 2026 01:47
Taking PATH from PATH itself or from an undefined variable leaves it as is or undefines it, so the expected values read os.environ without a branch.
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.
If
PATHis undefined when you activate,deactivate.batleaves the environment'sScriptsfolder onPATH, and a secondactivate.batadds it twice. A new cmd window always hasPATHdefined, so you only hit this after something runsset PATH=, such as a build script that starts from a clean environment and builds its ownPATH. With main (861cff7):cmd has no empty variables, so when
PATHis undefined,@set "_OLD_VIRTUAL_PATH=%PATH%"inactivate.batdeletes_OLD_VIRTUAL_PATHinstead of saving an empty value.deactivate.batonly touchesPATHwhen_OLD_VIRTUAL_PATHis defined, so it skips the restore. The re-activation guard from #3245 checks the same variable, so the secondactivate.batdoesn't deactivate first: it saves the activePATHas the one to restore and putsScriptsin front again.PKG_CONFIG_PATH, which that guard also protects, gets a secondlib\pkgconfigentry the same way and is still set afterdeactivate. This is the batch half of the emptyPATHcase that #3259 fixed for bash and fish.activate.batnow sets_OLD_VIRTUAL_PATH_UNSET=1whenPATHis undefined, in the same goto branch that saves_OLD_VIRTUAL_PATH, and the guard at the top also runsdeactivate.batwhen that marker is defined.deactivate.batundefinesPATHwhen it finds the marker, then clears it. Each new command is one line with no parenthesized block, so parentheses inPATHare still safe. Nothing changes whenPATHis defined. With this branch the script above prints[C:\work\venv\Scripts;],[C:\work\venv\Scripts;]and[].test_batch_path_round_tripnow also starts from an undefinedPATH, in normal and delayed expansion, for each of the existing environment names and prompt cases. It expects<Scripts>;after both activations and noPATHafterdeactivate. The script then setsPATH=%SystemRoot%and activates once more, to check thatdeactivateleft nothing behind; a stale marker would make that activation drop the caller'sPATH. The newpath-unsetcases fail with main's scripts.Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
ran the linter to address style issues (
tox -e fix). I ran the same hooks throughprekon the changed files; the semgrep hook only looks atactivate.sh.wrote descriptive pull request text
ensured there are test(s) validating the fix
added a
docs/changelog/+short-description.<type>.rstfragment for each user-facing change, or appliedskip newsupdated/extended the documentation (the docs already say
deactivaterestores the originalPATH)for changes to activation scripts,
pyvenv.cfg, wheel downloads, app-data or release workflows: the outside input is the caller'sPATH. The new lines only test whetherPATHand the marker are defined, set the marker to the literal1and clearPATH. They never expandPATH, so its contents can't run as code, and the existing quotedset "_OLD_VIRTUAL_PATH=%PATH%"save is unchanged.ran tests on Python 3.9 and 3.15 (
tox run -e 3.9,3.15). I ran 3.14 locally and left 3.9 and 3.15 to CI.linked upstream issues beside any temporary interpreter overrides (none)
kept the PR in draft until CI passes