haskellPackages.cryptonite: fix compilation with avx

[SuperSandro2000]

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.05 Release Notes (or backporting 22.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[cdepillabout]

I think cryptonite is already building on the haskell-updates branch. Is it building right now without avx, is that why this patch is needed? Do you have a personal overlay where you enable avx for all of nixpkgs?

[SuperSandro2000]

Is it building right now without avx, is that why this patch is needed? Do you have a personal overlay where you enable avx for all of nixpkgs?

yes and yes. I am setting nixpkgs.config.gcc.arch to skyflake and in all haskell packages I am using this was the only build failure.

[cdepillabout]

I'd like to wait until the cryptonite maintainer replies to haskell-crypto/cryptonite#373 before merging this in.

It is not clear to me the ramifications of this change (or why it was changed originally).

[SuperSandro2000]

@trofi already commented on the idea of the fix a while ago haskell-crypto/cryptonite#347 (comment)

[cdepillabout]

As far as I can tell, trofi isn't a maintainer of cryptonite?

[SuperSandro2000]

No, but the quoted commit message confirms the fix and the maintainers are usually a bit slow to respond and I don't really want to wait potentially multiple weeks.

[cdepillabout]

I don't feel comfortable merging a change to a widely-used crypto library in Haskell without at least the upstream maintainer commenting on it (especially for a use-case that most users won't be affected by).

Although cryptonite is somewhat known for having slow reviews of PRs, so I can understand your frustration...

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/best-practice-to-automaticly-build-all-installed-packages-to-put-them-into-a-binary-cache/25527/14

[SuperSandro2000]

@cdepillabout so nothing really happened in two months and there are more and more people that need this patch. What do we do?

[sternenseemann]

@cdepillabout so nothing really happened in two months and there are more and more people that need this patch. What do we do?

A lot of upstream projects don't include a patch for something downstream users “need” – our patching should be within reason.

Edit: To clarify, I still don't think it is reasonable for us to merge this revert of an upstream change without comment from upstream. Additionally, it is fair to say that the “need” for this change is exclusive to a handful of people that recompile the entirety of nixpkgs with added compiler flags. Having an overlay for that purpose is probably tolerable.

[SuperSandro2000]

What are we doing now that the repository is archived for a crypto those security relevant package?

[someplaceguy]

What are we doing now that the repository is archived for a crypto those security relevant package?

It probably makes sense to replace cryptonite with crypton, which is a fork that appears to be maintained:

https://old.reddit.com/r/haskell/comments/14245q8/crypton_is_forked_from_cryptonite_with_the/jn36k6s/

Although crypton probably still has the same issue as cryptonite, so I guess a new issue should be filed.

[sternenseemann]

Hackage has the ability to deal with an unmaintained package. We'll just have to see what happens.