-
Notifications
You must be signed in to change notification settings - Fork 81
docs: mention Android support #1142
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
Changes from 1 commit
66386b1
869c9c1
cbc124d
f065aa0
9469469
4701585
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,7 +52,7 @@ correct suffix. These values are set by cibuildwheel when cross-compiling. | |
|
|
||
| It should be possible to cross-compile to Linux, but due to the challenges of | ||
| getting the manylinux RHEL devtoolkit compilers, this is currently a TODO. See | ||
| `py-build-cmake <https://tttapa.github.io/py-build-cmake/Cross-compilation.html>`\_ | ||
| [py-build-cmake](https://tttapa.github.io/py-build-cmake/Cross-compilation.html) | ||
| for an alternative package's usage of toolchain files. | ||
|
|
||
| ### Intel to Emscripten (Pyodide) | ||
|
|
@@ -64,3 +64,16 @@ by setting `_PYTHON_SYSCONFIGDATA_NAME`. This causes values like `SOABI` and | |
| This is unfortunately incorrectly stripped from the cmake wrapper pyodide uses, | ||
| so FindPython will report the wrong values, but pyodide-build will rename the | ||
| .so's afterwards. | ||
|
|
||
| ## Android | ||
|
|
||
| To build for Android, you'll need the following items, all of which will be | ||
| provided automatically if you use cibuildwheel: | ||
|
|
||
| - An Android | ||
| [toolchain file](https://cmake.org/cmake/help/latest/variable/CMAKE_TOOLCHAIN_FILE.html) | ||
| which adds the location of the Python headers and libraries to | ||
| `CMAKE_FIND_ROOT_PATH`. | ||
|
||
| - Compiler paths and flags, either in the toolchain file or in environment | ||
| variables. | ||
| - A Python environment which simulates Android. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -254,11 +254,13 @@ def configure( | |
| cache_config[f"{prefix}_ROOT_DIR"] = Path(sys.prefix) | ||
| cache_config[f"{prefix}_INCLUDE_DIR"] = python_include_dir | ||
| cache_config[f"{prefix}_FIND_REGISTRY"] = "NEVER" | ||
| # FindPython may break if this is set - only useful on Windows | ||
| if python_library and sysconfig.get_platform().startswith("win"): | ||
| cache_config[f"{prefix}_LIBRARY"] = python_library | ||
| if python_sabi_library and sysconfig.get_platform().startswith("win"): | ||
| cache_config[f"{prefix}_SABI_LIBRARY"] = python_sabi_library | ||
| # Setting the library location is only needed on some platforms - on | ||
| # other platforms it may break FindPython. | ||
| if sysconfig.get_platform().startswith(("win", "android")): | ||
| if python_library: | ||
| cache_config[f"{prefix}_LIBRARY"] = python_library | ||
| if python_sabi_library: | ||
| cache_config[f"{prefix}_SABI_LIBRARY"] = python_sabi_library | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part needs some discussion. I think this would point these to an unwanted location. Could really use a more concrete example of how to cross-compile to see how these are expanded. #1050 is also relevant for these.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what we already have, though, just with android added alongside windows?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I don't believe it is needed or that it has the intended effect when cross-compiling. E.g. are we passing the library of the host or target environment there? A more concrete example of how a cross-compiled android project would help a lot, as well as how each environment does it
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't pass it, then it is required to include Though, to your point, since
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is specific to windows though right? Because on linux it would use the bash file (forgot the name of it) that contains the configure values used when compiling (dunno why they don't provide something similar to windows though).
Yes that would be preferred imo with relevant documentation
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not needed on other platforms, only Windows and Android care. So I'm not really sure, as it's not used.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've now reverted this change, as it was unnecessary with the approach explained in my comment below. |
||
| if numpy_include_dir: | ||
| cache_config[f"{prefix}_NumPy_INCLUDE_DIR"] = numpy_include_dir | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping interpreter can be good and bad. Iirc cross compilation with
cross-envbehaves differently with and without it, but I don't remember which one is more preferred.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think CMake 4.1 kind of forces the no Interpreter case. From what I understand, you can set a emulated Python for it, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, since we don't correctly find and pass the Python library, adding the Interpreter components helps on Windows. If we could reliably find the Python library, then we don't need the Interpreter component. For some reason CMake is better than us at finding Python. I'd like a more principled approach than #1093's seeming "try everything" method, but we likely need something to get rid of the Interpreter component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think the user should provide the hints themselves when cross-compiling in the toolchain file, and we just document that. There are too many edge-cases we would have to account for, which otherwise could be better handled by
SYSROOTand suchThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm referring to when not cross compiling - we currently should compute this unless the user specifies something different, but instead we usually don't find it and rely on CMake to find the Python library using the interpreter. Yes, when cross-compiling, it has to be specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now reverted all the
Interpreterchanges as explained in my comment below.