Add support for targeting Android#824
Conversation
|
@rgommers: This PR is ready for review. Compared to the iOS PR, this one is a bit simpler:
|
rgommers
left a comment
There was a problem hiding this comment.
Thanks @mhsmith. The main question I have now is: it looks like you're assuming that only cibuildwheel will be supported, and not regular cross compilation to Android. Is that correct? If so, I'm not sure that's right - and the iOS support doesn't do that as far as I can tell (or it's more hidden and missing code comments?).
Other questions that came up for me, like what platforms Android supports (e.g., armv7) and whether that's supported when using cpu_family = {platform.machine()!r} kinda depend on that first question.
That's a good point: Android packages could be native-compiled in an environment like Termux. I've changed the code to only use cross-compilation mode when it detects it's running within cibuildwheel. As far as I know, there's no native compilation option on iOS, because the platform is too restrictive, e.g. apps cannot create subprocesses.
Officially Python only supports Android on aarch64 and x86_64. However, it's used unofficially on other architectures, so I've added coverage for all the known values of |
Anecdotally, Termux for me shows python 3.12 with a platform value of "Linux". :) |
|
Yes, that was changed in Python 3.13 when we added official Android support. cibuildwheel will only support Android on Python 3.13 or later, so the cross file should use the new value. |
|
We get the occasional Termux-related bug report in NumPy, so that does appear to be used indeed. The approach you took now looks better to me. |
|
I'm not aware of any public way to detect that a build is running within cibuildwheel. The environment variables in their documentation are all used as configuration inputs to cibuildwheel, rather than outputs that you can rely on being set within the build. So I've changed it to use |
|
Thanks for the update.
It does depend on tools, right? I guess all we care about here is that |
|
Python doesn't really provide any "proper cross-compiling" mechanism, so the crossenv-like approach is the only feasible option for cross-compiling Android and iOS wheels at the moment.
I've mentioned that in the comment on line 749. |
|
Thanks @mhsmith. For context: Python itself doesn't, and probably won't for at least a couple more years, but both Meson and CMake have solid cross-compilation support, and with Meson cross files + PEP 739 ( Support in Meson for PEP 739 has landed, and support in tl;dr we should not build in implicit assumptions around |
dnicolodi
left a comment
There was a problem hiding this comment.
I am not sure I understand what this PR implements. The commit message states that this implements support for Android. However, it seems to implement support for cross-compiling for Android. I think native compilation for Android works already as I don't have evidence of the contrary and this PR does not affect native compilation.
Furthermore, this PR seems to only address the case of a cross compilation environment setup by cibuildwheel. However, the code and the comments therein seem to indicate that this is somehow more generic.
I think that supporting cibuildwheel covers an important use case, thus this PR is good to have. On the other hand, it seems that we are designing support for new systems with the same ugly hacks that were used in the past to work around limitations it the build tools. Concretely, i don't understand why we need to rely on sys.cross_compiling. If this is designed to support only cibuildwheel I am sure we can use some other indicator to detect the cross compilation environment, especially if we already rely on environment variables to setup the compilation environment.
|
Thanks for the comments. I have some business travel and vacation coming up, so it'll probably take me a couple of weeks to reply. |
|
@dnicolodi I think your main review comment above covers the same ground that my comments did? What this implements is the same as for iOS. And yes I already pointed out that regular cross-compiling is a thing and this is
Initial reply from @mhsmith in this comment above. I'd also like a cleaner way to do this. I looked through the |
|
I haven't read the conversation in detail, and my questions are maybe due to my complete ignorance about how you go about compiling something for Android. I saw that you planned to merge this unless someone spoke up, thus I assumed you are happy with the status of the PR. I noticed the disconnect between the commit message (that is the base for our changelog entries, especially when reporting about features not implemented by one of the maintainers) and I thought that some clarification is necessary. It is fine to merge this to support the |
My intention was to avoid privileging cibuildwheel over any similar tools, but I guess we are still making some cibuildwheel-specific assumptions, so this makes sense. I'm not aware of any other active Android Python cross-compilation tools, but if they appear in the future, we can revisit this. |
I've updated the commit message. |
dnicolodi
left a comment
There was a problem hiding this comment.
Thanks for working on this. Just a couple of minor things, mostly to make the whole easier for me to understand when I'll go back to it in the future.
ebea700 to
4a53fe0
Compare
|
Thanks. Merged with a couple of small tweaks. |
Using the same approach as on iOS.
This PR branch is temporarily used by several other PRs: see the cross-references below.