-
Notifications
You must be signed in to change notification settings - Fork 8
Fixed wheels issues with Python 3.14 + improvements #53
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
Conversation
👋 Hello jakub-kocka, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
be717ed to
44e46c8
Compare
|
Opened issue about |
3a2c8fc to
232271a
Compare
|
@jakub-kocka I noticed that the https://dl.espressif.com/pypi/ruamel-yaml-clib/ once again contains packages built with newer versions of setuptools, which use normalized names. For instance I believe this should be handled, probably by not creating such wheels for python < 3.10 or to make sure these are created by setuptools < 75.8.1. It's been some time since I was looking into this, so I may be missing something. Otherwise LGTM, my only worry is about reintroducing problem with the ruamel.yaml.clib package. |
@fhrbata, there is a lot of forms in which is this package uploaded, as it can be seen here https://github.com/espressif/idf-python-wheels/actions/runs/18129873715/job/51597185640#step:5:1 I am not sure if I understand this correctly. The issue is with the normalized name of the package in the binary/wheel name?
So the first bullet point can't be resolved in the releases before the fix? If there is a variety of packages with and without the name normalization, will it be OK, when |
These are build with older setuptools, which keep the dot in the distribution name, and since there is a bug in the upload script they ended up in the clib directory. IOW they are not visible to pip. The package I mentioned is in ruamel-yaml-clib and was uploaded by the action I mentioned earlier. You can take a look at the https://dl.espressif.com/pypi/clib/ruamel.yaml.clib-0.2.14-cp312-cp312-manylinux2014_aarch64.whl vs https://dl.espressif.com/pypi/ruamel-yaml-clib/ruamel_yaml_clib-0.2.14-cp39-cp39-linux_armv7l.whl The problem is IMHO described in detail in espressif/esp-idf@3bad434 which I pointed out earlier.
IMHO it's not about the pip or this fix for normalized project names. The problem is that with this fix you will IMHO reintroduce the problem described in espressif/esp-idf#15713, unless we exclude |
|
@fhrbata,
IMHO, there is no possible way the action you have mentioned could have uploaded the directory with wheels. The bug in the upload script ended up in the The packages uploaded are as follows: e.g., for ruamel_yaml_clib-0.2.14-cp310-cp310-linux_armv7l.whl |
Hm, I may be reading it wrong, but I can see https://github.com/espressif/idf-python-wheels/actions/runs/18129873715. This was started by you 3 weeks ago manually with this branch. And I can see the Since the packages were built with newer setuptools, they have the proper distribution name and the bug in the upload script does not apply. Hence the packages were uploaded to https://dl.espressif.com/pypi/ruamel-yaml-clib/ instead of https://dl.espressif.com/pypi/clib/. If you take a look at the https://dl.espressif.com/pypi/clib/ there are six |
232271a to
9e24e77
Compare
|
@fhrbata, @dobairoland, I have addressed all your pointed out misleadings and concerns. Do you agree to proceed with the test, including the upload of the wheels with the defined wheels action? |
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.
FWIW LGTM :). I guess we will see how it goes. Thank you
d50807c to
1bb1235
Compare
92a02c7 to
6432c32
Compare
d2f8cb2 to
57fd083
Compare
46f0a7d to
7130418
Compare
|
Guys, @dobairoland, @fhrbata, can you please take a final look before merging? I have addressed all the issues mentioned in the related internal tasks, as well as all those that occur during the process. |
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.
FWIW LGTM, thank you!
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
Description
exclude_listthat can not be built under Python 3.14ruamel.yaml.clib-0.2.12-cp313-cp313-linux_armv7l.whlresolution--no-build-isolationtag (now all the packages are built in the Action environment, not its own - speeding the process and resolving, for example, xcffi MacOS issue)macos-13tomacos-15for Intel CPU runnersRelated
Testing
Checklist
Before submitting a Pull Request, please ensure the following: