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

Produce an alias for an xcframework's ios_sim_arm64 slice if available #810

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

jszumski
Copy link
Collaborator

@jszumski jszumski commented Dec 8, 2023

If apple.arm64_simulator_use_device_deps is False, an xcframework providing slices for ios-arm64 and ios-arm64_x86_64-simulator wouldn't select the second slice for an ios_sim_arm64 build.

ERROR: SomeFramework/BUILD.bazel:11:15: configurable attribute "actual" in //SomeFramework:SomeFramework-import-SomeFramework.xcframeworkdefault_vfs doesn't match this configuration. Would a default condition help?

Conditions checked:
 //SomeFramework:SomeFramework-import-SomeFramework.xcframework-ios_arm64
 //SomeFramework:SomeFramework-import-SomeFramework.xcframework-ios_simulator_x86_64

The configuration being built had these values:

FragmentOptions com.google.devtools.build.lib.analysis.config.CoreOptions {
  cpu: ios_sim_arm64
  host_cpu: darwin_arm64
  ...
}

FragmentOptions com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions {
  apple_platform_type: ios
  apple_split_cpu: sim_arm64
  ios_multi_cpus: [sim_arm64]
  ...
}

This change emits a new condtion for //SomeFramework:SomeFramework-import-SomeFramework.xcframework-ios_simulator_arm64 that matches @build_bazel_rules_apple//apple:ios_sim_arm64 to the SomeFrameowrk.xcframework-ios-arm64_x86_64-simulator slice.

@mattrobmattrob
Copy link
Collaborator

This is a big QOL improvement! Most folks probably won't need the apple.arm64_simulator_use_device_deps feature at this point and it hasn't worked without it for me in the past. Awesome!

@jszumski jszumski marked this pull request as ready for review December 8, 2023 18:33
@@ -345,14 +345,13 @@ def _xcframework(*, library_name, name, slices):
elif arch == "arm64":
if platform_variant == "simulator":
arm64_simulator_slice = name

# Skip this - it's later defined
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jerrymarino you added this originally, do you have any background context or know what might break that isn't covered by the tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

There were some very odd permutations of xcframeworks we handle and can try to run this code against with CI, it's unlikely all possible permutations are tested here

AFAIK it seems to create an alias to this here

Copy link
Contributor

Choose a reason for hiding this comment

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

When turning on arm64_simulator_use_device_deps it gets you the option -

Copy link
Collaborator

@luispadron luispadron left a comment

Choose a reason for hiding this comment

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

Sweet one less flag to worry about

@mattrobmattrob
Copy link
Collaborator

Are you ready to merge this one, @jszumski?

@jszumski jszumski merged commit 883cc85 into master Jan 3, 2024
9 checks passed
@jszumski jszumski deleted the jszumski/xcframeworks-pick-sim_arm64-slice branch January 3, 2024 18:37
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.

4 participants