[wheel] fix windows wheel building #58343
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Windows wheel building script to improve its reliability and maintainability. The key changes include removing the intermediate step of installing Ray before building the wheel, pinning build dependencies for reproducible builds, and explicitly using the modern PEP 517 build process. These are excellent improvements. I've added a couple of suggestions to further enhance the script's efficiency and debuggability.
| python -m pip install pip==25.2 | ||
| python -m pip install wheel==0.45.1 delvewheel==1.11.2 setuptools==80.9.0 |
There was a problem hiding this comment.
These two pip install commands can be combined into a single one for better efficiency. This reduces the number of pip invocations and allows pip to perform dependency resolution for all packages at once.
| python -m pip install pip==25.2 | |
| python -m pip install wheel==0.45.1 delvewheel==1.11.2 setuptools==80.9.0 | |
| python -m pip install pip==25.2 wheel==0.45.1 delvewheel==1.11.2 setuptools==80.9.0 |
| RAY_INSTALL_CPP=1 python -m pip wheel -v -w dist . --no-deps | ||
| # No extra dlls are needed, do not call delvewheel | ||
| uninstall_ray | ||
| RAY_INSTALL_CPP=1 python -m pip wheel -v -w dist . --no-deps --use-pep517 |
There was a problem hiding this comment.
For consistency with the ray wheel build command on line 116 and for better debuggability, consider adding the --debug flag to this command as well. It will provide more verbose output, which is helpful for diagnosing CI failures.
| RAY_INSTALL_CPP=1 python -m pip wheel -v -w dist . --no-deps --use-pep517 | |
| RAY_INSTALL_CPP=1 python -m pip wheel -v -w dist . --no-deps --debug --use-pep517 |
do not install ray or its dependencies when building wheel, it is not required Signed-off-by: Lonnie Liu <[email protected]>
aslonnie
force-pushed
the
lonnie-251030-winwhlfix2
branch
from
2d1817b to
216a6b9
Compare
aslonnie
changed the title
|
test build here: https://buildkite.com/ray-project/postmerge/builds/14157 |
add `--use-pep517` flag; otherwise some part of the wheel building logic does not work on windows python 3.12 also removes the unnecessary "uninstall+reinstall" dance. the script only builds the wheel, it does not (and should not) install the wheel. Signed-off-by: Lonnie Liu <[email protected]>
add `--use-pep517` flag; otherwise some part of the wheel building logic does not work on windows python 3.12 also removes the unnecessary "uninstall+reinstall" dance. the script only builds the wheel, it does not (and should not) install the wheel. Signed-off-by: Lonnie Liu <[email protected]>
add `--use-pep517` flag; otherwise some part of the wheel building logic does not work on windows python 3.12 also removes the unnecessary "uninstall+reinstall" dance. the script only builds the wheel, it does not (and should not) install the wheel. Signed-off-by: Lonnie Liu <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
add `--use-pep517` flag; otherwise some part of the wheel building logic does not work on windows python 3.12 also removes the unnecessary "uninstall+reinstall" dance. the script only builds the wheel, it does not (and should not) install the wheel. Signed-off-by: Lonnie Liu <[email protected]>
add
--use-pep517flag; otherwise some part of the wheel building logic does not work on windows python 3.12also removes the unnecessary "uninstall+reinstall" dance. the script only builds the wheel, it does not (and should not) install the wheel.