fix(pkg): stop the Windows build overwriting the venv's pip - #10396
Conversation
CREATE_VIRTUAL_ENV creates a virtualenv and then copies the whole of the
system Python's Lib over it, so that the relocated interpreter has a
standard library to use. That copy includes Lib\site-packages, and XCOPY
/Y overwrites the files that exist on both sides without removing the
ones that only exist in the venv, so the venv's freshly seeded pip ends
up spliced together with whatever pip C:\Python313 happens to have.
That was harmless while the two layouts agreed. pip 26.2 turned
pip/_internal/build_env from a module into a package, so on a builder
whose system pip predates 26.2 the new build_env/ directory survives the
copy whilst utils/misc.py is overwritten by the older one, and every
subsequent pip invocation dies before it does anything:
File "...\pip\_internal\build_env\installer.py", line 22, in <module>
from pip._internal.utils.misc import get_runnable_pip
ImportError: cannot import name 'get_runnable_pip' from
'pip._internal.utils.misc'
It cannot recover on its own either, since the next line of the script
is the pip invocation that upgrades pip.
Copy the standard library with ROBOCOPY and exclude site-packages, which
the venv is entitled to keep to itself. ROBOCOPY rather than XCOPY
because /XD takes a directory to skip and the existing
:CHECK_ROBOCOPY_ERROR helper already handles its exit codes; /R:3 /W:5
so a locked file fails the build rather than retrying for a million
attempts.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
WalkthroughWindows environment setup now propagates ChangesEnvironment setup
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to Windows virtual-environment setup now preserves its seeded packages while copying the standard library, avoiding pip package conflicts. No concrete merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
One consequence worth a reviewer's eye before this is merged. The venv is seeded with pip alone — the build log shows What the build actually needs is unaffected, as far as I can tell from the script:
There is a second effect, which I think is an improvement but is a behaviour change either way. Under the old behaviour that meant whatever the builder happened to have pip-installed globally shipped inside pgAdmin too — The risk is the mirror image of that: if some import is currently satisfied only by a globally installed package that never made it into |
Follow-up review fixes to the site-packages exclusion. Exclude site-packages by name rather than by full path. The full path is built from PGADMIN_PYTHON_DIR, so if that is ever set with a trailing backslash the source and the /XD argument both gain a doubled separator; should the match then fail, site-packages is copied over the venv again and the build fails with the same ImportError this was meant to remove. No directory inside the standard library is legitimately called site-packages, so matching by name is both simpler and unconditional. Drop "> nul" from both ROBOCOPY calls in favour of /NFL /NDL /NP. ROBOCOPY writes its errors to stdout, so with the redirect in place a copy that returns 8 or more aborted the build with nothing in the log to say which file or share was at fault; the switches suppress the per-file noise whilst keeping the header, the summary and any errors. Guard the copy of the web directory with "|| EXIT /B 1" as well, since EXIT /B inside :CHECK_ROBOCOPY_ERROR returns from the label rather than from the caller and a failed copy was therefore ignored, leaving the build to package an incomplete tree. Guard virtualenv.exe likewise: with the standard library now copied by ROBOCOPY, which creates its destination, a failure there would otherwise surface later at the activate step instead of where it happened.
|
Reviewed, and the diagnosis holds up against the Jenkins log: the traceback is I have pushed four follow-up fixes to the branch rather than leaving them as comments, since they are all one-liners:
On your question about the shipped Neither your change nor mine has been run on Windows, so the buildfarm remains the only test either of us can give it; nothing under There is also a pre-existing typo at |
2 participants
The Windows x64 build has been failing since the buildfarm's virtualenv started seeding pip 26.2.x:
CREATE_VIRTUAL_ENVcreates a virtualenv and then copies the whole of the system Python'sLibover it, so the relocated interpreter has a standard library. That copy includesLib\site-packages, andXCOPY /Yoverwrites the files present on both sides without removing the ones only the venv has, so the venv's freshly seeded pip ends up spliced together with whatever pipC:\Python313happens to have.That was harmless while the two layouts agreed. pip 26.2 turned
pip/_internal/build_envfrom a module into a package, so on a builder whose system pip predates 26.2 the newbuild_env/directory survives the copy whilstutils/misc.pyis overwritten by the older one — and the resulting install is internally inconsistent. I checked the pristine 26.2.1 wheel:build_env/is a package andutils/misc.pydoes defineget_runnable_pip, so the released artifact is fine; only the spliced copy is broken.It cannot recover on its own, because the next line of the script is the pip invocation that upgrades pip.
Copy the standard library with
ROBOCOPYand excludesite-packages, which the venv is entitled to keep to itself.ROBOCOPYrather thanXCOPY /EXCLUDEbecause/XDtakes a directory to skip, whereas/EXCLUDEtakes a file of path substrings and breaks on quoted paths containing spaces./R:3 /W:5so a locked file fails the build instead of retrying a million times.Note that
CALL :CHECK_ROBOCOPY_ERRORis followed by|| EXIT /B 1here:EXIT /Binside the label returns from the label rather than from the caller, so without it a failed copy would be ignored.Upgrading the system pip on the builder (
C:\Python313\python.exe -m pip install --upgrade pip) also unblocks it immediately, but the splice will bite again the next time the two pip layouts diverge.Not verified on a Windows builder — I have no Windows host to run
Make.baton. A buildfarm run is the real test.Summary by CodeRabbit