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

AutomergeUniffi compile target for visionos and visionos-simulator #174

Merged
merged 14 commits into from
May 25, 2024

Conversation

cklokmose
Copy link
Contributor

Updated to the latest rust nightly and created a compile target for visionOS.

Copy link
Collaborator

@heckj heckj left a comment

Choose a reason for hiding this comment

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

I'd like to check with the community about the iOS support potential, and would like to know about Rust versions to align CI correctly with the nightly you choose.

@@ -1,10 +1,10 @@
// swift-tools-version:5.6
// swift-tools-version:5.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

The only possible downside here is that this constrains support to iOS 12 (per https://forums.developer.apple.com/forums/thread/734896), so earlier versions of iOS are dropped with this push forward. I don't think that'll be an issue, but we'll check with the community on Discord to be sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe 5.9 is needed to include visionos as a target.

Copy link
Collaborator

@heckj heckj May 24, 2024

Choose a reason for hiding this comment

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

The WASM build also crapped out with this, and dropping WASM support in favor of VisionOS isn't a good move, so I think we'll need to sort how how to enable that as well.. We'll need to dig into https://github.com/automerge/automerge-swift/blob/main/.github/workflows/ci.yaml#L43 and see if we can nail down a WASM toolchain for Swift 5.9 to continue that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! That's a good point.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I opened #175 to update the CI toolchains indepdent of this PR, so that if that goes smoothly, we can merge it and have this try again and see how CI fares. I didn't investigate any of the swift build failures though

@@ -64,7 +64,7 @@ if ProcessInfo.processInfo.environment["LOCAL_BUILD"] != nil {

let package = Package(
name: "Automerge",
platforms: [.iOS(.v13), .macOS(.v10_15)],
platforms: [.iOS(.v13), .macOS(.v10_15), .visionOS(.v1)],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is strictly needed, as this is the base version of the OS that the library will work with, and .v1 was the first version ever, but it also doesn't hurt anything.

@@ -32,7 +32,7 @@ XCFRAMEWORK_FOLDER="$THIS_SCRIPT_DIR/../${FRAMEWORK_NAME}.xcframework"
# (as of 10/10/23), but leaving it open to float seems less useful than
# moving the pinning forward, since Catalyst support (target macabi) still
# requires an active, nightly toolchain.
RUST_NIGHTLY="nightly-2023-10-09"
RUST_NIGHTLY="nightly-2024-05-23"
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a cooresponding version of Rust that's matched with this nightly toolchain that we want to make sure is aligned (https://github.com/automerge/automerge-swift/blob/main/.github/workflows/ci.yaml#L18, and a few other places). In CI, we're running Rust checks with 1.75.0 - does that need to be pushed forward to align with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For visionos the toolchain is compiled. We could also consider using the newest nightly only for the visionos target for now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the nightly you picked is after 1.78, but the core bits not yet nailed down into 1.79. I think pushing CI to use 1.78 would be the first steps, and we can keep using the nightly where it stands.

@heckj heckj merged commit a80ca33 into automerge:main May 25, 2024
4 checks passed
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