cabal2nix: split into a lightweight version and a wrapper #81125

madjar · 2020-02-26T15:18:48Z

Current, the cabal2nix derivation contains both the executable, and a wrapper
that adds nix and nix-prefetch-scripts, which are required for some
features.

However, when calling callCabal2nix to create a derivation from a cabal file
at evaluation time,
these features are not actually used, but the huge closure of
nix-prefetch-scripts (which includes multiple vcs, as well as python and perl)
still needs to be fetched.

This commit splits cabal2nix into a lightweight version that is a standalone
static binary (cabal2nix-unwrapped), and a wrapper that includes the proper
dependencies in the path for full usage of the command line
utility (cabal2nix).

This commit also switches to the default ghc, to reduce the likelyhood of
building a different ghc when calling callCabal2nix.

Motivation for this change

Things done

madjar · 2020-02-26T15:20:14Z

@peti Do you know why cabal2nix was using ghc 8.8.1 in the first place?

madjar · 2020-02-26T16:03:06Z

Note for reviewers: I'm not sure what are the best practices for writing such a wrapper, so suggestions are definitely welcome there.

cdepillabout

Can you make this target the haskell-updates branch instead of master?

This seems reasonable to me except for the following three things:

There was probably a reason @peti set cabal2nix to use ghc881 instead of ghc865. That said, haskell-updates is switching to ghc882, so maybe this doesn't matter.
cabal2nix-unwrapped probably shouldn't be exposed at the top level.
symlinkJoin is probably not the right thing to use here, since you're not joining a bunch of derivations. Although, I dunno, maybe its easier than using the normal stdenv.mkDerivation.

madjar · 2020-02-28T14:20:36Z

@cdepillabout Thanks for the review! Regarding your points:

I agree that it was a bit rash to change the ghc along with this change. But I also agree that aligning it to the default version of haskell-updates is probably for the best.
I feel too that it's weird to have cabal2nix-unwrapped living at the top-level, but I dont really know where else to put it (apart from inlining it both in cabal2nix and in haskellSrc2nix, which doesn't sound great). Maybe it can be available as cabal2nix.unwrapped, but that might make overriding much harder. Do you have a suggestion?
EDIT: grep '\-unwrapped' pkgs/top-level/all-packages.nix |wc -l (58) suggest that it's an existing pattern to expose the unwrapped version at the top level.
For symlinkJoin, I followed https://nixos.wiki/wiki/Nix_Cookbook#Wrapping_packages, but it shouldn't be hard to rewrite it as a plain mkDerivation. I'll do that now.

cdepillabout · 2020-02-28T15:56:43Z

I feel too that it's weird to have cabal2nix-unwrapped living at the top-level, but I dont really know where else to put it (apart from inlining it both in cabal2nix and in haskellSrc2nix, which doesn't sound great)

Hmm, in make-package-set.nix, can you just get the normal unwrapped version from buildHaskellPackages instead of buildPackages?

edit: Ah, I see that would end up pulling a cabal2nix built with a version of ghc depending on the haskell package set.

Yeah, I agree that we don't have many good solutions here.

I think I am okay with having cabal2nix-unwrapped on the top-level like you have here currently.

I need to try running this once or twice, but I think it should be pretty good as-is.

madjar · 2020-02-28T16:07:17Z

Not really: we want make-package-set.nix to use the justStaticExecutables version to avoid the gigantic closure of haskell libraries. From there, I thought that is would be nice to make sure the actual cabal2nix binaries are shared between make-package-set.nix and the top-level cabal2nixm which implies using both justStaticExecutables and generateOptparseApplicativeCompletion, and maybe having a way to share these.

On the other hand, we can decide we don't care about the (binary) duplication, and just use justStaticExecutables buildHaskellPackages.cabal2nix in make-package-set.nix. However, this would mean using a different cabal2nix for each version of ghc, which may not be what we want.

In the end, if the duplication of binary is acceptable, then changing make-package-set.nix to use justStaticExecutables buildPackages.haskellPackages.cabal2nix and leaving the top-level unchanged might be a good solution. What do you think?

cdepillabout · 2020-02-28T16:09:37Z

