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

sha1: Add extra .S for AArch64 on macOS (Fixing building on M1 Macs) #38

Merged
merged 2 commits into from
Jul 16, 2021

Conversation

tlercher
Copy link
Contributor

@tlercher tlercher commented Jul 4, 2021

I applied the same fix as in #35 to the sha1 version with @PAGE for addressing and @PAGEOFF for loading.

It might not be an ideal solution, but it works for me and fixes #28 on my machine.

@tarcieri
Copy link
Member

tarcieri commented Jul 6, 2021

I'm on vacation and away from my M1, so I'm unable to confirm this works correctly at this time.

I'd prefer to wait until I can do that (or another projector contributor can) before merging this.

Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

I do not (and prefer not to) own any Apple devices, so I can't test this PR. But at the first glance it looks good.

@tarcieri
Copy link
Member

tarcieri commented Jul 9, 2021

I’ll be able to test it in a few days

@Absolucy
Copy link

Absolucy commented Jul 14, 2021

Is there any reason why target_os == "macos" is used instead of target_vendor == "apple"? The build error is also present on iOS targets.

@tarcieri
Copy link
Member

I can confirm it builds. I would suggest adopting @Absolucy's suggestion to use target_vendor == "apple" so it works on iOS as well

@tlercher
Copy link
Contributor Author

Quite the nice solution, thanks for the Input! I added a commit which is also renaming the files (+ fixing sha2 too)

Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@newpavlov any objections to merging this?

@newpavlov
Copy link
Member

@tarcieri
No objections. Feel free to merge and cut a release.

@tarcieri tarcieri merged commit 7e1b366 into RustCrypto:master Jul 16, 2021
@tarcieri tarcieri mentioned this pull request Jul 16, 2021
@tarcieri
Copy link
Member

Okay, sha1-asm v0.5.1 has been released. However, the sha-1 crate still needs to be bumped to use v0.5.x of sha1-asm.

@tarcieri tarcieri mentioned this pull request Jul 16, 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.

sha1: Apple M1 build failures
4 participants