🐛 fix(util): replace a stale symlink instead of writing through it - #3229
Open
darrenhuai wants to merge 2 commits into
Open
🐛 fix(util): replace a stale symlink instead of writing through it#3229darrenhuai wants to merge 2 commits into
darrenhuai wants to merge 2 commits into
Conversation
`ensure_safe_to_do` clears whatever sits at the destination before a ref is copied or symlinked into an environment, but it decides there is nothing to clear with `dest.exists()`. That call follows the link, so a symlink whose target is gone reads as an empty slot and the destination is left in place. An environment hits this as soon as its interpreter moves: create it with `--symlinks`, let pyenv or brew or a distro upgrade remove the interpreter that `bin/python` points at, then run virtualenv over the same directory again, which is the supported way to refresh one. In symlink mode the recreate dies with an uncaught `FileExistsError`, because `symlink_to` gets a path that is still occupied. In copy mode it is quieter and worse: `shutil.copy` opens the destination for writing, follows the stale link, and drops the interpreter at whatever the link used to point to - outside the environment. virtualenv reports success, the environment keeps a symlink pointing out of itself instead of the copy `--copies` asked for, and an 8 MB binary is left behind somewhere else on disk. Checking `is_symlink()` as well treats a dangling link as something in the way, so it is unlinked and replaced like any other occupied destination.
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.
Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
tox -e fix)docs/changelogfolderensure_safe_to_doclears whatever sits at the destination before a ref is copied or symlinked into an environment, but it decides there is nothing to clear withdest.exists():That call follows the link, so a symlink whose target is gone reads as an empty slot and the destination is left in place.
An environment hits this as soon as its interpreter moves. Create one with
--symlinks, let pyenv or brew or a distro upgrade remove the interpreter thatbin/pythonpoints at, then run virtualenv over the same directory again to refresh it.In symlink mode the recreate dies:
In copy mode it is quieter and worse.
shutil.copyopens the destination for writing, follows the stale link, and drops the interpreter at whatever the link used to point to — outside the environment:virtualenv reports success, the environment keeps a symlink pointing out of itself instead of the copy
--copiesasked for, and an 8 MB binary is left behind somewhere else on disk.Checking
is_symlink()as well treats a dangling link as something in the way, so it is unlinked and replaced like any other occupied destination. With the fix the same run puts a regular file atenv/bin/pythonand leaves nothing outside.Two tests, one per mode:
symlinkreplaces a dangling destination rather than raising, andcopywrites a real file at the destination rather than through the stale link. Both fail onmain. They need a filesystem that can make symlinks, so they use the samepytest.skipguard already used elsewhere in the file.Verified on Linux (3.12, where the symlink tests actually run) and on Windows (3.14,
tests/unitis 311 passed / 35 skipped, the two new ones among the skips since this machine cannot create symlinks).ruffandty check src/virtualenvpass against both the 3.9 and 3.14 targets.I also tried the same guard on the alias cleanup in
ExePathRefToDest.run, which has the same shape. Reverting it changed no behaviour I could produce — the base link is rewritten before the aliases resolve through it — so I left that line alone rather than add an untested change.