@madjar Sorry, I ninja-edited my last comment while you were typing, I think.

I think this PR looks fine as you have it. Give me a day or so to actually play around with this and make sure it is working.

madjar · 2020-02-28T16:14:21Z

No worries! I think I'll make one more change to the way cabal2nix is defined, to use runCommand instead of raw mkDerivation (to get the local build and substituting settings right).

madjar · 2020-03-02T14:13:18Z

On second thought, I'm happy with this version that uses mkDerivation.

If you're satisfied with this, give me a sign. I'll make sure to rebase on top of a working commit, and merge.

cdepillabout · 2020-03-03T02:01:32Z

@madjar This looks good to me.

I'd like to get @peti to have a quick look at this just in case.

When @peti signs off on it, I'm happy merging this in.

For @peti: the commit in question is a26545b.

infinisil · 2020-03-06T14:16:39Z

pkgs/top-level/all-packages.nix

+      mkdir -p $out/bin
+      ln -s ${cabal2nix-unwrapped}/share $out
+      ln -s ${cabal2nix-unwrapped}/bin/hackage2nix $out/bin
+      makeWrapper ${cabal2nix-unwrapped}/bin/cabal2nix $out/bin/cabal2nix \
        --prefix PATH ":" "${nix}/bin" \
        --prefix PATH ":" "${nix-prefetch-scripts}/bin"


I recommend this instead:

--prefix PATH : "${lib.makeBinPath [ nix nix-prefetch-scripts ]}"

makeBinPath takes care of appending /bin and using the bin output if necessary

Oh, that's cool! I'll do that right away.

madjar · 2020-03-06T14:43:24Z

Rebased on top of haskell-updates, and applied @infinisil's recommendation.

infinisil · 2020-03-06T14:54:01Z

I hope you don't mind, I just pushed a change so it doesn't need a whitelist of paths to be copied from the unwrapped derivation. Makes it a bit more succinct. The symlinkJoin approach is described here: https://nixos.wiki/wiki/Nix_Cookbook#Wrapping_packages

madjar · 2020-03-06T15:41:48Z

@infinisil No worries! I think I used the whitelist to avoid copying position, but now that I look again, it seems to be overridden anyway.

Current, the `cabal2nix` derivation contains both the executable, and a wrapper that adds `nix` and `nix-prefetch-scripts`, which are required for some features. However, when calling `callCabal2nix` to create a derivation from a cabal file at evaluation time, these features are not actually used, but the huge closure of `nix-prefetch-scripts` (which includes multiple vcs, as well as python and perl) still needs to be fetched. This commit splits cabal2nix into a lightweight version that is a standalone static binary (`cabal2nix-unwrapped`), and a wrapper that includes the proper dependencies in the path for full usage of the command line utility (`cabal2nix`). This commit also switches to the default ghc, to reduce the likelyhood of building a different ghc when calling `callCabal2nix`.

cdepillabout · 2020-03-07T02:44:11Z

@GrahamcOfBorg build cabal2nix

@GrahamcOfBorg build cabal2nix-unwrapped

@madjar The haskell-updates branch recently updated, so I rebased this PR again.

I've also tested this with the command:

$ nix-build -E 'with import ./. {}; haskellPackages.callHackageDirect { pkg = "pretty-simple"; ver = "3.2.2.0"; sha256 = "197fmg7j97x6fibzshmqc5h6mbrk7fl3ss5kqy8x8qyd23ai3rpc"; } {}'

As far as I could tell, it looks like it didn't have to pull in a bunch of language runtimes, so it worked as expected.

cdepillabout · 2020-03-07T03:09:30Z

@madjar Thanks for putting this together!

madjar · 2020-03-09T14:49:17Z

My pleasure!

madjar requested review from cdepillabout and infinisil as code owners February 26, 2020 15:18

ofborg bot added the 6.topic: haskell label Feb 26, 2020

madjar force-pushed the split-cabal2nix branch from b049c69 to d5fe57e Compare February 26, 2020 16:01

ofborg bot added the 8.has: package (new) label Feb 26, 2020

ofborg bot requested a review from peti February 26, 2020 16:07

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Feb 26, 2020

cdepillabout requested changes Feb 27, 2020

