Skip to content

🐛 fix(create): replace read-only interpreter files on Windows - #3492

Merged
gaborbernat merged 4 commits into
pypa:mainfrom
darrenhuai:fix/replace-read-only-generated-files
Oct 30, 2026
Merged

gaborbernat merged 4 commits into
pypa:mainfrom
darrenhuai:fix/replace-read-only-generated-files

Conversation

@darrenhuai

@darrenhuai darrenhuai commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

On Windows, running virtualenv a second time over an existing environment fails if one of its copied interpreter files has the read-only attribute. With main and Python 3.14:

$ python -m virtualenv env --no-seed --activators batch -q
$ python -c "import os, stat; os.chmod(r'env\Scripts\python.exe', stat.S_IREAD)"
$ python -m virtualenv env --no-seed --activators batch -q
PermissionError: [WinError 5] Access is denied: 'C:\\...\\env\\Scripts\\python.exe'

python3 and the pythonw3.exe alias fail the same way, while --clear on the same environment works. The attribute can come from outside, for example a Perforce workspace, a restored backup, attrib +R, or Explorer's read-only option applied to a folder. Those tend to mark every file, and then the default seeded run also stops, at Scripts\pip-3.14.exe.

#3466 taught write_file to clear the attribute and retry, which is why pyvenv.cfg and the activation scripts already get replaced fine. The other files go through three deletes that are still a bare unlink(): ensure_safe_to_do, which copy() uses on an existing destination, the alias loop in ExePathRefToDest.run, and _uninstall_dist in the app-data seeder, which removes the previous pip's RECORD entries such as its console scripts. Windows refuses to delete a read-only file, so all three raise. POSIX deletion depends only on the parent directory, so this is Windows-only. --clear already works because the safe_delete error handler calls clear_readonly.

I moved the retry out of write_file into a remove_file helper, unchanged, and call it from write_file and those three places. In _uninstall_dist the call stays in the branch that has already checked that the entry resolves inside the environment, so the containment check the UncontainedMetadataDelete query looks for still guards it. The query now lists remove_file as a delete next to safe_delete, so a missing check is reported at the caller rather than at the unlink inside the helper.

test_copy_replaces_read_only_file is the copy() twin of the existing test_write_file_replaces_read_only_file. Two Windows-only tests build an environment, make its files read-only and run virtualenv again. test_recreate_environment_over_read_only_executables does that for everything in Scripts with the builtin creator and no seeding, which also reaches the alias loop through pythonw3.exe. test_reseed_over_read_only_environment does it for every file of a seeded environment. All three fail on main.

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 through prek on the changed files; the semgrep hook only looks at activate.sh.

  • wrote descriptive pull request text

  • ensured there are test(s) validating the fix

  • added a docs/changelog/+short-description.<type>.rst fragment for each user-facing change, or applied
    skip news

  • updated/extended the documentation (no docs describe this case)

  • for changes to activation scripts, pyvenv.cfg, wheel downloads, app-data or release workflows: the outside input is the read-only attribute of a file already in the environment, such as pyvenv.cfg, the activation scripts or the seeded pip's files. The change only clears that attribute before deleting the old file, as 🐛 fix(bash): escape the prompt for every PS1 state #3466 already did for write_file, and the containment check in _uninstall_dist runs before it as before. What gets written is unchanged, and nothing from the old file is read or run.

  • ran tests on Python 3.9 and 3.15 (tox run -e 3.9,3.15). I ran the changed and related test files on 3.9 and 3.15.0a7 directly and the full suite on 3.14, and left the full 3.9 and 3.15 runs to CI.

  • linked upstream issues beside any temporary interpreter overrides (none)

  • kept the PR in draft until CI passes

Re-running virtualenv over an existing environment on Windows stopped with PermissionError [WinError 5] when a copied
interpreter file such as Scripts\python.exe, or the pythonw3.exe alias, had the read-only attribute. ensure_safe_to_do
and the alias loop in ExePathRefToDest removed the old file with a plain unlink, and Windows refuses to delete a
read-only file. pypa#3466 made write_file clear the attribute and retry, which covers pyvenv.cfg and the activation
scripts, and --clear goes through safe_delete, which already calls clear_readonly. These two removals were the ones
left. The attribute comes from places like a Perforce workspace, a restored backup, attrib +R, or Explorer's read-only
option applied to a folder.

The retry now lives in remove_file, shared by write_file, ensure_safe_to_do and the alias loop. It leaves hard links
alone: NTFS keeps the attribute on the file rather than on the name, so clearing it through a hard link would make the
file's other names writable too, and those can sit outside the environment.
…heck

With every file in the environment read-only, re-seeding still failed in _uninstall_dist, which removed the old
pip-3.14.exe with a bare unlink. It now goes through remove_file like the creator does.

The hard-link check in remove_file is gone. safe_delete and pypa#3466 clear the attribute through hard links, so the check
only made copy() of a file behave differently from copy() of a folder.
CI runners can make symlinks, and chmod on a link either marked the link itself read-only or followed it to the base
interpreter. Build the environment with --copies, skip links when setting the attribute, and keep coverage's .pth out
of the environment like the other metadata tests do.
@darrenhuai
darrenhuai marked this pull request as ready for review October 30, 2026 02:06
The uninstall loop now deletes RECORD entries through remove_file. The query
still followed it into the helper, so a missing containment check was
reported at the unlink inside _sync.py rather than at the uninstall line.
Naming remove_file as a sink, like safe_delete, keeps the alert at the
caller. The reseed test filters its files in the loop instead of an if.
@gaborbernat
gaborbernat merged commit 58c5c10 into pypa:main Oct 30, 2026
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants