Repository navigation
🐛 fix(seed): keep partial wheel images out of the cache - #3487
Merged
Merged
Conversation
The app-data seeder skips building a wheel image whenever the image folder exists and is not empty, but the build extracted straight into that folder and left whatever it had written when a step failed. A full disk, an antivirus lock or a killed process could leave a half-built image, and so does Windows without long path support when the app-data path falls in the narrow band where only the long PEP 639 license paths at the end of the pip wheel are over the limit. The first run fails loudly. Every later run with the same app-data then reuses the partial image, prints "added seed packages" and exits 0, and the pip it seeds has no METADATA, RECORD or INSTALLER, so pip list shows "pip None" and pip cannot upgrade or reinstall itself. The image is now built in a staging folder next to the installer folder and moved into place with os.replace once the RECORD fix-up and, for symlink images, the read-only marking are done. A failed build deletes the staging folder through the path the build used, which is the 8.3 short path when Windows had to switch to it, and the next build deletes one that a killed process left behind. The staging path is exactly as long as the image path, so the Windows path length handling makes the same choices as before, and when it switches to the short path the install reads through the shortened parent path.
for more information, see https://pre-commit.ci
The direct build test read the private dist-info cache under a lint suppression; it now installs the built image and checks the pip script. The leftover test takes its path and files from the parameters instead of branching, and the failure parameter is typed without an ellipsis by passing the disk-full error itself where no partial extraction runs.
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.
When extracting pip's wheel into the app-data image stopped part way (a full disk, a killed process, or a path over the Windows length limit), that run failed, but every later run with the same app-data reported success and seeded a pip without
RECORD,INSTALLERor the.virtualenvmarker. 🐛pip listthen showspip None, andpip install --force-reinstall pipfails withuninstall-no-record-file. A cut earlier in the wheel left no dist-info at all and every later run failed withno .dist-info at ..., the symptom behind #1971 and #2326. Either way the cache stayed broken until--reset-app-data. After this change a failed build leaves nothing behind, and the next run builds the image again.The image is reused whenever its folder exists and has anything in it, so it now appears only once it is complete. #3430 gave the wheel download the same treatment: stage, check, then move into place. The build extracts the wheel into a staging folder beside the installer folder, finishes the
RECORD, marker and read-only marking there, and moves the folder into place in one rename. A failure deletes the staging folder, and the next build clears one that a killed process left behind. The staging path is exactly as long as the image path, so the Windows short path handling decides the same way it did before.An image truncated before this change still looks complete, so
--reset-app-dataremains the way out of a cache that is already broken. Building into a folder that already holds an image now fails instead of merging into it; virtualenv itself never calls the build in that case.Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
ran the linter to address style issues (
ruff checkandruff format; the other pre-commit hooks run in CI)wrote descriptive pull request text
ensured there are test(s) validating the fix
added news fragment in
docs/changelogfolderupdated/extended the documentation (
docs/explanation.rstalready says the seeder builds complete install images; thebuild_imagedocstring describes the staging step)for changes to activation scripts,
pyvenv.cfg, wheel downloads, app-data or release workflows: this touches app-data but handles no new outside input. The wheel it extracts is the bundled one or one the download step already verified,_safe_extract_zipstill checks every entry against the target before anything is written, now against the staging folder, and virtualenv creates that folder inside the same per-user cache while holding the lock that already serializes image builds. Nothing in it runs as code until a later run copies or links the finished image, exactly as before. THREAT_MODEL.md's description of app-data (downloaded wheels and extracted wheel images) stays accurate, so it is unchanged.ran tests on Python 3.9 and 3.15 (ran on 3.14 on Windows and 3.12 on Linux directly with pytest; leaving 3.9 and 3.15 to CI)
linked upstream issues beside any temporary interpreter overrides (none)
kept the PR in draft until CI passes