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.

Sign up for GitHub

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

Jump to bottom

haskellPackages.hevm: unbreak #105298

Closed
wants to merge 0 commits into from
Closed

haskellPackages.hevm: unbreak #105298

wants to merge 0 commits into from

Conversation

arcz
Copy link
Member

@arcz arcz commented Nov 28, 2020

Motivation for this change

This PR unbreaks hevm package. It tried to find libff under ff attribute. Some dependencies are specified to tightly and most of the tests depend on a specific version of solc.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

SuperSandro2000
SuperSandro2000 requested changes Nov 29, 2020
View reviewed changes
pkgs/development/libraries/libff/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libff/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libff/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libff/default.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 105298 run on x86_64-linux 1

1 package built:
  • libff

@arcz arcz changed the base branch from master to haskell-updates November 29, 2020 01:25
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 105298 run on x86_64-linux 1

1 package built:
  • libff

cdepillabout
cdepillabout requested changes Nov 29, 2020
View reviewed changes
pkgs/development/haskell-modules/configuration-nix.nix Outdated

# tests depend on a specific version of solc,
# original derivation expects libff to be under ff attribute, add libff explicitly
hevm = dontCheck (doJailbreak (addExtraLibrary super.hevm pkgs.libff));
Copy link
Member

Choose a reason for hiding this comment

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

This addExtraLibrary shouldn't be here.

The problem here is that cabal2nix doesn't know about this libff library.

What you need to do is send a PR to master adding libff, and then a separate PR to https://github.com/NixOS/cabal2nix creating this ff to libff mapping. Please feel free to ping me on both PRs.

Here is the function you have to add to:

https://github.com/NixOS/cabal2nix/blob/d19490223d91a2db1708e423eb205884e9a9d558/src/Distribution/Nixpkgs/Haskell/FromCabal/Name.hs#L16-L27

Once both of these are merged in, you can finally send a PR adding dontCheck and doJailbreak for hevm. (Although keep in mind that if you send a PR adding doJailbreak for hevm, we'll ask you to send a PR / create an issue upstream reporting that they need to loosen their version bounds, so you might want to do that in advance.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@cdepillabout here is the libff PR #105400 and cabal2nix PR NixOS/cabal2nix#471

This was referenced Nov 29, 2020
Merged
Merged
@peti
Copy link
Member

peti commented Dec 4, 2020

I cherry-picked the relevant commit 2742cc43da2a57bc8f63aaac8d67cb27ab430e29. Thank you very much.

@peti peti closed this Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout requested changes

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

Assignees
No one assigned
Labels
6.topic: haskell 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 0 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@arcz @SuperSandro2000 @peti @cdepillabout