Make scrypt test dep optional and not required on darwin#85
Make scrypt test dep optional and not required on darwin#85cdepillabout merged 3 commits intomasterfrom
Conversation
See the following for more information: - NixOS/nixpkgs#351305 - #84
| extractParams | ||
| testsParams8Rounds | ||
| #ifndef IS_MAC_OS | ||
| , testProperty "scrypt <-> cryptonite" $ withMaxSuccess 10 checkScrypt |
There was a problem hiding this comment.
I don't feel like acting we're testing something when we're not is a good idea.
I'd prefer:
#ifndef darwin_HOST_OS
, testProperty "scrypt <-> cryptonite" $ withMaxSuccess 10 checkScrypt
#endifAnd just not defining checkScrypt.
There was a problem hiding this comment.
That's a good point. I've changed this back in b5a96ef.
password/password.cabal
Outdated
|
|
||
| -- The scrypt package is optionally used in tests to ensure compatibility | ||
| -- between password and scrypt. However, scrypt doesn't work on OSX, so | ||
| -- we only add the scrypt dependency if we're not building on OSX. |
There was a problem hiding this comment.
That's not enough. scrypt requires x86_64 with SSE2 instruction set, so also aarch64-linux etc. aren't available.
There was a problem hiding this comment.
So we should be using x86_64_HOST_ARCH?
Is that also the reason it doesn't work on OSX? Or can darwin also be x86_64 sometimes? 🤔
There was a problem hiding this comment.
Yes. x86_64-darwin works fine (with a new enough CPU). aarch64-darwin is broken.
There was a problem hiding this comment.
Ah, thanks for the clarification here, I've updated the tests to only run on x86_64 in b5a96ef.
`password` contains a test that the Scrypt support in `password` is compatible with the actual `scrypt` Haskell package. However, the `scrypt` Haskell package only runs on x86_64 (with the SSE2 instruction set). This commit updates the Scrypt compatibility tests in `password` to only be included if we're building on x86_64. See #85 (review) for more information.
|
Looks good 👍 You could also include a version bump + Changelog addition. (-> |
|
Thanks! I've tagged and released as https://hackage.haskell.org/package/password-3.1.0.1 |
`password` contains a test that the Scrypt support in `password` is compatible with the actual `scrypt` Haskell package. However, the `scrypt` Haskell package only runs on x86_64 (with the SSE2 instruction set). This commit updates the Scrypt compatibility tests in `password` to only be included if we're building on x86_64. See cdepillabout#85 (review) for more information.
3 participants
This is a slight improvement on #84.
I've also confirmed that
stack test --flag password:-scrypt passwordbuilds without problems (on Linux at least).See the following for more information: