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 experimental device_and_simulator rule #507

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

brentleyjones
Copy link
Contributor

@brentleyjones brentleyjones commented Jun 3, 2022

Fixes #285, #438.

This rule is experimental because it might not be the right API to enable proper device support, but it's better to get something out there so we can iterate on the support.

Also updated the multiplatform example to use the device_and_simulator rule. Of note, the //examples/multiplatform/Tool target is not included in //examples/multiplatform:device_targets, as it would generate two configurations of the same binary (because the swift_binary doesn't have the rules_apple transition). It's things like this that make me feel this API isn't quite done yet.

@brentleyjones brentleyjones marked this pull request as draft June 3, 2022 19:59
Base automatically changed from bj/use-local_provisioning_profile to main June 3, 2022 20:02
@brentleyjones brentleyjones force-pushed the bj/device_and_simulator branch 3 times, most recently from c0798b2 to 3f1bf9f Compare June 3, 2022 20:11
@brentleyjones brentleyjones marked this pull request as ready for review June 3, 2022 20:11
@brentleyjones
Copy link
Contributor Author

Ideally we wouldn't need another rule for this, but when applying the split transition (which is needed in order to get both simulator and device builds in the same build graph) we can't see any details of the target the transition is being applied to. And we can't blindly apply the transition, because rules that don't have the rules_apple transition (mainly cc_binary and swift_binary) will get two different Starklark Transition hashes, but otherwise will be identical. This results in two versions of those binaries, and potentially all of their transitive dependencies.

Maybe there is a way, either in Starlark or generator, to detect these "no really different" configurations. Right now we could do that by seeing that the cpu portion of the bin directory didn't change, even though it has a different ST-HASH. I believe in the future that portion will go away in favor of only the Starlark Transition hash... but in the future maybe the hash won't change if the rule doesn't use those values, or we will find another way to detect that the configurations are effectively the same?

Either way, I think we should take this change as is, but think about ways to improve the UX of the API.

@brentleyjones
Copy link
Contributor Author

Ohh, also, if a rule doesn't have a provisioning_profile set for the device configuration, then rules_apple will fail analysis. That adds another wrinkle, but possibly one we could fix on the rules_apple side (maybe a build setting that we can transition to tell it not to validate the profile, since we aren't building).

@brentleyjones brentleyjones force-pushed the bj/device_and_simulator branch 3 times, most recently from a739882 to 188caa9 Compare June 6, 2022 13:09
This rule is experimental because it might not be the right API to enable proper device support, but it's better to get something out there so we can iterate on the support.

Also updated the multiplatform example to use the `device_and_simulator` rule. Of note, the `//examples/multiplatform/Tool` target is not included in `//examples/multiplatform:device_targets`, as it would generated two configurations of the same binary (because the `swift_binary` doesn't have the rules_apple transition). It's things like this that make me feel this API isn't quite done yet.
@brentleyjones brentleyjones merged commit ddc03dc into main Jun 6, 2022
@brentleyjones brentleyjones deleted the bj/device_and_simulator branch June 6, 2022 17:34
def device_and_simulator(*, name, **kwargs):
_device_and_simulator(
name = name,
testonly = True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this set to test only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it can depend on tests.

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.

Bug: Generated iOS projects only support iossimulator Feature Request: Support building for arm64 iOS
4 participants