-
Notifications
You must be signed in to change notification settings - Fork 8
Repair workflow for dynamically linked libraries of wheels #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
👋 Hello jakub-kocka, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
d1d5295 to
5347ca5
Compare
19d4498 to
f721ef6
Compare
1d2f1c3 to
637b1e4
Compare
9713370 to
adb6f22
Compare
4cf5be5 to
958697c
Compare
d2847df to
7c65545
Compare
|
With this done, we can turn on the Espressif's server upload, WDYT? In the following PR, I would like to introduce simple tests for the built wheels as well. |
7c65545 to
b5be694
Compare
b5be694 to
806d9d9
Compare
peterdragun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand the topic this LGTM, but I will leave the review of the process to someone that understands this more 😅
I have left a couple of comments, most of which are nitpicks, so feel free to ignore those.
806d9d9 to
9f06120
Compare
fhrbata
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, overall it looks good to me. I'm not certain if glibc 2.34 is too new, but I suppose we'll find out if there are any complaints. Additionally, I'm not sure why we're fixing the wheels in a separate step rather than immediately after they are generated. Regardless, I believe this is a significant improvement, and we'll likely discover more in the future if further adjustments are needed. Very nice work! Also thanks
for replacing the self-hosted runners with qemu.
Hi, thank you! I guess we can add upload and run the final test run, right? (cc @dobairoland ) |
dobairoland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one naming suggestion. Please wait with the merge after the end of the holidays.
9f06120 to
3312ec7
Compare
|
Thank you all for the reviews. I have addressed all the points you have mentioned, and the PR is ready for the final tests (with upload), and then it can be merged. The final tests will be made after the holidays (approximately 5th of January) If you have anything else in mind, feel free to share it. Otherwise, after a successfull tests, I believe this can be merged. |
Description
This PR adds a repair mechanism for wheels that need linked libraries. For Linux (manylinux tag), the
auditwheelpackage is used to repair already built wheels. For Windows, thedelvewheelpackage and for Mac, thedelocatepackage.The new job runs after the main builds. It runs the repair tool on every package (which needs repair), and then re-uploads the artifacts and deletes the original.
Jobs have been renamed to a better human-readable form.
Also, there is a small fix taken from automatic PR creation.
self-hosted runners were removed, qemu is used instead for Linux ARMv7 builds
Custom Docker image (built with the DockerFile in resources) is used for the manylinux ARMv7 repair
Re-enabled wheels upload to Espressif's PyPI
Related
Testing
Checklist
Before submitting a Pull Request, please ensure the following: