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

Added tvos-sim support #951

Merged
merged 8 commits into from
Feb 20, 2024
Merged

Conversation

ErikEverson
Copy link
Contributor

This PR allows using cc-rs lib to target tier 3 *-apple builds simulator builds

  • The aarch64-apple-tvos-sim target was added so both tvos-sim and x86 versions are simulator versions.
  • I removed the target.contains("aarch64-apple-tvos") as it seems to be redundant and iOS and watchOS implementations do not include it. The build was not effected with this removal.
  • Added a new check-tvos-sim step to actions

how was this tested:

  • cargo test -Z build-std=std --no-run --workspace --target aarch64-apple-tvos{and -sim}
  • cargo test -Z build-std=std --no-run --workspace --target aarch64-apple-tvos{and -sim} --release
  • cargo test -Z build-std=std --no-run --workspace --target aarch64-apple-tvos{and -sim} --features parallel

@ErikEverson
Copy link
Contributor Author

CC: @phatblat for eventual visionOS support additions

@NobodyXu NobodyXu mentioned this pull request Feb 17, 2024
src/lib.rs Outdated
Comment on lines 1962 to 1964
cmd.args.push(
format!("--target={}-apple-tvos{}", arch, deployment_target).into(),
);
Copy link
Contributor

@BlackHoleFox BlackHoleFox Feb 17, 2024

Choose a reason for hiding this comment

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

This change seems to misalign with XCode at least. When building Objective-C with clang via XCode it passes the -target flag to convey the deployment target: -target arm64-apple-tvos16.0 instead of the os-version-min flag we pass (which doesn't seem documented for tvOS/watchOS? ignoring that though...)

Did you try building any libs with this just to make sure? I didn't see a problem getting the deployment target into the final Mach-o executable at least.

Copy link
Contributor Author

@ErikEverson ErikEverson Feb 20, 2024

Choose a reason for hiding this comment

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

That is a good point. I will add back this code as everything worked fine either way and that will align it with XCode's behavior.
As for libraries that I built this on there are two. Blake3 and Ring. Both are dependencies of the project I am working on Ditto which is included in my iPad/tvOS application/s.

the produced binary Mach-O Platform type was verified with otool.

@@ -113,6 +113,24 @@ jobs:
- run: cargo test -Z build-std=std --no-run --workspace --target aarch64-apple-tvos --release
- run: cargo test -Z build-std=std --no-run --workspace --target aarch64-apple-tvos --features parallel

# This is separate from the matrix above because there is no prebuilt rust-std component for the target.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create a new matrix to combine this with the aarch64-apple-tvos test above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Be-ing I updated to have a matrix for all -Z build-std targets. The only ones right now are tvOS targets but any tier 3 targets would fall in this same bucket as well.

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

@NobodyXu NobodyXu merged commit e814ca5 into rust-lang:main Feb 20, 2024
21 checks passed
@ErikEverson ErikEverson deleted the EE/tvOS_Support branch February 20, 2024 23:01
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