-
Notifications
You must be signed in to change notification settings - Fork 34
Command for support of macOS/iOS universal binaries #197
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
Are you sure you want to change the base?
Conversation
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 for your contribution @gmeeker
Unfortunately, the current approach wouldn't be valid, because of the re-entrancy issue commented below.
I have been thinking of other possible approaches, but at the moment I don't see anything that could be reasonably done without actively modifying recipes and/or build systems integrations that would provide some more built-in approach, because this kind of hacks from the consumer side seem a bit too much of a long shot.
for arch in archs: | ||
ConanOutput().success(f"Building universal {ref} {arch}") | ||
arch_folder = os.path.join(conanfile.build_folder, arch) | ||
run(f'conan install --requires={ref}{more_args} -d direct_deploy --deployer-folder "{arch_folder}" -s arch={arch} -b missing', shell=True) |
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.
This is making Conan re-entrant, and it is not supported, see https://docs.conan.io/2/knowledge/guidelines.html -> Forbidden practices.
Conan is not re-entrant: Calling the Conan process from Conan itself cannot be done. That includes calling Conan from recipe code, hooks, plugins, and basically every code that already executes when Conan is called. Doing it will result in undefined behavior.
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 for the feedback. I suspected that was a bad idea.
I've updated with a different approach that loads and builds the graph several times. This is a bit crude, for example, it makes it hard for some recipe to only get built once as universal with CMake.
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.
Well, this isn't quite resolved. It can fail due to skipped binaries but I'll give it some thought.
I realize that with Apple announcing end of support for Intel Macs in 2027, there is probably not a lot of reason to touch Conan's core code. However, I think something like this command (despite its use of Conan internals) could bridge that gap. We've been using a wrapper script that deploys Conan's binaries, runs lipo, and patches the generator files in the top level (executable) recipe's folder. Other users seem to avoid that approach and just lipo two single architecture apps together. |
I've rewritten this command to identify the out of date universal packages and then build the single architectures in-process. It seems to work, even if it reliably heavily on Conan's internals. |
…ely building more than necessary
This is an attempt to build on the CMake support for '-s arch=armv8|x86_64' by pretending that all generates can produce universal binaries. When called to package a recipes as universal binaries, it bypasses the build() / package() and runs lipo with the single architecture binaries instead.
This currently duplicate the code of the install command and calls a lot of non-public API. It also executes install with direct_deploy for each architecture. This could be more efficient, both by avoiding the copy and moving this directly into Conan's dependency graph. However, it seems to work.
Note that this currently builds single architectures even for CMake recipes. There is no guarantee that all CMake recipes support this (for example they may detect the architecture to use ARM or x86 assembly). Recipes would probably need a way to opt-in for universal CMake builds.