View reviewed changes

madjar changed the base branch from master to haskell-updates February 27, 2020 09:37

madjar force-pushed the split-cabal2nix branch 3 times, most recently from 776f42d to bb71f8c Compare February 27, 2020 13:46

peti force-pushed the haskell-updates branch from 05357fc to 83c4b73 Compare February 28, 2020 11:38

madjar force-pushed the split-cabal2nix branch from bb71f8c to a26545b Compare February 28, 2020 15:16

peti force-pushed the haskell-updates branch 6 times, most recently from 518e64c to e81ec2f Compare February 28, 2020 19:42

infinisil reviewed Mar 6, 2020

View reviewed changes

madjar force-pushed the split-cabal2nix branch from a26545b to 42dd19b Compare March 6, 2020 14:40

infinisil force-pushed the split-cabal2nix branch from 42dd19b to 4a0fc1a Compare March 6, 2020 14:51

peti force-pushed the haskell-updates branch 6 times, most recently from 0cad7b0 to 6e2198b Compare March 6, 2020 21:15

cdepillabout force-pushed the split-cabal2nix branch from 4a0fc1a to 9f46e2b Compare March 7, 2020 02:41

cdepillabout merged commit 2167fc9 into NixOS:haskell-updates Mar 7, 2020

madjar deleted the split-cabal2nix branch March 9, 2020 14:49

madjar mentioned this pull request Mar 18, 2020

callCabal2nix to use lightweight cabal2nix? #54551

Closed

cdepillabout mentioned this pull request Mar 22, 2020

Infinite recursion in Haskell overlays for some packages #83098

Open

This was referenced Jan 3, 2021

callCabal2nix is broken on nixos-20.09 #108246

Closed

haskellSrc2Nix: Use wrapped cabal2nix #108248

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cabal2nix: split into a lightweight version and a wrapper #81125

cabal2nix: split into a lightweight version and a wrapper #81125

madjar commented Feb 26, 2020 •

edited

Loading

madjar commented Feb 26, 2020

madjar commented Feb 26, 2020

cdepillabout left a comment •

edited

Loading

madjar commented Feb 28, 2020 •

edited

Loading

cdepillabout commented Feb 28, 2020 •

edited

Loading

madjar commented Feb 28, 2020

cdepillabout commented Feb 28, 2020

madjar commented Feb 28, 2020 via email •

edited

Loading

madjar commented Mar 2, 2020

cdepillabout commented Mar 3, 2020

infinisil Mar 6, 2020

madjar Mar 6, 2020

madjar commented Mar 6, 2020

infinisil commented Mar 6, 2020

madjar commented Mar 6, 2020

cdepillabout commented Mar 7, 2020

cdepillabout commented Mar 7, 2020

madjar commented Mar 9, 2020

cabal2nix: split into a lightweight version and a wrapper #81125

cabal2nix: split into a lightweight version and a wrapper #81125

Conversation

madjar commented Feb 26, 2020 • edited Loading

Motivation for this change

Things done

madjar commented Feb 26, 2020

madjar commented Feb 26, 2020

cdepillabout left a comment • edited Loading

Choose a reason for hiding this comment

madjar commented Feb 28, 2020 • edited Loading

cdepillabout commented Feb 28, 2020 • edited Loading

madjar commented Feb 28, 2020

cdepillabout commented Feb 28, 2020

madjar commented Feb 28, 2020 via email • edited Loading

madjar commented Mar 2, 2020

cdepillabout commented Mar 3, 2020

infinisil Mar 6, 2020

Choose a reason for hiding this comment

madjar Mar 6, 2020

Choose a reason for hiding this comment

madjar commented Mar 6, 2020

infinisil commented Mar 6, 2020

madjar commented Mar 6, 2020

cdepillabout commented Mar 7, 2020

cdepillabout commented Mar 7, 2020

madjar commented Mar 9, 2020

madjar commented Feb 26, 2020 •

edited

Loading

cdepillabout left a comment •

edited

Loading

madjar commented Feb 28, 2020 •

edited

Loading

cdepillabout commented Feb 28, 2020 •

edited

Loading

madjar commented Feb 28, 2020 via email •

edited

Loading