Build PyTorch wheel via PEP 517 without poisoning the CI image - #21685
Build PyTorch wheel via PEP 517 without poisoning the CI image#21685shoumikhin wants to merge 3 commits into
Conversation
This reverts the install_pytorch.sh change from #21562. It broke test-riscv on main, 17 of 17 jobs, on every commit since it landed. The PEP 517 change itself is correct. The problem is that the PR installs the PEP 517 build requirements permanently into the image conda environment. scikit-build-core registers a setuptools build_ext plugin, so every later "pip install --no-build-isolation" in that image runs its cmake configure instead of the plain setuptools one. That turns on C++20 module dependency scanning, which the image GCC 14.2.0 cannot satisfy, and the tokenizers build fails with "cc1plus: error: to generate dependencies you must specify either -M or -MM". Reverting to unblock main. Re-landing separately with the build requirements confined to a throwaway virtualenv.
…ay venv Re-land of #21562 with the leak fixed. The PEP 517 build requirements now go into a temporary virtualenv instead of the image conda environment, so scikit-build-core stops hijacking every later setuptools build in the image.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21685
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 28 PendingAs of commit be65128 with merge base 48741ac ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
shoumikhin
added
ciflow/trunk
release notes: none
shoumikhin
changed the title
|
Updated this PR to be a complete forward fix for #21562, not only the RISC-V half. #21562 had two separate effects, and both came from installing the PEP 517 build requirements into the image's conda environment:
Moving the build requirements into a throwaway venv fixes both. The image build log shows This replaces #21684, which is a plain revert. Main has been red for about three hours, so a review here would be very welcome. |
|
Verification is complete.
Same number of tests executed and the same number skipped, so nothing was skipped away. The 12 that failed on main now run and pass. That covers both the 10 LAPACK failures in
Full status against the 23 checks that #21562 broke:
This is ready. It needs one approving review and nothing else. |
Forward fix for #21562.
#21562 moved the PyTorch wheel build from
setup.py bdist_wheeltopython -m build --wheel --no-isolation. That direction is correct and is kept here. The problem was that it installed the PEP 517 build requirements permanently into the image's conda environment, and two different things in that install changed how every later build in the image behaves. Both broke main.1. All 17 RISC-V jobs failed
scikit-build-core ships a setuptools plugin that registers itself as the
build_extcommand class. Once it is present in the environment, any laterpip install --no-build-isolationpicks it up. The tokenizers build is exactly that, so its CMake configure started running through scikit-build-core, which turns on C++20 module dependency scanning. The image compiler cannot produce the scan output:2. The wheel was built without LAPACK
The same install also put a pip
cmakewheel into the environment, which lands on PATH ahead of the image's own cmake. The two are the same version but not interchangeable: the pip wheel does not search the conda environment, soFindMKLstopped finding the MKL libraries that are installed there and PyTorch was configured with MKL and LAPACK off. 12 tests inextension/llm/modules/test/test_turboquant_kv_cache.pythen failed with:Fix
Install the build requirements into a throwaway venv instead of into the image, and invoke the build through that venv's interpreter by absolute path. The venv is created with
--system-site-packages, so PyTorch still builds against the image's numpy and toolchain and still reuses sccache. The venv is deleted once the wheel is built, so neither the scikit-build-core plugin nor the pip cmake survives into the image, and the image's own cmake stays in front.Evidence
From the docker image build log, PyTorch's configure summary:
.../site-packages/cmake/data/bin/cmake/opt/conda/envs/py_3.10/bin/cmakeUSE_MKLOFFONUSE_LAPACK01CI on this PR:
test-riscv: 17 of 17 green. On main it is 17 of 17 red.test-lora-multimethod-linux: green. On main it is red.Note on the jobs that did not finish
unittest / linux,unittest-editable / linuxandtest-lora-linuxwere all cancelled at 1 hour 31 minutes, which is the 90 minute job cap. Any PR that touches.ci/docker/forces a full image rebuild that consumes roughly 30 minutes of that budget, so these three cannot complete on a PR of this shape. That is also why the LAPACK problem was not visible on #21562 before it merged. TheUSE_LAPACKline in the image build log above is the direct evidence for that half of the fix, since it is read at configure time rather than at test time.Relationship to the revert
This branch contains two commits, a revert of #21562 followed by the corrected version. The net diff against main is a single file,
.ci/docker/common/install_pytorch.sh, and squash merge collapses them into one commit.This supersedes #21684, which is a straight revert of #21562. If this lands, #21684 should be closed without merging. Do not merge #21684 after this one, since it would restore the previous file wholesale and undo this fix.