-
Notifications
You must be signed in to change notification settings - Fork 253
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
Disable ASM hashes on MSVC? #315
Comments
not-an-aardvark
added a commit
to not-an-aardvark/lucky-commit
that referenced
this issue
Sep 9, 2021
not-an-aardvark
added a commit
to not-an-aardvark/lucky-commit
that referenced
this issue
Sep 9, 2021
not-an-aardvark
added a commit
to not-an-aardvark/lucky-commit
that referenced
this issue
Sep 9, 2021
Sounds reasonable. But I wonder whether we should use black- or white-list approach here (i.e. disable for MSVC or enable for GNU and other supported toolchains). |
excitoon
pushed a commit
to excitoon/lucky-commit
that referenced
this issue
Aug 29, 2022
excitoon
pushed a commit
to excitoon/lucky-commit
that referenced
this issue
Aug 31, 2022
Dependency on asm-hashes will be removed in v0.11 releases. |
@newpavlov should we close the remaining issues on https://github.com/RustCrypto/asm-hashes and archive it? |
Yes, but I think we should do it after v0.11 hashes are released. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As documented in RustCrypto/asm-hashes#17 and elsewhere, the crates in
asm-hashes
don't work on Windows MSVC targets. This is unfortunate, but it's understandable given that assembly is inherently difficult to port to all platforms.However, it seems unnecessary that this detail leaks into the crates in the
hashes
repository. Would it make sense to use conditional compilation to force the software versions of hashes on MSVC, even when theasm
feature is enabled? (There is already precedent for ignoring theasm
feature flag when no assembly builds are available for the current platform.)My use case is that I want to enable the
asm
feature to allow for faster performance on compatible platforms, but I don't want to completely break the build for Windows users. I'd be happy to create a PR if you think this is a reasonable change.The text was updated successfully, but these errors were encountered: