-
-
Notifications
You must be signed in to change notification settings - Fork 6
Add modifications to support compiling on iOS. #19
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
base: main-numpymeson
Are you sure you want to change the base?
Add modifications to support compiling on iOS. #19
Conversation
4175e29
to
495a012
Compare
There are some CI failures, are you sure this is passing upstream? Do you need to rebase off numpy |
Yes - it's passing CI upstream... the CI failures here are very odd - there's a bunch of linting errors in files I haven't touched; test failures that appear unrelated to changes that I've made; and packaging errors caused by workflows that have been deprecated. It's the end of my workday here; I can take a closer look in the morning. Does the main branch of this repo currently pass CI? It looks like it hasn't been run on |
No worries about the CI failures. Meson's CI config isn't set up to work well on repos other than github.com/mesonbuild/meson. We can ignore them. The way I've been going about testing PRs here is to integrate them in NumPy itself and ensure NumPy's CI is happy. |
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 @freakboy3742!
The BLAS-related changes here LGTM. I'll keep an eye on mesonbuild#14444, which is actively being reviewed right now. I think once that lands we can integrate this and update the bundled version in NumPy. The second bullet point sounds like the largest change, it'd be good to not diverge from upstream Meson there.
An update - it looks like mesonbuild#14444 has been closed in favor of mesonbuild#14481 by a Meson collaborator; that PR has all the same changes, but has a couple of other modifications that look like they could be useful for Rust compilation. |
Thanks for the update - subscribed to the new PR. It looks like that is moving and will land well before the next NumPy release (2.3.0, branch date is at least 6 weeks off), so there is time - but if it hasn't landed in a month and you need this for the next NumPy release, then I'm willing to merge this temporarily into our forked meson @freakboy3742, so do feel free to ask if needed. |
Looks like that new PR has been merged; but there's been an almost immediate follow up (mesonbuild#14486) to correct a bug in some of the extra changes. @rgommers Procedurally - how would you like to me to handle this? Do you want to manage the merge with upstream yourself, or would you like to update this PR to reflect the minimal set of changes needed for compatibility? |
Great!
If you could update this PR to the minimal set of changes needed, that'd be helpful. Upgrading to upstream |
All of iOS, tvOS, visionOS, watchOS use the XNU kernel. Report that and also make them return true for is_darwin() which is really more like "is_xnu()". Co-authored-by: Russell Keith-Magee <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
Apple linkers need to use different arguments on macOS and iOS-like platforms. Pass the system to the constructor so that it can be examined. Co-authored-by: Russell Keith-Magee <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
iOS should not use -undefined,dynamic_lookup; whereas macOS should revert from -dynamiclib to -bundle, as was the case before commit cfb5a48 ("linkers: darwin: do not use -bundle for shared_modules", 2025-03-24), because Postgres wants to use -bundle_loader and that is only possible together with -bundle. Co-authored-by: Russell Keith-Magee <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
``` $ LDFLAGS="-fuse-ld=lld" meson setup builddir/ [...] linker = lld_cls( compiler, for_machine, comp_class.LINKER_PREFIX, override, system=system, version=v) TypeError: LLVMDynamicLinker.__init__() got an unexpected keyword argument 'system' ``` Fixes regression in commit cece1a7. We pass system=system to the linker construction, which worked fine for Apple LLVM LD64 as that inherited `__init__` from DynamicLinker. But non-Apple LLD had an overridden `__init__` which wasn't adapted. Even if it is thrown away and never used, since LLVM isn't an Apple linker, we still need to obey the argument contract.
495a012
to
6fa5876
Compare
@rgommers I've just force-updated this branch to contain the 4 cherry picked commits from upstream (48fc3ec, cece1a7, 7a43b6e and 944456b) that implement the iOS changes, plus an additional commit that adds the blas/accelerate piece for iOS support. I've tested my numpy branch with this set of changes, plus mesonbuild/meson-python#731; with those changes, numpy builds. The wheels work in simple manual testing, but the test suite isn't running yet - iOS can't use subprocesses, which the numpy test suite seems to hinge on. I'm still working on that. |
I've been working on improving iOS support in the Python ecosystem. I'm currently working on porting NumPy as an example of a high profile, non-trivial Python package.
This PR makes a number of small changes to support building Numpy binary wheels for iOS:
ios
,tvos
andwatchos
system typesenv
argument; addingsystem
as a property of the linker seemed a lot less invasive.-undefined,dynamic_lookup
. The use of-undefined,dynamic_lookup
is listed as deprecated in the iOS compiler, and using this flag will raise a deprecation warning.-dynamiclib
when linking iOS modules, rather than-bundle
. iOS apps have very strict guidelines on what can be included;bundle
content is rejected by the App Store pre-validation process. The only non-static content allowed in an iOS app is a dynamically linked library, distributed as a Framework that is bundled with the app. This change has already been merged as part of linkers: darwin: do not use -bundle for shared_modules mesonbuild/meson#14340.With the exception of the
blas_lapack
identification mechanism, these changes have been submitted upstream to Meson as mesonbuild#14444.