Skip to content
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

Add support for Mac Catalyst (on Intel and Apple Silicon chips) #73

Closed
wants to merge 1 commit into from

Conversation

jrose-signal
Copy link
Contributor

Builds on #60 to support compilation for Mac Catalyst targets as well (iOS-on-macOS). These are Tier 3 targets, but apart from working around a hardcoded version in the 'cc' crate everything does work.

Recommended to review with whitespace diffing disabled.

Comment on lines 200 to 203
// Work around hardcoded deployment target in cc crate by defining CMAKE_C_FLAGS
// instead of using the cflag builder.
boringssl_cmake.define("CMAKE_C_FLAGS", &compiler_flags);
boringssl_cmake.define("CMAKE_CXX_FLAGS", &compiler_flags);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/cc-rs#471 hardcoded the *-apple-ios-macabi targets to use a minimum OS version of "13.0", but the Clang provided in the latest Xcode only supports a minimum OS version of "13.1". rust-lang/cc-rs#678 discusses a possible fix for this, but it's not a clear win.

But we don't need to wait for them; by changing to -target x86_64-apple-ios-macabi (or aarch64-apple-ios-macabi), we let the compiler pick the earliest possible version, which is sufficient for compiling boring-ssl even if it isn't sufficient for all Rust use cases.

This isn't ideal because it means other cflags won't work, and it's not a complete solution because cargo test doesn't work (linking an executable still goes through the cc crate). But manually linking and running the tests shows them passing, and our experiments with boring-sys in signalapp/libsignal show the library working correctly when linked statically.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask you to add a user-friendly remark about that in crate's docs in boring/lib.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catalyst stuff seems to have gotten better in Rust 1.71, which is slightly ahead of the toolchain we're currently using, so I'll hang on to this a bit longer and maybe the workaround will become unnecessary.

This is a Tier 3 target, but apart from working around a hardcoded
version in the 'cc' crate everything does work.
@jrose-signal
Copy link
Contributor Author

Okay, between the improvements on the Rust side and Signal itself deciding not to pursue Catalyst at this time, I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants