Add support for abi3.abi3t tag from PEP 803#1099
Add support for abi3.abi3t tag from PEP 803#1099
Conversation
src/packaging/tags.py
Outdated
| yield from (Tag(interpreter, "abi3", platform_) for platform_ in platforms) | ||
| if use_abi3t: | ||
| yield from ( | ||
| Tag(interpreter, "abi3.abi3t", platform_) for platform_ in platforms |
There was a problem hiding this comment.
The tags returned here should all be simple tags, not compound ones (i.e., no dots in the components).
src/packaging/tags.py
Outdated
| for platform_ in platforms: | ||
| version = _version_nodot((python_version[0], minor_version)) | ||
| interpreter = f"cp{version}" | ||
| yield Tag(interpreter, "abi3.abi3t", platform_) |
There was a problem hiding this comment.
Same here - no dots in returned tags.
|
@pfmoore, thank you! I also pushed an accompanying fix to my setuptools PR that allows setuptools to correctly support compressed ABI tags without incorrectly returning compressed tags from |
|
Just to be 100% clear here, when you returned You seem now to be saying that free threaded builds only support abi3t. I think one of us is confused here 🙁 |
Take a look at the test I added that checks the output of It turns out setuptools doesn't currently support compressed ABI tags unless I make |
|
OK, some thoughts. But I'm getting way out of my depth here, and I'm concerned that we're not going off the PEP here but rather trying to define behaviours that aren't clearly stated in the PEP. It's quite likely we need to get the PEP clarified before making decisions here. For example, why is I can't comment on the setuptools point. But I do know that pip won't work if I feel that this might need to be brought back to Discourse. At the very least, I think we need more eyes on the problem. |
Because it simply didn't occur to me that packaging/src/packaging/tags.py Lines 654 to 660 in cd1520c I adjusted the logic in my latest commit.
I agree, let's ask Petr to clarify the tag priority order for the GIL-enabled build, since it's ambiguous in PEP 803 right now. |
|
And after trying to draft a message about this, I think I finally understand what needs to happen here. Installers should only install wheels on the free-threaded build with an abi3t tag, that means I was confused in my original implementation, because I thought that I was also confused by the fact that setuptools didn't handle compressed tag sets in abi tags. I'll raise this issue with the setuptools maintainers separately from PEP 803 support. I agree that this is a little "sparely" described in the PEP. I'll post a followup to discourse about this. |
src/packaging/tags.py
Outdated
| yield Tag(interpreter, "abi3", platform_) | ||
| if use_abi3: | ||
| yield Tag(interpreter, "abi3", platform_) | ||
| if use_abi3t and minor_version > 14: |
There was a problem hiding this comment.
| if use_abi3t and minor_version > 14: | |
| if use_abi3t: |
[edit: practically this could be minor_version >= 13, but it's not worth the special case.]
|
Given the clarification in the Discourse thread, I'd strongly recommend adding some sort of test(s) here to validate that if you change the Python implementation from 316 to 316t, the list of tags remains unchanged except for |
| tags.Tag("cp316", "cp316t", "platform"), | ||
| tags.Tag("cp316", "abi3t", "platform"), | ||
| tags.Tag("cp316", "none", "platform"), | ||
| tags.Tag("cp315", "abi3t", "platform"), |
There was a problem hiding this comment.
The Discourse discussion suggests that cp314-abi3t, cp313-abi3t, cp312-abi3t, ... should also be returned here...
|
I think the last push responds to all comments. Thanks for helping me to understand this better! |
|
This looks reasonable to me. |
This updates the tags logic to handle
abi3tfrom PEP 803. Also adds tests to validate everything.Opening as a draft for visibility and to aid end-to-end testing of PEP 803. See https://github.com/Quansight-Labs/stable-abi-testing.