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

[CMake] Match lowercase "aarch64" #111

Merged
merged 1 commit into from
Sep 14, 2021
Merged

Conversation

neonichu
Copy link
Member

Seems like it is also possible for "aarch64" to be spelled lowercase, this fixes building on Ubuntu on arm at least.

@lorentey lorentey changed the base branch from main to release/1.0 September 13, 2021 23:20
@lorentey lorentey changed the base branch from release/1.0 to main September 13, 2021 23:21
@lorentey
Copy link
Member

Gah, GitHub cannot do rebases. @neonichu can you please rebase this on top on release/1.0 instead of main?

Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

Looks good!

How are we supposed to test this case?

Seems like it is also possible for "aarch64" to be spelled lowercase, this fixes building on Ubuntu on arm at least.
@neonichu neonichu force-pushed the fix-aarch64 branch 2 times, most recently from ac5152d to 39a16a1 Compare September 13, 2021 23:31
@neonichu neonichu changed the base branch from main to release/1.0 September 13, 2021 23:31
@neonichu
Copy link
Member Author

Rebased on release/1.0.

In terms of testing, @shahmishal should be able to answer this. I am assuming we have some way of doing CI on aarch64, but not sure what it is.

@lorentey
Copy link
Member

@swift-ci test

@shahmishal
Copy link
Member

We should be able to cross pull request test this by creating a fake PR on Swift repo.

@lorentey
Copy link
Member

We should be able to cross pull request test this by creating a fake PR on Swift repo.

IIUC, toolchain builds will only test tagged releases, by design. But that's still much better than nothing! 👍

@neonichu
Copy link
Member Author

Looks like the toolchain build succeeded and I also tested locally in an aarch64 Docker container.

@lorentey lorentey added this to the 1.0.1 milestone Sep 14, 2021
@lorentey lorentey merged commit 2d33a0e into apple:release/1.0 Sep 14, 2021
@tomerd
Copy link
Member

tomerd commented Sep 15, 2021

@lorentey lmk when we have a tag and I will pull this in

@tomerd
Copy link
Member

tomerd commented Sep 20, 2021

@lorentey lmk when we have a tag and I will pull this in

@lorentey any plans on tagging?

@lorentey
Copy link
Member

@tomerd Oh, I'll make a new tag right now!

@lorentey
Copy link
Member

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.

None yet

4 participants