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

gh-116622: Android sysconfig updates #118352

Merged
merged 4 commits into from
May 1, 2024
Merged

Conversation

mhsmith
Copy link
Member

@mhsmith mhsmith commented Apr 27, 2024

This is the final feature PR for adding Android support. All that remains after this is test infrastructure and documentation updates.

  • Implemented sysconfig.get_platform in the format specified by PEP 738.

  • Added the -Wl,--no-undefined linker flag, both for CPython itself, and for third-party packages built using the sysconfig LDFLAGS. Based on past experience of building packages for Android, this can save a lot of time by detecting problems at compile time which would otherwise happen at runtime.

  • Added Android to the Tier 3 support list.

@mhsmith
Copy link
Member Author

mhsmith commented May 1, 2024

@encukou: Thanks for your help on my Android PRs in the last few days. Could I trouble you to review this one as well? It's pretty small, and it's the final one that needs to get merged before the feature freeze on May 7.

Comment on lines +610 to +613
"x86_64": "x86_64",
"i686": "x86",
"aarch64": "arm64_v8a",
"armv7l": "armeabi_v7a",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is in the PEP (sorry I didn't catch it) but let me ask -- more to bring this to your attention than anything else:
Is there a specific reason to use Android arch names, rather than ones from configure?

For reference: Fedora used their own arch names, and spent several years migrating to the configure ones when wheels became more universal.
Android wheels will probably always be separate, but it seems they might be compared to other Linux-y wheels more often than other Android apps. Also, maintaining this dict in CPython might not be worth it.

If CPython-style arch names would be better, this is the time to change them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using autoconf-style architecture names everywhere else, including in extension module filenames (see test_android_ext_suffix in test_sysconfig.py). This PR really only affects the platform tag in wheel filenames.

The main reason to use this format is for consistency with the existing Chaquopy wheel repository, which contains over a thousand wheels for Python 3.8-3.12. Having two formats in use would complicate package installers such as pip (which we intend to contribute Android and iOS support for in the near future), and higher-level app building tools such as Briefcase.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using autoconf-style architecture names everywhere else

Right, and this inconsistency is somewhat jarring.
If you'd be interested in setting up aliases in the Chaquopy repository, or adding aliases to packaging, let me know & I can help.

But, no reason for that to hold this PR back.

@encukou
Copy link
Member

encukou commented May 1, 2024

Hah, it was the next thing on my list when you wrote that!

Looks good, tell me to merge & I will :)

@encukou encukou enabled auto-merge (squash) May 1, 2024 16:45
@encukou encukou merged commit 7595511 into python:main May 1, 2024
37 checks passed
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants