Skip to content
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

Add support for iOS and Android platform tags. #17559

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Conversation

freakboy3742
Copy link
Contributor

@freakboy3742 freakboy3742 commented Feb 5, 2025

Adds support for PEP 730 iOS tags, and PEP 738 Android tags.

Fixes #17549
Fixes #17557
Closes #17556

@freakboy3742 freakboy3742 requested a review from a team as a code owner February 5, 2025 02:40
@freakboy3742
Copy link
Contributor Author

FYI @mhsmith

@di
Copy link
Member

di commented Feb 5, 2025

I think the PEP 738 bits here are blocked as noted in #17556.

But if you would like to ship the PEP 730 part separately, the specification in the PEP seems sufficient and I'd be ok merging that today.

Although this seems to be missing the minimum iOS version?

@freakboy3742
Copy link
Contributor Author

I think the PEP 738 bits here are blocked as noted in #17556.

I've submitted pypa/packaging.python.org#1804 to address that requirement.

But if you would like to ship the PEP 730 part separately, the specification in the PEP seems sufficient and I'd be ok merging that today.

Although this seems to be missing the minimum iOS version?

As noted in this comment about Android, while Python sets a default minimum iOS version (13.0 at present), it's possible to use older versions. iOS 12.0 is known to work with some compile-time warnings. I haven't tested older versions, but I wouldn't be surprised to learn older iOS versions (back to 8.0, when support for dynamic linking was added) could be made to work.

On that basis, I figured it wasn't worth adding an explicit version check, as there's nothing that fundamentally prevents older versions from working if you've gone to the trouble of compiling Python with an earlier minimum iOS version. However, I'm happy to add a minimum version check (12.0 would be a reasonable bound based on the wording of the PEP) if you feel it is required.

@di
Copy link
Member

di commented Feb 5, 2025

I've submitted pypa/packaging.python.org#1804 to address that requirement.

Thanks! I added a review there.

On that basis, I figured it wasn't worth adding an explicit version check, as there's nothing that fundamentally prevents older versions from working if you've gone to the trouble of compiling Python with an earlier minimum iOS version. However, I'm happy to add a minimum version check (12.0 would be a reasonable bound based on the wording of the PEP) if you feel it is required.

Got it, I think I misread https://peps.python.org/pep-0730/#packaging which is just providing "12.0" as an example and not an actual minimum. I'm OK with not including a minimum but I think we need to specify what we consider valid values for these fields just a little more (as I noted in my review of the update to the guide).

warehouse/utils/wheel.py Outdated Show resolved Hide resolved
@mhsmith
Copy link

mhsmith commented Feb 5, 2025

I'm OK with not including a minimum

I agree with not including a minimum, for the reasons given here.

@freakboy3742
Copy link
Contributor Author

@di pypa/packaging.python.org#1804 has now been merged; was that the only remaining blocker on merging this?

@di di merged commit d0dbd6d into pypi:main Feb 10, 2025
20 checks passed
@freakboy3742 freakboy3742 deleted the mobile-tags branch February 10, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants