fix: ensure many/musl tags not selected #698
Conversation
Signed-off-by: Henry Schreiner <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #698 +/- ##
=======================================
Coverage 86.30% 86.30%
=======================================
Files 64 64
Lines 3315 3315
=======================================
Hits 2861 2861
Misses 454 454 ☔ View full report in Codecov by Sentry. |
|
Hi @henryiii, looks like after this change it would not be possible to set "manylinux" platform tag during the wheel packaging. |
|
It was not correct. The fix is to run |
|
This is the bug in packaging: pypa/packaging#160 It was sadly closed in favor of a new tag "local" that never went anywhere. Producing a manylinux/musllinux by default is clearly incorrect. |
Follow up to #698. The statement about producing `linux` wheels in build.md has been there since well before this bug was fixed. :) Hopefully this will make it a bit more discoverable. Signed-off-by: Henry Schreiner <[email protected]>
Hi @henryiii , thank you for the answer. You are right that default value should not be a manylinux. But by this change you disabled an ability to set this tag manually at all. We build wheels which satisfy manylinux requirements and using additional tool to change the tag only is an unnecessary dependency for us( |
|
No other build backends (such as setuptools) has a way to configure this. It's very dangerous; a broken "manylinux" wheel will always work on the system it was built on; it also may work on some other systems. It's very hard to tell without validating it, which is what auditwheel does. It also depends on where you build the wheel; if you are using the manylinux docker container, then maybe you are producing manylinux-compliant wheels already, but if someone builds on a normal OS, it might not be compliant. The only way to be sure is to verify with auditwheel. Also, you are producing a manylinux_2_27 tag on ARM, what is that? Manylinux docker images come in 2_24 (abandoned) and 2_28, not 2_27 as far as I know. And manylinux2014 is the standard tag name for 2_17, but this is grabbing the less-compatible "modern" name accidentally. Also, when I run auditwheel on your published wheel, I get "libopenvino" can't be found; checking, it's actually inside a different wheel, openvino! This is not allowed by manylinux. All wheels must bundle their own dependencies and name mangle them so they can be safely loaded with other wheels that have different requirements on the lib. Now, if you specifically require exactly one version of $ # I'm starting from PyPI, you don't need these steps
$ docker run --rm -it quay.io/pypa/manylinux2014_x86_64
# python3.12 -m pip download openvino-tokenizers
Saved /openvino_tokenizers-2024.1.0.0-81-py3-none-manylinux_2_17_x86_64.whl
Saved /openvino-2024.1.0-15008-cp312-cp312-manylinux2014_x86_64.whl
Saved /numpy-1.26.4-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Saved /openvino_telemetry-2024.1.0-py3-none-any.whl
Saved /packaging-24.0-py3-none-any.whl
Successfully downloaded openvino-tokenizers openvino numpy openvino-telemetry packaging
# pipx install wheel
installed package wheel 0.43.0, installed using Python 3.10.14
These apps are now globally available
- wheel
done! ✨ 🌟 ✨
# wheel tags --remove --platform-tag linux_x86_64 openvino_tokenizers-2024.1.0.0-81-py3-none-manylinux_2_17_x86_64.whl
openvino_tokenizers-2024.1.0.0-81-py3-none-linux_x86_64.whl
# # Now we have the current output, a linux wheel
# # This is the line you need
# auditwheel repair openvino_tokenizers-2024.1.0.0-81-py3-none-linux_x86_64.whl --exclude libopenvino_tensorflow_frontend.so.2410 --exclude libopenvino.so.2410
INFO:auditwheel.main_repair:Repairing openvino_tokenizers-2024.1.0.0-81-py3-none-linux_x86_64.whl
INFO:auditwheel.lddtree:Excluding libopenvino_tensorflow_frontend.so.2410
INFO:auditwheel.lddtree:Excluding libopenvino.so.2410
INFO:auditwheel.wheeltools:Previous filename tags: linux_x86_64
INFO:auditwheel.wheeltools:New filename tags: manylinux_2_17_x86_64, manylinux2014_x86_64
INFO:auditwheel.wheeltools:Previous WHEEL info tags: py3-none-linux_x86_64
INFO:auditwheel.wheeltools:New WHEEL info tags: py3-none-manylinux_2_17_x86_64, py3-none-manylinux2014_x86_64
INFO:auditwheel.main_repair:
Fixed-up wheel written to /wheelhouse/openvino_tokenizers-2024.1.0.0-81-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whlThis is explicit, and other dependencies can't sneak in over time. (This is how wheels using CUDA are handled, FYI) As an added bonus, notice the wheel is also tagged with the compat tag |
This should have been done anyway - it is being done now to ensure we have the correct platform tags. The scikit-build-core 0.9.0 release changed such that the platform tags were now "correct", but not what we need - `linux_x86-64` and `linux_aarch64` instead of `manylinux_2_17` and `manylinux_2_28`. Reference: scikit-build/scikit-build-core#698 (comment) -
This should have been done anyway - it is being done now to ensure we have the correct platform tags. The scikit-build-core 0.9.0 release changed such that the platform tags were now "correct", but not what we need - `linux_x86-64` and `linux_aarch64` instead of `manylinux_2_17` and `manylinux_2_28`. Reference: scikit-build/scikit-build-core#698 (comment) <!-- Thank you for contributing to pynvjitlink :) Here are some guidelines to help the review process go smoothly. 1. Please write a description in this text box of the changes that are being made. 2. Please ensure that you have written units tests for the changes made/features added. 3. If you are closing an issue please use one of the automatic closing words as noted here: https://help.github.com/articles/closing-issues-using-keywords/ 4. If your pull request is not ready for review but you want to make use of the continuous integration testing facilities please label it with `[WIP]`. 5. If your pull request is ready to be reviewed without requiring additional work on top of it, then remove the `[WIP]` label (if present) and replace it with `[REVIEW]`. If assistance is required to complete the functionality, for example when the C/C++ code of a feature is complete but Python bindings are still required, then add the label `[HELP-REQ]` so that others can triage and assist. The additional changes then can be implemented on top of the same PR. If the assistance is done by members of the rapidsAI team, then no additional actions are required by the creator of the original PR for this, otherwise the original author of the PR needs to give permission to the person(s) assisting to commit to their personal fork of the project. If that doesn't happen then a new PR based on the code of the original PR can be opened by the person assisting, which then will be the PR that will be merged. 6. Once all work has been done and review has taken place please do not add features or make changes out of the scope of those requested by the reviewer (doing this just add delays as already reviewed code ends up having to be re-reviewed/it is hard to tell what is new etc!). Further, please do not rebase your branch on main/force push/rewrite history, doing any of these causes the context of any comments made by reviewers to be lost. If conflicts occur against main they should be resolved by merging main into the branch used for making the pull request. Many thanks in advance for your cooperation! -->
Auditwheel is really useful to verify the manylinux wheel, but I would disagree that this is only one way to build correct manylinux wheel. The provided solution require to manually exclude some libraries( |
This was tagging as manylinux (at least sometimes).