Android improvements needed for building NumPy and related packages - #2695
Conversation
mhsmith
changed the title
|
Is there anything you'd like to pull out and get in for the next version? |
|
Not at the moment, thanks. |
* Set up Android env after installing pkgconf * Add tests for successfully using an older API level * Previous commit's auditwheel failure is fixed in the auditwheel PR
| # TODO: use an official auditwheel version once | ||
| # https://github.com/pypa/auditwheel/pull/643 has been released, and add it to the | ||
| # constraints files. |
There was a problem hiding this comment.
auditwheel 6.7.0 has been released
There was a problem hiding this comment.
I've updated the PR to use it.
|
Are we going to try to get this into 4.0? Still waiting on PyPy anyway. |
joerick
left a comment
joerick
left a comment
There was a problem hiding this comment.
Really nice work on this. If we can get this into 4.0 that would be nice, but I suppose it probably shouldn't block for more than a few days.
The other question would be - can/should xbuild-files also be supported on iOS and pyodide too?
| ARCHS = ["aarch64", "x86_64"] | ||
|
|
||
| # The compiler is built for Linux x86_64, so we use Docker on macOS. | ||
| DOCKER_IMAGE = "debian:trixie" |
There was a problem hiding this comment.
Would we get away with debian:trixie-slim here?
There was a problem hiding this comment.
Apparently yes, so I've changed it.
| # Merge the extracted trees together, along with the necessary parts of the NDK. Based on | ||
| # https://github.com/kivy/python-for-android/blob/develop/pythonforandroid/recipes/fortran/__init__.py) | ||
| flang_toolchain = tmp_dir / "toolchain" | ||
| (tmp_dir / "out/install/linux-x86/clang-dev").rename(flang_toolchain) |
There was a problem hiding this comment.
Does this assume the build machine is x86? Perhaps the build machine is (linux) aarch64.
There was a problem hiding this comment.
None of the other Android development tools are available for Linux aarch64, so that isn't a concern.
| args = ["docker", "run", "--rm", "--platform", "linux/amd64"] | ||
| for path in ["/private", "/Users", "/tmp"]: | ||
| # Docker on macOS only allows certain directories to be mounted as volumes | ||
| # by default, but they include all the locations we're likely to need. | ||
| args += ["-v", f"{path}:{path}"] | ||
| args += ["--workdir", str(Path.cwd())] | ||
| args += ["--entrypoint", flang_args[0], DOCKER_IMAGE, *flang_args[1:]] |
There was a problem hiding this comment.
This is quite the hack! I am quite impressed, but I do wonder what the potential for unintended consequences is here? Is it possible that linux does something to the filesystem that mac doesn't understand, or vice-versa? do we need to mount /private and /tmp? /private in particular contains a lot of stuff that is very OS-specific.
It also implicitly requires Docker, which might not be available on a Mac. I suppose I just wonder if we should just declare that the Fortran compilation shim is a Linux-only thing and leave it at that. Are people doing a lot of Android compilation on Mac in CI?
There was a problem hiding this comment.
do we need to mount
/privateand/tmp?/privatein particular contains a lot of stuff that is very OS-specific.
I've reduced this to only the home directory and the temporary directory, and verified that this works with SciPy.
Are people doing a lot of Android compilation on Mac in CI?
Not in CI, because none of the major CI services' Mac runners support running the Android emulator. But this comment suggests this might be improved in the future with the M3 processor. I hope that happens, because Mac is the only platform on which Android aarch64 wheels can be tested, since the emulator isn't available for Linux or Windows aarch64.
Outside of CI though, I compile for Android on Mac all the time, because it's my main development machine.
| setup_rust(config, python_dir, android_env) | ||
|
|
||
| # Create shims which install additional build tools on first use. | ||
| setup_fortran(android_env) |
There was a problem hiding this comment.
While it's a bit strange compared to other platforms, since android.py env does indeed install all the compilers, this seems consistent.
| meson_build_path.write_text( | ||
| meson_build_path.read_text() | ||
| .replace("'c'", "'c', 'fortran'") | ||
| .replace("'spam.c'", "'spam.c', 'fortran.f90', link_language: 'fortran'") | ||
| ) |
There was a problem hiding this comment.
It would be slightly better to add these customisations as parameters to new_meson_project(), populated into the meson.build file via jinja variables.
|
I expect we are about 24 hours from release - PyPy just got released, so just waiting on it in manylinux. |
Co-authored-by: Joe Rickerby <joerick@mac.com>
It will probably be needed for iOS, but I don't think anyone's tried to build a package that needs it yet. I encountered it when building SciPy, which links against On Pyodide a similar feature is already built into pyodide-build, which is where I got the idea from. It might be useful for Pyodide to support the cibuildwheel option as well, but it looks like that would require changes to pyodide-build.
I think I've dealt with all of your comments, so let's get this into 4.0 if possible. |
| } | ||
| for os_name, command in [ | ||
| ("linux", "auditwheel repair -w {dest_dir} {wheel}"), | ||
| ("macos", "delocate-wheel --require-archs {delocate_archs} -w {dest_dir} -v {wheel}"), |
There was a problem hiding this comment.
I think we have windows now too
| } | ||
| ], | ||
| "title": "CIBW_REPAIR_WHEEL_COMMAND", | ||
| "default": "auditwheel repair --ldpaths {ldpaths} -w {dest_dir} {wheel}" |
There was a problem hiding this comment.
Not sure what the point of the three way (should be four) setting was, only one make it into the schema?
There was a problem hiding this comment.
The schema has separate sections for each platform.
There was a problem hiding this comment.
Pull request overview
Improves Android cross-compilation in cibuildwheel so it can successfully build NumPy and packages that depend on it (e.g. via Meson/Fortran). The built-in libc++ patcher is replaced by full auditwheel-based repair, host build tools are installed per-build instead of as global cibuildwheel deps, and new hooks let Android cross-builds pull target-specific files (NumPy headers/static libs) and invoke a Fortran compiler.
Changes:
- Replace the limited Android repair logic with
auditwheel repair --ldpaths ...(requires upstream auditwheel PR), default API level bumped to 24, and update docs/tests accordingly. - Add a new
xbuild-filesoption that downloads target-platform copies of selected files from host-installed build requirements (defaults include NumPy), plus a new on-demand Fortran (flang-new) shim and pkgconf/PKG_CONFIG_RELOCATE_PATHSwiring. - Install
auditwheel,patchelf,pkgconf(andbuild) into the per-build virtualenv; drop the corresponding top-level dependencies frompyproject.toml.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cibuildwheel/platforms/android.py | Core: auditwheel-based repair, xbuild-files, Fortran shim, pkgconf env, default API 24, refactored helpers |
| cibuildwheel/options.py | Adds xbuild_files option parsing using wildcard kw arg names |
| cibuildwheel/util/helpers.py | parse_key_value_string supports "*" to allow arbitrary keys |
| cibuildwheel/resources/android/_cross_venv.py | Pre-imports ctypes to avoid post-monkeypatch import failure |
| cibuildwheel/resources/android/fortran_shim.py | New shim that downloads/configures Termux Flang, uses Docker on macOS |
| cibuildwheel/resources/defaults.toml | Adds default xbuild-files for NumPy and Android repair-wheel-command |
| cibuildwheel/resources/constraints*.txt / .in | Pin auditwheel, patchelf, pkgconf for build env |
| cibuildwheel/resources/cibuildwheel.schema.json | Schema entries for xbuild-files, Android repair default |
| bin/generate_schema.py | Generates schema entries and per-OS repair defaults |
| bin/run_example_ci_configs.py | Refactor for jinja2 dep / project construction |
| pyproject.toml | Drop patchelf/pyelftools/wheel runtime deps |
| .pre-commit-config.yaml | Drop pyelftools mypy dep |
| README.md / docs/*.md / docs/diagram.html | Document xbuild-files, new repair behavior, API 24 default |
| test/test_projects/setuptools.py | Move libraries = [] above setup_py_add so injections can append |
| test/test_projects/meson.py | Allow extra project/extension args |
| test/test_android.py | New tests: repair_none, repair_ldpaths, repair_error, meson, xbuild_files; updated api_level/libcxx/archs |
| test/utils.py | expected_wheels accepts android_api_level and uses ANDROID_API_LEVEL env default |
| unit_test/options_test.py | Tests for xbuild-files default/parse |
| unit_test/utils_test.py | Tests for parse_key_value_string wildcard support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
henryiii
left a comment
henryiii
left a comment
There was a problem hiding this comment.
This looks great. I think we missed a few things for Windows devlewheel, but that's not related to this PR.
Fixes for several issues found while building NumPy and related packages for Android:
Replace the limited built-in
repair-wheel-commandwith full auditwheel support.Add a shim which installs a Fortran compiler on demand, using a similar approach as we did for Rust.
Set up pkgconf in the build environment, and enable PKG_CONFIG_RELOCATE_PATHS. This is necessary for Meson, which uses pkgconf to find Python.
When a package builds against NumPy, replace platform-specific files with their Android equivalents, using a similar approach to pyodide-build.
This PR branch is temporarily used by several other PRs: see the cross-references below.