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

Fix the Swift target on arm64 simulator. #32

Merged
merged 1 commit into from
May 17, 2021

Conversation

xianwen
Copy link
Collaborator

@xianwen xianwen commented May 15, 2021

Fix the Swift target on arm64 simulator.

@shepting @qyang-nj

@xianwen xianwen force-pushed the xianwen--swift-m1-simulator-sdk branch from 04e06ac to 16ab002 Compare May 15, 2021 01:33
@xianwen xianwen marked this pull request as ready for review May 15, 2021 01:34
// For arm64 simulator target, we need to add a "simulator" postfix to differentiate.
// Otherwise it'll be treated as targeting "ios" and would fail the compilation.
if (this.flavor.getName().contains("simulator") && this.flavor.getName().contains("arm64")) {
swiftTargetTriple += "-simulator";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fairly certain that we can append -simulator without checking the architecture. x86_64-apple-ios12.0-simulator is also valid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@qyang-nj, we need to check the architecture because the user might be building for "ios device".

Copy link
Collaborator

Choose a reason for hiding this comment

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

@xianwen Isn't this line this.flavor.getName().contains("simulator") guarantee it's building for simulator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@qyang-nj Ah, I see. I misunderstood you the first time.

I was trying to minimize the risk and impact of my changes. Maybe we can remove the check of architecture sometime later, but it's not a high priority issue and I wouldn't feel comfortable removing the checking without doing some extensive testing first.

@xianwen xianwen merged commit a511b1c into airbnb-ios May 17, 2021
@xianwen xianwen changed the title iXianwen swift m1 simulator sdk Fix the Swift target on arm64 simulator. Sep 22, 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