Support XCFramework builds, Catalyst#18826
Conversation
|
|
||
| if target == "Catalyst": | ||
| buildcmd.append("-destination 'platform=macOS,arch=%s,variant=Mac Catalyst'" % arch) | ||
| buildcmd.append("-UseModernBuildSystem=YES") | ||
| buildcmd.append("SKIP_INSTALL=NO") | ||
| buildcmd.append("BUILD_LIBRARY_FOR_DISTRIBUTION=YES") | ||
| buildcmd.append("TARGETED_DEVICE_FAMILY=\"1,2\"") | ||
| buildcmd.append("SDKROOT=iphoneos") | ||
| buildcmd.append("SUPPORTS_MAC_CATALYST=YES") | ||
|
|
There was a problem hiding this comment.
Depending on the results of https://gitlab.kitware.com/cmake/cmake/-/issues/21436 this whole block might not be necessary. If it is, we could still probably trim this down to a more minimal set of overrides because I think some of these are already handled elsewhere in the toolchain.
| if target.lower().startswith("iphonesimulator"): | ||
| build_arch = check_output(["uname", "-m"]).decode('utf-8').rstrip() | ||
| if build_arch != arch: | ||
| print("build_arch (%s) != arch (%s)" % (build_arch, arch)) | ||
| cmakecmd.append("-DCMAKE_SYSTEM_PROCESSOR=" + arch) | ||
| cmakecmd.append("-DCMAKE_OSX_ARCHITECTURES=" + arch) | ||
| cmakecmd.append("-DCPU_BASELINE=DETECT") | ||
| cmakecmd.append("-DCMAKE_CROSSCOMPILING=ON") | ||
| cmakecmd.append("-DOPENCV_WORKAROUND_CMAKE_20989=ON") | ||
| if target.lower() == "catalyst": | ||
| build_arch = check_output(["uname", "-m"]).decode('utf-8').rstrip() | ||
| if build_arch != arch: | ||
| print("build_arch (%s) != arch (%s)" % (build_arch, arch)) | ||
| cmakecmd.append("-DCMAKE_SYSTEM_PROCESSOR=" + arch) | ||
| cmakecmd.append("-DCMAKE_OSX_ARCHITECTURES=" + arch) | ||
| cmakecmd.append("-DCPU_BASELINE=DETECT") | ||
| cmakecmd.append("-DCMAKE_CROSSCOMPILING=ON") | ||
| cmakecmd.append("-DOPENCV_WORKAROUND_CMAKE_20989=ON") | ||
| if target.lower() == "macosx": | ||
| build_arch = check_output(["uname", "-m"]).rstrip() | ||
| build_arch = check_output(["uname", "-m"]).decode('utf-8').rstrip() | ||
| if build_arch != arch: | ||
| print("build_arch (%s) != arch (%s)" % (build_arch, arch)) |
There was a problem hiding this comment.
I'm not sure if we ever determined if these were strictly necessary for these other platforms but, if they are, we might want to consider consolidating the additional arguments into a single block since they are shared among these 3 platforms.
283cdfe to
36c6f85
Compare
|
/cc @komakai |
|
@alalek We would like the xcframework build artifact eventually available as GitHub release assets (as seen here), as it's a pre-requisite for Swift Package Manager support. From an initial investigation it seems that we would want to submit a pull request to the opencv-infrastructure/opencv-master-config repo with some changes here:
Is there anything else that we'd need to modify to help support the process for GitHub release assets? Thanks! |
|
@chrisballinger Not sure that we should request to prepare patches in these scripts from you.
As alternative, it would be nice to setup Travis.org / "GitHub Actions" for that (or similar service with OSX support, like )
|
|
@alalek Ah interesting! From some initial research, it seems that GitHub Actions has a 6 hour job limit and 2 GB org-wide artifact limit and already supports Xcode 12.2 which is required for this PR's Apple Silicon support. Building the 10 architecture/platform combinations takes about 2-3 hours on my 2018 15" MBP, so it would probably be okay. We could also probably drop support for armv7s iOS (retaining armv7) and i386 simulator to speed things up a little and save some space. We will also probably want to create two CI jobs - one to create a static xcframework and one for a dynamic xcframework. It looks like there is a bug in Xcode/SPM that prevents the static xcframework from working properly (it tries to copy the framework into the app bundle and code sign it, which is invalid behavior for a static framework so it fails). So for SPM users we will need to provide a dynamic xcframework until that bug is fixed. I think that CocoaPods does properly support static xcframeworks so I am going to investigate that route as a workaround. |
|
Please take a look on "Custom Mac" builder error (OSX framework + tests): |
@alalek This looks like the error we were getting with Xcode 10.2.1 - have you upgraded the environment yet? |
|
This error message is from buildworker with AppleClang 11.0.0.11000033 (Xcode 11, |
|
@colejd @chrisballinger |
|
@colejd After fixing the framework alias problem for opencv binary(create alias for every framework header and binary, put them in the framework directory), I still get some error. |
|
@alalek @komakai This PR was designed to be built with Xcode 12.2 or higher for xcframework support, but we tried to ensure that the existing use cases would be backwards compatible. There are some additional fixes from my WIP SPM PR that we'll cherry-pick into this PR, which will probably resolve the issues people have noticed so far. @tianye2856 Please try out my SPM PR, it also allows building with |
If you pass in the C/CXX flags from the Python script, they won't be respected. By doing it in the actual toolchain, the options are respected and Catalyst successfully links.
Needed to specify EXE linker flags to get compiler tests to link to the Catalyst ABIs.
|
Now that this PR is passing tests and is feature-complete by our standards, I would say this is ready for another round of review. @alalek |
|
@alalek @komakai Since the initial PR, we've added/fixed the following:
We are starting our iteration on GitHub Actions support this week, but will keep that work isolated in another branch to keep this branch stable for review. |
|
@colejd Could you please apply (cherry-pick) commit with minor coding style changes from here: https://github.com/alalek/opencv/commits/pr18826_r |
|
@alalek Done! |
|
Hello Firstly, thank you. The build xcframework is almost perfect, however I did run into a issue as of master (today) with de38500 The intermediate framework targets all build correctly, however, the final step of the framework building has invalid arguments to Master as of this comment, produces the following output: Which if you run as a command:
Paying close attention, you'll notice we don't pass in our intermediate folders for say, iphonesimulator or the actual framework. The correct script invocation is :
Assuming Just a heads up. |
|
@vade Thanks for the report! It might be best to make a new issue for it, because it's a ticket that needs to be resolved. It used to work, but I think @alalek We should get the GitHub Actions PR that checks the XCFramework build merged soon (and enabled for this repo) in order to prevent breakage in the future: #18976 |
|
Sure, no worries. This seems almost like a one liner fix now that I'm looking at the Should prob be Considering this took a while to build for me, I can test this local change later and maybe submit a patch time permitting. I thought it prudent to just post and share my discoveries :) Thanks for the quick response! |
|
It used to be like this: So we just need to put that back, I think. I'll put in a PR. |
|
Don't you love it when you go do a |
|
I could see that build_xcframework.py doesn't support some arguments like --without to remove the unused modules. Will this be added in the future or I understood the script wrong ? |
|
@sivrish any parameters that you supply that aren't recognized by the script will be passed through to the respective iOS/OSX build scripts. You should be able to use the |
|
@colejd thanks for the clarification. |
|
@colejd I tried the script but it's adding an extra Command I tried -
Error -
The same args work on ios/build_framework.py
|
|
@sivrish does |
|
@sivrish Please make a new issue and tag me in it. The way the |
|
@colejd sure, |
…catalyst-xcframework Support XCFramework builds, Catalyst * Early work on xcframework support * Improve legibility * Somehow this works * Specify ABIs in a place where they won't get erased If you pass in the C/CXX flags from the Python script, they won't be respected. By doing it in the actual toolchain, the options are respected and Catalyst successfully links. * Clean up and push updates * Actually use Catalyst ABI Needed to specify EXE linker flags to get compiler tests to link to the Catalyst ABIs. * Clean up * Revert changes to common toolchain that don't matter * Try some things * Support Catalyst build in OSX scripts * Remove unnecessary iOS reference to AssetsLibrary framework * Getting closer * Try some things, port to Python 3 * Some additional fixes * Point Cmake Plist gen to osx directory for Catalyst targets * Remove dynamic lib references for Catalyst, copy iOS instead of macos * Add flag for building only specified archs, remove iOS catalyst refs * Add build-xcframework.sh * Update build-xcframework.sh * Add presumptive Apple Silicon support * Add arm64 iphonesimulator target * Fix xcframework build * Working on arm64 iOS simulator * Support 2.7 (replace run with check_output) * Correctly check output of uname_m against arch * Clean up * Use lipo for intermediate frameworks, add python script Remove unneeded __init__.py * Simplify python xcframework build script * Add --only-64-bit flag * Add --framework-name flag * Document * Commit to f-strings, improve console output * Add i386 to iphonesimulator platform in xcframework generator * Enable objc for non-Catalyst frameworks * Fix xcframework builder for paths with spaces * Use arch when specifying Catalyst build platform in build command * Fix incorrect settings for framework_name argparse configuration * Prefer underscores instead of hyphens in new flags * Move Catalyst flags to where they'll actually get used * Use --without=objc on Catalyst target for now * Remove get_or_create_folder and simplify logic * Remove unused import * Tighten up help text * Document * Move common functions into cv_build_utils * Improve documentation * Remove old build script * Add readme * Check for required CMake and Xcode versions * Clean up TODOs and re-enable `copy_samples()` Remove TODO Fixup * Add missing print_function import * Clarify CMake dependency documentation * Revert python2 change in gen_objc * Remove unnecessary builtins imports * Remove trailing whitespace * Avoid building Catalyst unless specified This makes Catalyst support a non-breaking change, though defaults should be specified when a breaking change is possible. * Prevent lipoing for the same archs on different platforms before build * Rename build-xcframework.py to build_xcframework.py * Check for duplicate archs more carefully * Prevent sample copying error when directory already exists This can happen when building multiple architectures for the same platform. * Simplify code for checking for default archs * Improve build_xcframework.py header text * Correctly resolve Python script paths * Parse only known args in ios/osx build_framework.py * Pass through uncaptured args in build_xcframework to osx/ios build * Fix typo * Fix typo * Fix unparameterized build path for intermediate frameworks * Fix dyanmic info.plist path for catalyst * Fix utf-8 Python 3 issue * Add dynamic flag to osx script * Rename platform to platforms, remove armv7s and i386 * Fix creation of dynamic framework on maccatalyst and macos * Update platforms/apple/readme.md * Add `macos_archs` flag and deprecate `archs` flag * Allow specification of archs when generating xcframework from terminal * Change xcframework platform argument names to match archs flag names * Remove platforms as a concept and shadow archs flags from ios/osx .py * Improve documentation * Fix building of objc module on Catalyst, excluding Swift * Clean up build folder logic a bit * Fix framework_name flag * Drop passthrough_args, use unknown_args instead * minor: coding style changes Co-authored-by: Chris Ballinger <cballinger@rightpoint.com>
Hello!
For this PR, @chrisballinger and I have added support for building OpenCV into an xcframework, enabling support on a variety of Apple devices with one build command. As part of this, we also introduced support for generating Catalyst-compatible frameworks through
platforms/osx/build_framework.py. As a side effect, this makes it easier to build for Apple Silicon / M1.By default, the generated xcframework supports the following platforms and architectures:
--iphoneos_archs): arm64, armv7--iphonesimulator_archs): x86_64, arm64--macos_archs): x86_64, arm64--catalyst_archs): x86_64, arm64Additionally, you can now use a new
--build_only_specified_archsflag on the iOS and OSX build scripts to build only directly specified archs. This prevents the scripts from falling back to a default set of architectures if none are specified. We leverage this to generate platform-specific frameworks that we later combine into an xcframework.This resolves #17929, and relates to #17291.
Testing
You can generate the framework by going into
platforms/appleand runningpython3 ./build_xcframework.py ./xcframework-build. The script requires that you have CMake 3.18.5+, Python 3.6+, MacOS 10.15+, and Xcode 12.2+, which are enforced by the build script.Caveats
CMake 3.18.5/3.19 or greater is required to build non-x86_64 iOS Simulator frameworks with CMake (discussion here), so we require this as a minimum version.
Support for the objc module on Catalyst is currently disabled, as there are currently incompatibilities with CMake (discussion here). We plan to follow up with a PR to re-enable this support when possible.The situation here has updated. See here for more info.The new
--catalyst_archsflag on the osx build script does not have any defaults, though every other archs-setting flag does. Since the scripts select a default set of architectures if none are specified directly, we need to default to None for Catalyst to avoid breaking any existing CI. When it's possible to make a breaking change, it'd be nice to have the behavior match the other archs-setting flags.Future Work
We plan to put in a PR for hooking into OpenCV's CI infrastructure to build/upload the generated xcframework, though we need some guidance on this.
We'd like to put in another PR that adds Swift Package Manager support using the binary xcframework as soon as it's available in a future tagged release of OpenCV. This would let users add OpenCV to their projects through Xcode very easily. (Update: We're working on this at Swift Package Manager [WIP] Rightpoint/opencv#4)
We'd also like to put in a PR to move the
iosandosxbuild scripts and supporting files into the newappleplatform directory, though this would be a breaking change for CI. We could do this now by moving the scripts and providing aliases.Pull Request Readiness Checklist
Patch to opencv_extra has the same branch name.