-
Notifications
You must be signed in to change notification settings - Fork 172
python: Write WHEEL tags in expanded form #989
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
The wheel specification requires that the Tag field contains all of the tags in their expanded form. Leaving the compressed form unchanged is usually fine, but pip (check only!) misidentifies the wheel as incompatible with the system when fed a compressed wheel tag set
|
Cool. Would be great to have it fixed in the next version. Currently we had to workaround it in |
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.
@bglw Pagefind is great, thanks for building it.
We started using it in Airflow since yesterday with apache/airflow#59658 after a lot of frustration with Sphinx's search.
It would be great to get this merged in so we use this with pip and uv both.
bglw
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.
Makes sense to me — thanks for opening the PR to fix!
(this reminds me, Pagefind has tipped over a pypi limit which is blocking a new publish there, I'll look at that soon as well)
Head branch was pushed to by a user without write access
|
Sorry about failing CI. I pushed a new commit to fix Ruff format @bglw! |
|
I am runnig a test for it apache/airflow#59816 - but it looks great, I installed it locally and checked that One thing that I might want to suggest (if I correctly understand the split between pagefind and pagefind-bin) you do in the future (maybe 1.5.0) - is to explicitly pin pagefind-bin to the same version as pagefind Currently in I think what the intetion is however - is to always install the same "bin" version as the "non-bin" version when "bin" extra is used, so likely this would be better: or maybe (depending what your compatibility promises are betweeen bin and non-bin distributions). Side effect of the current "requires-dist" is that when you do this: or It follows https://peps.python.org/pep-0440/#handling-of-pre-releases handling:
And it installs: Which is good by the "letter" of PEP-440, but it's not likely following the spirit and user expectation. That's why in airflow I had to change: into: Because just Also - another side effect of non-pinning is that if someone does today: and then (after you release 1.5.0) They will still keep pagefind-bin 1.4.0 installed - even if pagefind will be 1.5.0 (which is likely not what you would like to happen). |
|
Aha, great tips, tyvm. Yes, the intention is indeed that versions are always in lockstep, which is enforced in the npm wrapper. I'm less versed in the Python publishing world so mostly I rely on helpful people pointing things like this out! :) I'll get these changes in. |
|
I should also note that a big change in this current alpha (so far) is that the search in the browser has been offloaded into a web worker. I've tested it a bunch and I'm not aware of anything funky, and it's great for perceived performance, but just an FYI in case you notice anything due to that. |
I tested it with locally build airflow docs and have not seen any issues so far. I am just about to publish this change to Airflow main and cherry-pick it to 3.1.6 so that when we publish documentation next time we will see it live. But we have a lot of contributors who work on the docs locally, so in case we see anythng Also FYI - coincidentally - I am also one of the mentors in the Apache Software Foundation incubator - and we are testing pagefind as a search solution to index our CWiki pages and we are discussing whether (and how) to index all contents instead of just a subset of it -> https://lists.apache.org/thread/ro2973lzyqd5hfdhb7fggqr39sth7gwm if you would like to chime in the discussion - feel free @bglw :) |
Hello, pip maintainer here. 👋
The wheel specification requires that the
Tagfield in.dist-info/WHEELcontains all of the tags in their expanded form. Leaving the compressed form unchanged is usually "fine", but pip (check only!) misidentifies the wheel as incompatible with the system when fed a compressed wheel tag set. We just got a bug report about this: pypa/pip#13709While I'm unfamiliar with this repository and can't test the build script directly, the tag expansion logic has been tested below (it also comes from the specification FWIW).