-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Improve support for iOS targets. #14444
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
Conversation
ce18db9 to
725c1a2
Compare
|
Thanks for looking into this. :) The bundle thing was previously reported at #14240 and presumed fixed in #14340 by unconditionally switching to -dynamiclib, but that's very recent. Did you draft these changes before that was merged and then rebase, or did you intentionally start using bundle again on non-ios systems? I'm not overly familiar with the environment so I can't say which is the correct route to take :( but I daresay you know this quite well... On the testing front, it's possible to define a meson cross file, and the project tests runner can take that as an argument and run the full test suite in cross compile mode. We do that in CI for mingw/wine to test general cross compile support. I believe that as long as you can get the project tests running with your cross environment that should provide sufficient coverage. We already have python tests that test compiling simple modules and seeing if they successfully import and return the intended results, e.g. by running an emulator such as If I recall correctly the only way to target iOS/tvos/watchos is via cross compilation, yes? So if we want to add tests that's basically the only route for it. I assume that maybe could be set up as an extension to our existing macOS workflows? If it's easier / only practical to set it up on a dedicated CI runner which someone else volunteers to us then that's fine too, we are happy to accept offers. At the end of the day we can and will merge code that seems correct even if we can't test it in CI -- we support a number of proprietary C/C++/Fortran compilers that we don't test -- assuming the stakeholders tell us that they tested it and it works for their use cases. It just means that we can't guarantee it won't regress. |
|
CI activation of cross compilation tests, that we have today: meson/.github/workflows/nonnative.yml Line 42 in 95d7fac
meson/.github/workflows/os_comp.yml Lines 127 to 134 in 95d7fac
This uses the cross files here (most of them are just examples but a couple of them are actually used by the CI): https://github.com/mesonbuild/meson/tree/master/cross |
I've been working on porting NumPy off-and-on for a couple of weeks, so my Meson fork is indeed a couple of weeks out of date. Just my luck that I missed an update in the last couple of weeks that fixes that problem :-) I'll rebase and update the patch to reflect that change; regressing bundle definitely wasn't intentional.
I've got a patch for meson-python that creates a cross file; I'll look into running the test suite with a pre-canned version of that cross file in an iOS-compatible cross environment.
That is correct. There's no native "on device" compilation capability on any of the non-macOS Apple platforms.
There's no need for a dedicated machine; any macOS runner (x86 or ARM) should be able to do the job.
Understood. Obviously I'd prefer to be able to avoid regressions; but practicality beats purity etc. I'll see what I can work out on the testing front. |
725c1a2 to
f9a81a8
Compare
|
@eli-schwartz After a bit of investigation, I'm not sure testing iOS is going to be simple. From the look of it, the expectation of most of the tests is that they're going to produce a binary, and the "test" is to run the binary. That's going to be difficult to manufacture with iOS - even if you did compile an iOS-compatible binary, it needs to run in a simulator, and the process of running a app in a simulator isn't easy to coordinate outside of an Xcode project. So - in an iOS cross environment, the initial "Checking that testing works" test fails, because it can't run the test project, and I can't see any way to disable that test. If I manually comment out that one test, the 136 of the unit tests fail... and from the look of it, every one of the failures is because they can't run the test code. That does leave 14 skipped and 152 passed tests... but those test passes aren't really validating anything that isn't already validated on a macOS pass AFAICT. Have I missed something obvious here? Do I need to add some different mechanism for iOS testing? Or is this the point at which we hold our collective noses and accept we might not be able to test this? |
I had a feeling it would need to run in a simulator, but if doing that isn't easy to coordinate then oh well. You can add iOS specific tests to Lines 1107 to 1125 in 95d7fac
you can define a TestCategory for platform-ios. The test cases in that directory will be whatever you have handcrafted and think is sufficient to demonstrate in CI that support hasn't regressed. If that means avoiding test-running the results, then that is what we need to do... The tests could then be run using |
|
FWIW, doing this change for macos, not just iOS, broke postgres' build on macos, because we specify -bundle_loader, which is incompatible with -dynamiclib. So yes, I think it'd be better to restrict this to iOS. While I think there are folks building parts of postgres for iOS it's a vastly smaller number, and they might just be building the client libraries, which would not be affected. |
|
WRT
We had been fighting with that in postgres: |
This reverts commit d0d7af3. This patch should not have been backported but was added to the milestone by accident. It's a risky change. It also broke building postgres on macOS. See: #14340 (comment) And discussion of a better fix: #14444 (comment) #14444 (comment)
The problem is whether -bundle_loader may apply to iOS as well? Long term the solution could be to add a kwarg to choose between |
| def is_ios() -> bool: | ||
| return platform.system().lower() == 'ios' | ||
|
|
||
|
|
||
| def is_tvos() -> bool: | ||
| return platform.system().lower() == 'tvos' | ||
|
|
||
|
|
||
| def is_watchos() -> bool: | ||
| return platform.system().lower() == 'watchos' | ||
|
|
||
|
|
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.
Can you run Meson on these platforms, ie, can you use ios, tvos, and watchos to compile? If the answer is no then these can never be correct, since they will always point at the build machine.
Side note: I'd really like to get rid of these because they often end up creating bugs for cross compilation.
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.
You can't run Meson on iOS, as there's no compiler tooling that is run on device. However, you can set up a cross platform build environment that pretends to be iOS, and I've successfully used that approach to build binary Python wheels for Pillow, Numpy, and a handful of other packages. It might be possible to build a full iOS app with Meson, but I haven't tried. My interest here is entirely tied to meson-python and the use of Meson in the Python ecosystem to manage the compilation of binary wheels.
I haven't tried using Meson with tvOS and watchOS, but the situation there will be much the same (i.e., cross compilation environments). I added entries for those platforms here because there are already references to tvOS and watchOS in other sections of the code, so I figured I'd retain the symmetry.
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 we could rephrase the question.
universal.py contains some logic for determining what operating system the meson application is running on. This is sometimes important to know how to interact with the operating system, to take one example we use mesonlib.is_windows() to determine whether ninja files need to use cmd /c, and in programs.py we use it to determine whether to add hacks for Windows Store Python or otherwise how to handle parsing shebang lines on platforms where simply execing a script may not actually work.
These are obvious cases where it doesn't matter what the build machine or host machine is. It's not about what we're compiling, it's about how we're communicating with the operating system / filesystem / task scheduler. And that much is a constant, even if I'm running on Windows (!) and rigged a cross platform build environment that identifies itself as FreeBSD (!!!). I still need to communicate with cmd, run del instead of rm, and use funny backwards paths that have drive letters but aren't my automobile that I used to drive to New York and attend a FreeBSD convention. (This analogy may have run away from me a bit at some point.)
For other purposes, it's probably not very useful to ask what the operating system platform is, but rather ask what the build machine and host machine claim to be. And that includes, what does the build machine masquerade as via running in a simulator such as wine or the iOS simulator or qemu-user.
We currently don't clearly separate these cases. We have a lot of code that queries mesonlib, rather than querying an interpreter env object. We should probably convert some of it, rather than adding more.
So, I guess the question to ask is, are there situations where we'd want to write new code in meson which asks "am I running the meson application on an iPhone and it's important to distinguish this from a MacBook"? Or do we just want to write new code that knows "we are natively building an iOS artifact" / "we are cross compiling an iOS artifact"?
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.
And since we aren't currently using these new functions, it's easy to defer the decision to add them until some other time, when someone comes up with a good reason why they definitely need it.
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.
Ok - I think I understand the intention; there's definitely a gap here in terms of identifying cross-platform build environments and differentiating the build and host environment, but that's way outside my pay grade :-)
FWIW, meson won't ever be running on an iOS device (or, if it is, it would be mostly pointless, because there's no compiler or ability to call subprocess). The situation on Android is a little different, but
But - I've gone back and checked my original usage, and it turns out I'm not actually using these methods anyway - all the practical usage is checking the env object. On that basis, I'll strip these methods out.
|
|
||
| def get_std_shared_module_args(self, target: 'BuildTarget') -> T.List[str]: | ||
| return ['-dynamiclib'] + self._apply_prefix('-undefined,dynamic_lookup') | ||
| return ["-dynamiclib"] + self.get_allow_undefined_args() |
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 change from single to double quotes looks unrelated (and we generally prefer single quotes)
It's certainly possible - it's just slow (as in - a couple of minutes to start the simulator, per executable that you want to run), and a little complicated.
That does provide a way to remove a bunch of problematic tests (or, at least, reduce the test set). However, the suite fails before it even gets to that point - What I don't understand is how any cross platform build is passing this test suite. The |
That one I can definitely answer - no, it doesn't. iOS has very strict constraints on what dynamic content can be loaded; the only dynamically loadable content that passes App Store validation is a dynamic library, packaged as a framework. The error reported on #14240 is an example of how that manifests. However, macOS doesn't have that constraint - even when publishing to the macOS App Store. I'm guessing this is mostly a historical artefact - there's enough bundle-linked code in the wild that Apple can't walk back that setting. But iOS only gained the ability to load dynamic content at all with iOS 8, so it's a (relatively) recent change where they're being conservative in what functionality they're adding.
Looking into this deeper, I'm increasingly convinced that I agree with @anarazel's comment above that the solution added with #14340 is incorrect. The DynamicLinker interface provides a different API entry point for I'd argue that my original form of the patch was more correct: that on macOS, SharedModules should be compiled with As for the decision between choosing |
|
The cross environment needs an exe_wrapper in the cross file -- a simulator or emulator. The ubuntu-armhf cross tests don't define one (qemu-user would be the standard method of executing armhf binaries on x86-64).
|
Ah - that's what I was missing - Just my luck that I was trying to reverse engineer the one example case that didn't define the flag that would have helped :-) if I add Interestingly - it also needs a top-level Now I just need to write some actual tests for iOS... |
|
Closing in favor of #14481. |
I've been working on improving iOS support in the Python ecosystem; Meson is a common build system for building Python binary modules (for example, NumPy uses a Meson build system).
This PR makes a number of small changes to support building binary modules for iOS:
ios,tvosandwatchossystem typesenvargument; addingsystemas a property of the linker seemed a lot less invasive.-undefined,dynamic_lookup. The use of-undefined,dynamic_lookupis listed as deprecated in the iOS compiler, and using this flag will raise a deprecation warning.I haven't added any tests because there didn't appear to be any existing tests for iOS (or tvOS or watchOS) as platforms. I'm happy to add tests if they're required, but I'd appreciate any guidance on how those tests should be structured.