Add support for building Android wheels #2349
Conversation
|
Let us know if you need (the rest of) CI triggered. :) |
|
@henryiii: This is close to being complete, so please enable the rest of CI. |
|
To run the integration tests on most of the CI machines, it looks like I'll need to automate installation of the correct Python version. For macOS this can be done the same way as iOS, but for Linux there's no existing code to reuse, because the native Linux build uses Docker. So I'll probably implement something that uses python-build-standalone, unless anyone has another suggestion. Apart from that, I think this PR is complete enough now that it's worth reviewing. @freakboy3742 and anyone else who's interested. |
|
python-build-standalone is fine, in fact, I'd like to use that for pyodide in the future, too. |
|
I've already written code to install a version of python-build-standalone in #2002, along with version pinning etc. I think it would work nicely here too. |
cibuildwheel/platforms/android.py
Outdated
|
|
||
|
|
||
| @dataclass | ||
| class Builder: |
There was a problem hiding this comment.
I'd say +1, for the sake of consistency with the other platforms, and passing around lots of variables (while it's a bit of a drag) prevents other classes of bugs in its explicitness.
|
Is |
|
(I see you've been using merge commits, so I forced myself to merge with main instead of rebasing on it... I needed the recent changes in main to test the pyodide-build fix in my downstream PR, and it's still based on this PR) We will aim to make this will be the first feature of 3.1, 3.0 is shipping soon and @joerick won't have time to review before 3.0 (and I want to get at least pybind11 working with it, which also will take time). |
I think it was only necessary on iOS because when building for the simulator, you're really building for macOS, so when a normal macOS tool is used by accident, the build might continue for some time until eventually failing in a way which doesn't clearly indicate the cause. But Android isn't binary-compatible with either Linux or macOS, so when you use the wrong tool you usually find out immediately. |
|
I wanted to play a bit with this before reviewing (not sure when I'll have time to review it properly).
I think there's one thing missing in exported environment variables to create such a toolchain file (or just passing the correct arguments to CMake): the Android NDK path. It can be inferred from the compiler path (in CC) but that seems a bit hacky. Here are the extra arguments I had to pass to CMake (the last one being specific to my project): elif is_android:
_, level, arch = platform.split("-")
if arch.startswith("arm"):
arch = arch.replace("_", "-")
cc = Path(os.getenv("CC", sysconfig.get_config_var("CC"))).resolve(strict=True)
android_home_env = os.getenv("ANDROID_HOME")
if android_home_env is None:
msg = "The ANDROID_HOME environment variable is required."
raise ValueError(msg)
android_home = Path(android_home_env)
android_ndk_root = android_home / "/".join(cc.relative_to(android_home).parts[:2])
extra_config.append("-DCMAKE_SYSTEM_NAME=Android")
extra_config.append(f"-DCMAKE_SYSTEM_VERSION={level}")
extra_config.append(f"-DCMAKE_ANDROID_ARCH_ABI={arch}")
extra_config.append(f"-DCMAKE_ANDROID_NDK={android_ndk_root}")
extra_config.append("-DCMAKE_ANDROID_STL_TYPE=none")CMake (and maybe other tools) will read the Android NDK path from Should this be handled by the build backends or should it be cibuildwheel responsibility to create the toolchain file & set the CMAKE_TOOLCHAIN_FILE environment variable (which would be a new thing, so far, e.g. on macOS where it's the backend responsibility to pass the correct CMAKE_OSX_ARCHITECTURES) ? |
|
@freakboy3742 FYI: First instance I've seen of "Messages dropped during live streaming" breaking the cibuildwheel iOS tests: https://github.com/pypa/cibuildwheel/actions/runs/15564982650/job/43826807636 |
I've now added a minimal implementation of that to this PR.
CMake comes with a large and complex body of code to determine the paths and flags to use when calling NDK tools. However, this is unnecessary in cibuildwheel, because we've already set those things in environment variables. So to avoid confusion, the toolchain file calls I'm currently still working through all the issues uncovered by pybind11, but I'll look at pybase64 after that. |
joerick
removed
the
Hold for future release
Thanks for your detailed answer about CMake handling @mhsmith. Everything works out of the box with the toolchain file injection added. |
|
OK, I have the pybind11 tests passing now with the code in henryiii/pybind11#23. On Android the OS doesn't provide a C++ library, so every app needs to bundle its own copy. This required giving cibuildwheel some auditwheel-like code which adds the C++ library to the wheel in the repair step if necessary. Eventually Android support should be added to auditwheel itself, but I can see that would be a substantial amount of work. |
|
I'm trying this out locally on pyinstrument. Building works, but I'm hitting an error running tests. Here's the output log
Edit: I've updated the SDK tools and still the same error... I've restarted my Mac and that seems to have fixed that particular issue. Now I've got this problem- It's curious. It looks like the test passes, but an error is reported: |
| if not configs: | ||
| return | ||
|
|
||
| try: |
There was a problem hiding this comment.
I think it would be worth adding a check here for the ANDROID_HOME environment variable and returning a nice error message if it's not found. Ideally with a link to the docs explaining how to download the SDK.
| call( | ||
| python_dir / "android.py", | ||
| "test", | ||
| "--managed", | ||
| "maxVersion", | ||
| "--site-packages", | ||
| site_packages_dir, | ||
| "--cwd", | ||
| cwd_dir, | ||
| *test_args, | ||
| env=build_env, | ||
| ) |
There was a problem hiding this comment.
Not a cibw thing, but I see in the logs for this that the testbed is using a raw.githubusercontent.com URL for downloading Gradle. We've seen HTTP 429 requests on these URLs in the past, specifically in #2002. I suspect Github might rate-limit based on volume of requests to a URL, so if it shipped in cibuildwheel we'd probably see the same. If you can download from a release URL instead, that might be better.
There was a problem hiding this comment.
We already experienced this problem when running the CPython test suite, and added a workaround in python/cpython#133193. That was 6 weeks ago, and I don't think the download has ever failed again (except for one general outage that affected all of GitHub).
| # Some packages may recognize sys.cross_compiling from the crossenv tool. | ||
| sys.cross_compiling = True # type: ignore[attr-defined] | ||
| sys.getandroidapilevel = cross_getandroidapilevel # type: ignore[attr-defined] | ||
| sys.implementation._multiarch = os.environ["HOST"] # type: ignore[attr-defined] |
There was a problem hiding this comment.
Who sets the HOST variable? On my Mac this variable contains the machine hostname e.g.
joerick@joerick5 ~> echo $HOST
joerick5.local
If its set by us, maybe we should consider a more specific name.
There was a problem hiding this comment.
In this context it originates from CPython's android.py script. It was inspired by conda-build, or its compiler activation scripts, which set the HOST and BUILD variables to support build scripts like this.
However, the clash with zsh's variable of the same name has caused some problems:
- activate/deactivate changes to HOST environment variable conda/conda#7031
- ctng-compilers-activation-feedstock scripts shouldn't set HOST AnacondaRecipes/aggregate#151
It's probably too late for Conda to change this, but there's no reason for us to perpetuate it. So I've changed _cross_venv.py to receive the host triplet in a CIBW_HOST_TRIPLET environment variable instead, replacing the boolean CIBW_CROSS_VENV.
| build_env, android_env = setup_env(config, build_options, build_path, python_dir) | ||
| before_build(build_options, build_env) | ||
| built_wheel = build_wheel(build_options, build_path, android_env) | ||
| repaired_wheel = repair_wheel( | ||
| build_options, build_path, build_env, android_env, built_wheel | ||
| ) |
There was a problem hiding this comment.
It will make it more verbose, which is a shame, but I think I'd feel better if these functions used keyword args - there are potential errors where e.g. android_env and build_env could get mixed up and the typechecker wouldn't catch it.
There was a problem hiding this comment.
I do think using the env var to toggle this venv's cross-build mode is very clever/elegant :)
| pb = ProjectBuilder.from_isolated_env(AndroidEnv(), build_options.package_dir) | ||
| if pb.build_system_requires: | ||
| call("pip", "install", *pb.build_system_requires, env=build_env) |
There was a problem hiding this comment.
So I understand this, if a package had an android wheel, ProjectBuilder would find that file and return a full URL to it here?
But the pip install command is executed in the build_env. So if it was compiling from a tar.gz it would build for the wrong platform in that case?
| for key in ["CFLAGS", "CXXFLAGS"]: | ||
| android_env[key] += " " + opt |
There was a problem hiding this comment.
Do we know that CFLAGS/CXXFLAGS exists in android_env? I'm confused why this line doesn't raise a KeyError.
Oh, I suppose it's always returned by android.py env. It might be worth making this more robust though.
| env_output = call(python_dir / "android.py", "env", env=build_env, capture_stdout=True) | ||
| for line in env_output.splitlines(): |
There was a problem hiding this comment.
Can we be sure that environment variables won't contain newlines? We've seen a few occasions where that wasn't the case. Generally I prefer to use JSON as the format to pass env vars between processes like this, if possible.
| if test_args[:2] in [["python", "-c"], ["python", "-m"]]: | ||
| test_args[:3] = [test_args[1], test_args[2], "--"] | ||
| elif test_args[0] in ["pytest"]: | ||
| test_args[:1] = ["-m", test_args[0], "--"] |
There was a problem hiding this comment.
Let's add a warning here to match the iOS behaviour.
cibuildwheel/cibuildwheel/platforms/ios.py
Line 630 in 5f22145
|
|
||
| * `python -c command ...` (Android only) | ||
| * `python -m module-name ...` | ||
| * `pytest ...` (deprecated; converted to `python -m pytest ...`) |
There was a problem hiding this comment.
I'd rather just not document this form, I think it's more of a convenience feature for users migrating from other platforms.
| * `pytest ...` (deprecated; converted to `python -m pytest ...`) |
| sts = strcmp(content, "spam"); | ||
| sts = strcmp(content, "spam") != 0; |
There was a problem hiding this comment.
I'm curious why this change?
| <sup>³ Requires a macOS runner; runs tests on the simulator for the runner's architecture.</sup> | ||
|
|
||
| <sup>³ Requires a macOS runner; runs tests on the simulator for the runner's architecture.</sup><br> | ||
| <sup>⁴ Building for Android requires the runner to be Linux x86_64, macOS ARM64 or macOS x86_64. Testing is supported on the same platforms, but also requires the runner to either be bare-metal, or support nested virtualization. CI platforms known to meet this requirement are: GitHub Actions Linux x86_64.</sup><br> |
There was a problem hiding this comment.
According to this it should also be possible to run the emulator for testing on "Azure Pipelines macOS (Microsoft-hosted agent)". But I haven't tried it yet.
Should there be a recommendation / example to link against the static one if possible rather than bundle ? |
| call( | ||
| "patchelf", | ||
| "--set-rpath", | ||
| f"${{ORIGIN}}/{libs_dir.relative_to(path.parent)}", |
There was a problem hiding this comment.
On this line I get the following error in a private pybind11 project:
Repairing wheel...
+ python -c 'import sysconfig; print(sysconfig.get_config_var("ANDROID_API_LEVEL"), end="")'
+ patchelf --set-soname libc++_shared-cd110349.so /tmp/cibw-run-536mrs3q/cp313-android_x86_64/repaired_wheel/unpacked/myprivatelib_pkg.libs/libc++_shared-cd110349.so
+ patchelf --replace-needed libc++_shared.so libc++_shared-cd110349.so /tmp/cibw-run-536mrs3q/cp313-android_x86_64/repaired_wheel/unpacked/myprivatelib/somelib.cpython-313-x86_64-linux-android.so
Traceback (most recent call last):
File "<frozen runpy>", line 198, in _run_module_as_main
File "<frozen runpy>", line 88, in _run_code
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/__main__.py", line 511, in <module>
main()
~~~~^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/__main__.py", line 63, in main
main_inner(global_options)
~~~~~~~~~~^^^^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/__main__.py", line 207, in main_inner
build_in_directory(args)
~~~~~~~~~~~~~~~~~~^^^^^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/__main__.py", line 372, in build_in_directory
platform_module.build(options, tmp_path)
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/platforms/android.py", line 127, in build
repaired_wheel = repair_wheel(
build_options, build_path, build_env, android_env, built_wheel
)
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/platforms/android.py", line 421, in repair_wheel
repair_default(android_env, built_wheel, repaired_wheel_dir)
~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/site-packages/cibuildwheel/platforms/android.py", line 496, in repair_default
f"${{ORIGIN}}/{libs_dir.relative_to(path.parent)}",
~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.13.4/x64/lib/python3.13/pathlib/_local.py", line 385, in relative_to
raise ValueError(f"{str(self)!r} is not in the subpath of {str(other)!r}")
ValueError: '/tmp/cibw-run-536mrs3q/cp313-android_x86_64/repaired_wheel/unpacked/myprivatelib_pkg.libs' is not in the subpath of '/tmp/cibw-run-536mrs3q/cp313-android_x86_64/repaired_wheel/unpacked/myprivatelib'
My final project structure should look like this:
myprivatelib_pkg:
-----------------
myprivatelib
├─ __init__.py
├─ version.py
└─ somelib.cpython-313-x86_64-linux-android.so
In my CMakeLists.txt I use:
install(TARGETS somelib DESTINATION myprivatelib)
If I change it to:
install(TARGETS somelib DESTINATION .)
the "Repairing wheel" step works, but my project structure is wrong and my unit-tests are failing.
That would be unsafe when a package has multiple .so files, for the reasons given here. In theory it may even be unsafe if there are multiple packages each linked against their own separate shared libc++. This exact issue wouldn't arise on Linux because the manylinux standard requires the C++ library to be provided by the OS. However, it could arise in other libraries which auditwheel includes multiple copies of in multiple packages. I think the reason why we usually get away with this on Linux is that separate packages usually interact with each other via Python interfaces rather than C++ ones, so the crash scenarios described in the above link do not occur. The same should be true on Android. |
The corresponding CPython PR, from which the Android Python releases in build-platforms.toml are being generated, is python/cpython#132870.