-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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-114099: Add configure and Makefile targets to support iOS compilation. #115390
Conversation
I don't have access to resolve conversations in this repository, so I've marked them with a thumbs up instead. The only outstanding one, which we need guidance from the core team on, is whether the iOS files should go in a top-level directory as this PR currently does, or a subdirectory of |
I personally would prefer one top-level directory, like the current |
That's good enough for me. I've modified the docs to use a folder inside iOS. |
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.
Thanks to @mhsmith for the thorough reviews. Waiting thumbs-up from @ned-deily before landing.
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.
Overall, this is a very nice PR and a pleasure to review. I don't have time left tonight to actually test it; I will do so tomorrow. But I do have a few, relatively minor review comments here. Let me know what you think.
iOS/Resources/bin/arm64-apple-ios-ar
Outdated
@@ -0,0 +1,2 @@ | |||
#!/bin/bash | |||
xcrun --sdk iphoneos ar $@ |
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.
The use of these stub scripts for executables is a clever solution. As they stand, I think there is one potential shortcoming if I understand the xcrun
documentation correctly (I haven't yet verified this): according to the man page, specifying --sdk
to xcrun
overrides any SDKROOT
environment variable, something which I've had occasion to use when building and testing for multiple macOS versions. It's probably less of a deal on iOS but I think it would be nice to allow for this. If the behavior is as documented, perhaps the stub scripts could do a test for the presence of an SDKROOT
env variable and, if defined, not specify --sdk iphoneos
?
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.
That's an easy enough change to make; however, it does open a potential class of bugs.
As currently defined, arm64-apple-ios-clang
will always be a clang install for ARM64 iOS devices. If we allow for SDKROOT
as you describe, a user could easily think they're compiling an iOS build, but actually be using an iOS simulator SDK (or worse - a macOS or watchOS SDK).
This is especially problematic when the values are embedded in sysconfig
, as the CC
value embedded in sysconfig will become dependent on the end-user's environment.
You can still switch between different Xcode installs with xcode-select
for testing purposes; I'd argue this is likely more useful for testing purposes, as Xcode leans heavily to having a single iOS SDK per Xcode install.
My inclination is that being explicit with --sdk
in these scripts and overriding SDKROOT
is preferable on balance, but I'll defer to your judgement if you feel strongly otherwise. If we do make this change, I'll add some docs clarifying how SDKROOT is interpreted, and clarifying that it's the user's responsibility to make sure SDKROOT matches their intended compilation target.
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 see two drawbacks with relying solely on xcode-select
. One, xcode-select -s
has effect system-wide, and so would affect all other uses and users of Xcode on that system. Of course, there is another way to effect this on a process basis: set a DEVELOPER_DIR
env variable. But, two, even with that, AFAICT there still would be no way to select a non-default iOS SDK within a particular Xcode instance and, while it may not be the default case at the moment, it would be better to not preclude supporting more than one SDK available in an Xcode instance. I think the primary value of supporting all this would be during development and testing of Python iOS frameworks themselves and, as such, you likely wouldn't want to use them in an actual release of a Python iOS framework but there might be cases where you would. (And, the end-user of the framework, presumably someone developing an iOS app, would need to ensure they had a compatible developer environment anyway.)
So perhaps a slightly simpler way to address these concerns would be to define a new env variable that would allow overriding the SDK name in the stubs, something like IOSSDK='iphoneos17.1'
(I'm not hung up on the name) instead of the default iphoneos
?, and mentioning setting DEVELOPER_DIR
to use a non-default Xcode installation:
xcrun --sdk ${IOSSDK:-iphoneos} clang -target arm64-apple-ios $@
Or we could just defer this until a demonstrated need arises but I think it is worth considering.
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.
Passing in a specific SDK (or even better, a specific SDK version) with a custom environment variable makes sense to me. I'll push an update shortly that uses IOS_SDK_VERSION
so you can get iphoneosX.Y
and iphonesimulatorX.Y
from a single environment variable (since the base names of the SDKs are stable, and I can't think of a use case where you'd want a different version for the device vs simulator SDK in a single build), along with docs covering the new variable and DEVELOPER_DIR
@@ -0,0 +1,2 @@ | |||
#!/bin/bash |
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.
Sorry, I didn't notice these earlier: all of the stubs should just use a shebang line of /bin/sh
rather than bash
since bash
is officially deprecated on macOS, and there are no bash-isms used.
@ned-deily I've think found the issue with parallel builds - turns out the issue was iOS specific in this case. I've also addressed the |
For Android the parallel build issue was fixed in #115780, which I see you've already found. |
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.
Almost there! See the two comments in the review. Otherwise, everything looked good! I didn't do an exhaustive build test but I think I covered the most interesting cases.
FWIW, I'm attaching a script I used to test builds. It does not attempt to cover everything nor build working libraries (for example, no third-party libs) but it does go through the motions of building both a macOS build Python and then building all three supported iOS archs plus a rudimentary test of using DESTDIR
. I ran it successfully on macOS 14.3.1 with Xcode 15.2 on both Apple Silicon and Intel Macs. I also successfully tested building using an Xcode pre-release and explicit SDK:
export DEVELOPER_DIR=/Applications/Xcode-beta.app
export IOS_SDK_VERSION=17.4
cd /path/to/srcdir
./build-python-ios-frameworks.sh
I'm attaching the script here just for future reference rather than suggesting including it in the PR. Perhaps it might be useful as a start for a buildbot or CI step.
build-python-ios-frameworks.sh
Makefile.pre.in
Outdated
rm -rf $(DESTDIR)$(PYTHONFRAMEWORKPREFIX)/include; \ | ||
fi | ||
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$(PYTHONFRAMEWORKINSTALLDIR) | ||
sed 's/%VERSION%/'"`$(RUNSHARED) $(PYTHON_FOR_BUILD) -c 'import platform; print(platform.python_version())'`"'/g' < $(srcdir)/$(RESSRCDIR)/Info.plist > $(DESTDIR)$(PYTHONFRAMEWORKINSTALLDIR)/Info.plist |
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.
Because the Info.plist
is marked as a config file in configure.ac
(AC_CONFIG_FILES([iOS/Resources/Info.plist]
), autoconf
ensures that configure
copies that file into the build directory, so the change to add $(srcdir)
here is not needed here (and causes an error) unlike the $(srcdir)
below for the stub scripts which are not config files which does now work correctly (thanks!).
iOS/README.rst
Outdated
Python, you can compile a python interpreter and then use that interpreter to | ||
run Python code. However, the binaries produced for iOS won't run on macOS, so | ||
you need to provide an external Python interpreter. This interpreter must be | ||
the *exact* same version as the Python that is being compiled. |
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.
One further thought here: perhaps we should be a bit more exact about what is meant by the *exact* same version
. A builder could rightfully question whether that means that the build Python has to be built from the exact same source checkout as the iOS version(s) being cross-built. Or is a build Python from the same branch OK, e.g. a Python 3.13.x? This is more of an issue once 3.13, say, is released and we promise things like ABI compatibility, etc.
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.
Once a release is finalised, it's generally safe to use any 3.X build as a host Python for a cross platform build of 3.X.Y. However, there were at least 2 commits in January (both threading related) that caused an issue with iOS cross-platform builds of 3.13 if the host Python wasn't the same commit hash. That's easy enough to put down to "main branch is a moving target"; but I believe this could manifest with 3.X.Y as a host for 3.X.Y+1 because there's no guarantee that the bytecode magic number will remain consistent in a 3.X series.
So - the safest answer here is "the same commit hash"; but there's often a bit of leeway. I'll clarify the language here to a "hope for the best, expect the worst" kind of framing.
@ned-deily Updated with a README clarification and |
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.
Looks good to me. Thanks again for the attention to detail. If any other comments come up, they can be resolved in a follow-on PR. Time to move on to the next phase.
I've already put together a more production-ready script for Android, based on the existing WASI build script from Tools/wasm/wasi.py, so I suggest iOS follows the same pattern. The current version is here, and I'll submit it in a PR as soon as the lower-level Android build fixes have been merged. |
This change is part of the work on PEP-738: Adding Android as a supported platform. * Remove the "1.0" suffix from libpython's filename on Android, which would prevent Gradle from packaging it into an app. * Simplify the build command in the Makefile so that libpython always gets given an SONAME with the `-Wl-h` argument, even if the SONAME is identical to the actual filename. * Disable a number of functions on Android which can be compiled and linked against, but always fail at runtime. As a result, the native _multiprocessing module is no longer built for Android. * gh-115390 (bee7bb3) added some pre-determined results to the configure script for things that can't be autodetected when cross-compiling; this change adds Android to these where appropriate. * Add a couple more pre-determined results for Android, and making them cover iOS as well. This means the --enable-ipv6 configure option will no longer be required on either platform.
This change is part of the work on PEP-738: Adding Android as a supported platform. * Remove the "1.0" suffix from libpython's filename on Android, which would prevent Gradle from packaging it into an app. * Simplify the build command in the Makefile so that libpython always gets given an SONAME with the `-Wl-h` argument, even if the SONAME is identical to the actual filename. * Disable a number of functions on Android which can be compiled and linked against, but always fail at runtime. As a result, the native _multiprocessing module is no longer built for Android. * pythongh-115390 (bee7bb3) added some pre-determined results to the configure script for things that can't be autodetected when cross-compiling; this change adds Android to these where appropriate. * Add a couple more pre-determined results for Android, and making them cover iOS as well. This means the --enable-ipv6 configure option will no longer be required on either platform.
This change is part of the work on PEP-738: Adding Android as a supported platform. * Remove the "1.0" suffix from libpython's filename on Android, which would prevent Gradle from packaging it into an app. * Simplify the build command in the Makefile so that libpython always gets given an SONAME with the `-Wl-h` argument, even if the SONAME is identical to the actual filename. * Disable a number of functions on Android which can be compiled and linked against, but always fail at runtime. As a result, the native _multiprocessing module is no longer built for Android. * pythongh-115390 (bee7bb3) added some pre-determined results to the configure script for things that can't be autodetected when cross-compiling; this change adds Android to these where appropriate. * Add a couple more pre-determined results for Android, and making them cover iOS as well. This means the --enable-ipv6 configure option will no longer be required on either platform.
This change is part of the work on PEP-738: Adding Android as a supported platform. * Remove the "1.0" suffix from libpython's filename on Android, which would prevent Gradle from packaging it into an app. * Simplify the build command in the Makefile so that libpython always gets given an SONAME with the `-Wl-h` argument, even if the SONAME is identical to the actual filename. * Disable a number of functions on Android which can be compiled and linked against, but always fail at runtime. As a result, the native _multiprocessing module is no longer built for Android. * pythongh-115390 (bee7bb3) added some pre-determined results to the configure script for things that can't be autodetected when cross-compiling; this change adds Android to these where appropriate. * Add a couple more pre-determined results for Android, and making them cover iOS as well. This means the --enable-ipv6 configure option will no longer be required on either platform.
… iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
…iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
…iOS compilation. (pythonGH-115390)
… iOS compilation. (pythonGH-115390)
Part of the PEP 730 work to add iOS support.
This PR adds the iOS targets to configure and Makefile to support compiling an iOS framework. It does not include the test harness and related targets that was included in #115063 - that will come in a follow up PR.
This PR slightly contains one deviation from PEP 730, as it uses the
<cpu>-<abi>
format for PLATFORM_TRIPLE and _PYTHON_HOST_PLATFORM, rather than<abi>-<cpu>
(i.e., it usesarm64-iphoneos
, notiphoneos-arm64
). This was identified during the review of #115120 as an inconsistency with the ordering of existing platform triples. This is an almost entirely cosmetic change with no compatibility considerations, as the only impact is the final naming scheme ofsysconfig
modules and thesys.implementation._multiarch
attribute.As of this PR, it is now possible to compile an iOS framework; but the framework still isn't usable at runtime.
Summary of changes
config.sub
to include 2 modifications required to support iOS/tvOS/watchOS builds (supporting the arm64_32 CPU identifier, and the-simulator
suffix). These were submitted upstream as part of a patch in August last year, but that patch was only partially applied. The autoconf contribution process is opaque, and no feedback was given on why the other parts were rejected (there wasn't even feedback that they'd been partially accepted - I discovered that by accident). The extra pieces have been re-submitted upstream.x86_64
buildbot is proving difficult because of hardware availability, so I've omitted that target from Tier 3. This possibility was flagged in PEP 730.ac_cv_file__dev_ptmx
andac_cv_file__dev_ptc
, which are usually mandatory command line arguments for cross-platform builds. iOS is a reliable cross-build target, so we know these features aren't available.pyconfig.h
file that can be used to switch between CPU-dependent pyconfig.h files.Differences between macOS and iOS Frameworks
iOS frameworks are slightly different to macOS frameworks.
Info.plist
is slightly different, and must be in the root of the framework, not a Resources subfolder.@rpath
, rather than an absolute path, as the install path for the framework will be app-specific..dylib
artefacts and headers; and the headers won't be copied into the final distribution binary. As a result, the standard library and support binaries cannot be distributed inside the framework - they must be distributed parallel to the Framework folder.To accomodate these difference, the
make install
target for iOS will generate a file structure like the following:(
--enable-framework
location)bin
include
symlink to ->Python.framework/Headers
lib
python3.X
Python.framework
Headers
Python
(the dylib binary)Info.plist
The task of copying the standard library into an appropriate location in the final app bundle is left as a task for the developer using the framework.
Stub binaries
Xcode doesn't expose explicit compilers for iOS/tvOS/watchOS; instead, it uses an
xcrun
script that resolves to a full compiler path (e.g.,xcrun --sdk iphoneos clang
to get the clang for an iPhone device). However, using this script poses 2 problems:xcrun
includes paths that are then machine specific, resulting in asysconfig
module that cannot be shared between usersCC
/CPP
/LD
/AR
definitions that include spaces. There is a lot of C ecosystem tooling (including distutils and setuptools) that assumes that you can split a command line at the first space to get the path to the compiler executable; this isn't the case when usingxcrun
.To avoid these problems, the
iOS/Resources/bin
folder includes shell script wrappers around the tools needed by configure, named using the scheme that autoconf expects by default. These scripts are relocatable, and will always resolve to the appropriate local system paths. By including these scripts in the "installed" bin folder, the contents of the sysconfig module becomes useful for end-users to compile their own modules.