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

Address issues with new Catalyst/M1 simulator targets #614

Merged
merged 3 commits into from
Aug 30, 2021

Conversation

eranrund
Copy link
Contributor

Hello!

With the introduction of the new M1 ARM computers, the target aarch64-apple-ios became somewhat ambiguous.
Before the ARM laptops, building for an ARM iOS target would indicate real hardware (e.g. iPhone), since simulators were only for x86 targets. However, when the M1 got introduced, there is now an ARM iOS simulator, that is different from real hardware. To handle this, there now exists a new build target aarch64-apple-ios-sim, that indicates simulator build.

This PR adds support for that, allowing one to build for aarch64-apple-ios-sim and having that result in the correct compiler flags (-mios-simulator-version-min).

Additionally, it adds support for addressing an issue I ran into in #556 (comment)
Similarly to ios-sim targets, the new Catalyst macabi targets benefits from having the ability to specify the SDK version.

Thanks!

@eranrund eranrund changed the title Ios sim Address issues with new Catalyst/M1 simulator targets Aug 19, 2021
src/lib.rs Outdated
let ios = if arch == "arm64" { "ios" } else { "ios13.0" };
cmd.args
.push(format!("--target={}-apple-{}-macabi", arch, ios).into());
let deployment_target = env::var("CATALYST_DEPLOYMENT_TARGET")
Copy link
Member

Choose a reason for hiding this comment

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

Is this an environment variables that other projects are using? Or is this invented for just cc here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately no, this was invented just for this use case. I tried finding out if there is a more standard solution but came up empty. I'm open to other suggestions, but I do think we should have a way of building with --target-arm64-apple-ios13.0-macabi.

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, in that case is it ok to avoid an env var? Perhaps some sort of configuration option on Build? I don't know anything about these targets or what the version means, so I don't know myself how best to design 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.

I'm not sure there's a good way to avoid it. A configuration option on Build is problematic, since this needs to be controlled externally by whoever is invoking cargo and specifying which target they're building for - and some projects such as ours target multiple Apple targets. Additionally, this would have to go inside third-party crate build scripts, so would require a lot of forking. I figured an environment variable is unobtrusive, and at least in our case has allowed the build to work for the numerous Apple targets we are building for.

Unfortunately I couldn't figure out what the difference between arm64-apple-ios13.0-macabi and -arm64-apple-ios-macabi is. Perhaps @sercand who originally wrote this patch could shine some light on it.

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, yeah if this is a per-build thing which would require you to fork dependencies then a Build option is a no-go. I think then our options are either making the env var (like you have here) or to always use the un-versioned target.

If we don't get input from @sercand and we don't otherwise know of downsides, I'd prefer to go with the un-versioned target unconditionally if we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually the opposite, we need the versioned target (ios13.0 when building for arm64).

Copy link
Member

Choose a reason for hiding this comment

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

Oh! Sorry I am... quite unfamiliar with all this...

But in any case, if we don't know of a downside of using the versioned target, I think this should use the versioned target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, this is mostly new to me as well :)
As far as I can tell the versioned one is fine, but I am still curious as to why that logic was placed there.

Copy link
Member

Choose a reason for hiding this comment

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

Ok haven't heard back in awhile, so wanna switch to just using versioned targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, just pushed a change for that.

@alexcrichton alexcrichton mentioned this pull request Aug 30, 2021
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