Haskell multiple outputs: bin etc lib (take three) #32629

domenkozar · 2017-12-13T10:19:41Z

Addresses failures found in #32112

Fixes #32082

Detailed changelog is in commits, but shortly:

@peti I'd kindly ask you for a jobset including darwin, since I can't build all of the packages myself.

jb55

Lightly skimmed, untested ACK. Although as a nit the sprinkled getBins seems a bit ugly, but I have no improvements to recommend. 😞

peti

After 2 failed attempts to get these changes merged, I would like someone to actually use this code before merging it because verifying the new code by means of Hydra only has clearly failed before. I.e. it would be great if someone would confirm, explicitly, "yes, I have used the code in this PR for the following non-trivial use-cases X and Y for the last 2 weeks and I can confirm that it all works as expected".

peti · 2017-12-14T11:35:17Z

pkgs/development/compilers/ghc/8.2.1.nix

@@ -10,7 +10,7 @@
 let
  inherit (bootPkgs) ghc;
  version = "8.2.1";
-
+  preReleaseName = "ghc-8.2.1";


What is this for?

peti · 2017-12-14T11:37:29Z

pkgs/development/compilers/halvm/2.4.0.nix

@@ -20,9 +20,9 @@ stdenv.mkDerivation rec {
    sed -ie 's|ld |${targetPackages.stdenv.cc.bintools}/bin/ld |g' src/scripts/ldkernel.in
  '';
  configureFlags = stdenv.lib.optional (!enableIntegerSimple) [ "--enable-gmp" ];
-  propagatedNativeBuildInputs = [ alex happy ];
+  propagatedNativeBuildInputs = [ (stdenv.lib.getBin alex) (stdenv.lib.getBin happy) ];


My understanding is that a reference like alex refers to the output by default that's defined first. Is that correct?

If that is so indeed, then we should make sure that the first output defined by Haskell packages is the one containing the binaries so that these references don't need to be wrapped in getBin all the time.

bin is the first one, this code is just trying to be explicit.

See https://github.com/NixOS/nixpkgs/blob/haskell-outputs-bin-etc-lib/pkgs/development/haskell-modules/generic-builder.nix#L204-L212

I think that being explicit in this case is not an advantage since it makes the code harder to read without adding any functional benefit. People might be confused by those explicit calls, wondering: "Why is this necessary?" Personally, I'd be in favor of dropping getBin calls whereever possible. Rather than being explicit, we should document that bin can be relied upon to be the first output of all Haskell derivations.

It doesn't need to be specifically documented for Haskell since it should hold for all of nixpkgs: https://github.com/NixOS/nixpkgs/blob/master/doc/multiple-output.xml#L39

domenkozar · 2017-12-15T11:52:32Z

@peti I have compiled https://github.com/input-output-hk/cardano-sl with it (non trivial project, 300+ dependencies) and it builds. Haven't deployed it yet.

domenkozar · 2017-12-15T11:59:06Z

Created jobset to test all packages at https://hydra.nixos.org/jobset/nixpkgs/pr-32629

grahamc · 2017-12-15T21:53:43Z

@GrahamcOfBorg eval

domenkozar · 2017-12-16T19:26:39Z

According to https://hydra.nixos.org/eval/1418670?compare=trunk#tabs-new there are following outstanding darwin failures:

vcunat · 2017-12-18T12:03:29Z

I have the same xmonad problem on x86_64-linux (as of a0b51de).
When trying that, I noticed that lots of *-doc haskell paths are downloaded to build xmonad-with-packages – they don't seem to be big, but it still surprised me.

domenkozar · 2017-12-18T12:05:31Z

@vcunat ghcWithPackages installs also doc output, I assume that's for having access to docs in the env.

vcunat · 2017-12-18T12:10:53Z

Hmm, that might be configurable in ghcWithPackages, but it's just a nitpick, and not that much related.

domenkozar · 2017-12-20T18:38:32Z

@vcunat fixed :)

domenkozar · 2017-12-21T16:00:31Z

I still need to fix cabal-helper on darwin, but otherwise this is ready for testing.

domenkozar · 2017-12-21T18:24:44Z

@Ericson2314 @shlevy @zimbatm @jb55 @ttuegel @basvandijk @nh2 @nc6 @Boothead @bennofs @Gabriel439 @taktoa @Profpatsch @dezgeg @ryantrinkle @abbradar @rickynils

Could I ask you to test this PR with your projects to catch any possible regressions?

bennofs · 2017-12-21T18:58:38Z

pkgs/development/compilers/elm/default.nix

@@ -45,7 +45,7 @@ let
      let hlib = haskell.lib;
          elmRelease = import ./packages/release.nix { inherit (self) callPackage; };
          elmPkgs' = elmRelease.packages;
-          elmPkgs = elmPkgs' // {
+          elmPkgs = lib.mapAttrs (name: value: hlib.disableSharedExecutables value) (elmPkgs' // {


Is this correct? I'm not a user of the elm packages, but this line looks wrong to me: it only disable shared executables for "outside" users of the elm packages, but if any package in the elmPkgs set depends on another one, it will get the shared executable version (b.c. the fixpoint set isn't changed. the modification is applied after the fixpoint).

For example, if we had this situtation:

elmPkgs = lib.mapAttrs lib.mapAttrs (name: value: hlib.disableSharedExecutables value) ({ elm-exe-a = ... derivation depending on self.elm-exe-b elm-exe-b = ... some executable ... })

then elm-exe-a will get the shared version of elm-exe-b, but elmPkgs.elm-exe-b will refer to the non-shared version. Maybe it does not matter in this case though because no elm package depends on executables from another one? Or is it actually meant to be only for the "outside"? (why?)

You're right, need to properly override the scope. Couldn't find an easy way to do that without explicitly enumerating all elm packages.

Fixed, this will do a few more rebuilds but it's a proper override.

bennofs · 2017-12-21T19:03:36Z

pkgs/development/haskell-modules/configuration-nix.nix

-  wxc = (addBuildDepend super.wxc self.split).override { wxGTK = pkgs.wxGTK30; };
+  wxc = (overrideCabal super.wxc (drv: {
+      buildDepends = (drv.buildDepends or []) ++ [self.split];
+      postInstall = "cp -v dist/build/libwxc.so.0.92.3.0 $lib/lib/libwxc.so";


Is this related to the multiple-outputs changes or something else? Hardcoding the exact version number here also seems a little brittle. And why is the copy needed, shoudn't cabal install take care to install the wxc library?

bennofs · 2017-12-21T19:15:59Z

pkgs/development/haskell-modules/generic-builder.nix

+    ${optionalString (hasLibOutput && ! enableSeparateDataOutput) ''
+    # Just like for doc output path in $out potentially landing in
+    # *.conf, we have to also remove the data directory so that it
+    # doesn't appear under data-dir field creating a cycle.


this looks to me like it has potential for silently breaking packages: if a library needs access to data files and enableSeparateDataOutput is not set, we will brick the library, right? (because references will be replaced with non-existent files).

Can we limit this to only .conf files, and have the build break in other cases?

Good catch.

shlevy · 2017-12-22T19:34:42Z

@domenkozar Can you fix conflicts/bring up to date with master and ghc 8.2.2? Then I'll test on our projects.

domenkozar · 2017-12-23T21:56:24Z

@shlevy done

domenkozar · 2017-12-23T22:07:40Z

@GrahamcOfBorg eval

shlevy · 2017-12-24T17:30:41Z

@domenkozar:

$ nix-build -A haskell.packages.ghc802.mtl
[snip]
/nix/store/0bvk0df880x9s5sfacb3z97f40lysmff-stdenv/setup: eval: line 1101: unexpected EOF while looking for matching `''
builder for '/nix/store/25mlqxgzcag7kscqmmbzsqj7w53asb45-mtl-2.2.1.drv' failed with exit code 2
error: build of '/nix/store/25mlqxgzcag7kscqmmbzsqj7w53asb45-mtl-2.2.1.drv' failed

bennofs · 2017-12-25T11:55:52Z

@shlevy that was just a missing ', I pushed a commit that fixes it

shlevy · 2017-12-25T16:18:55Z

$ nix-build -A haskellPackages.hspec-meta
these derivations will be built:
/nix/store/6sr84x4z0ff9j0xbmm9mfxc43fkx25j4-hspec-meta-2.4.4.drv
[snip]
checking for references to /tmp/nix-build-hspec-meta-2.4.4.drv-0 in /nix/store/iji5cysb6cfwzddwh5maz5r31dwak172-hspec-meta-2.4.4-lib...
cycle detected in the references of '/nix/store/ih60kbmw7wdr7ph9hg77ygn2vn1h017g-hspec-meta-2.4.4' from '/nix/store/iji5cysb6cfwzddwh5maz5r31dwak172-hspec-meta-2.4.4-lib'

Sadly on darwin, we always create links file in $lib to workaround sierra linker limit. This means we have to always create the lib output even if isLibrary is false. Moreover, binaries in $bin reference $out even if references files don't exist. Probably needs fixing in GHC itself. generic-builder for now just dereferences $bin to $out. This is fragile if a package uses custom setup.

Reduces closure size from 2.7GB to ~190MB (where most is python+nodejs) This optimization is now possible since bin is a separate output but we can build $lib so that other libraries can depend upon any elm component.

Darwin builds always have a lib *output* but may not produce a lib *directory* inside of it when they are not library packages and the executable does not require dylibs for circumventing Sierra linker limits.

It is just uuagc itself but preprocessed by uuagc. Not haddocking should save some space and build time.

The purpose here is to address the failure mode where $lib is not created. Additionally this reduces the number of store paths and nars.

ryantm · 2018-07-22T16:19:04Z

They are many references to this PR fixing open Haskell issues. What's the status of it?

domenkozar · 2018-07-22T16:32:15Z

It's kind of broken until staging is merged, then we can asses it's status.

roberth · 2018-07-23T09:57:29Z

New todo:

implement 'safe' remove-references that checks whether either target is empty or reference wasn't made
simplify by applying the safe variation generously
rebase
test using hydra job for this branch

dezgeg · 2018-07-23T10:16:18Z

pkgs/development/haskell-modules/generic-builder.nix

@@ -222,7 +253,15 @@ assert allPkgconfigDepends != [] -> pkgconfig != null;
 stdenv.mkDerivation ({
  name = "${pname}-${version}";

-  outputs = [ "out" ] ++ (optional enableSeparateDataOutput "data") ++ (optional enableSeparateDocOutput "doc");
+  outputs =


Hmm, no dev output? I would expect one to contain all the .o and .hi files, plus the GHC package .conf file.

dezgeg · 2018-07-23T10:19:27Z

pkgs/development/haskell-modules/generic-builder.nix

@@ -246,14 +285,14 @@ stdenv.mkDerivation ({

  postPatch = optionalString jailbreak ''
    echo "Run jailbreak-cabal to lift version restrictions on build inputs."
-    ${jailbreak-cabal}/bin/jailbreak-cabal ${pname}.cabal
+    ${stdenv.lib.getBin jailbreak-cabal}/bin/jailbreak-cabal ${pname}.cabal


It's not needed to call the getBin here. The rule of multiple outputs is that ${foo}/bin/bar will always, always do the right thing.

nh2 · 2018-11-27T12:09:52Z

Can this also split out profiling builds? That would be useful.

domenkozar · 2019-02-06T03:43:40Z

If we scale down ambitions of this PR and just get optional bin output first, this would be a small step forward. Enabling it by default can be phase 2. Same goes for the rest of splits.

domenkozar · 2019-03-29T07:10:20Z

Seems like that's the new plan then.

domenkozar · 2019-03-29T08:14:59Z

#58525

domenkozar added the 6.topic: haskell label Dec 13, 2017

domenkozar requested a review from peti as a code owner December 13, 2017 10:19

domenkozar mentioned this pull request Dec 13, 2017

Bring back multiple output for binaries #32082

Closed

domenkozar force-pushed the haskell-outputs-bin-etc-lib branch from 7378681 to 8751bdc Compare December 13, 2017 10:33

GrahamcOfBorg added 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ labels Dec 13, 2017

domenkozar mentioned this pull request Dec 13, 2017

Reduce Elm closure #15800

Closed

jb55 reviewed Dec 13, 2017

View reviewed changes

peti reviewed Dec 14, 2017

View reviewed changes

domenkozar force-pushed the haskell-outputs-bin-etc-lib branch from f73e524 to a0b51de Compare December 15, 2017 14:51

bennofs reviewed Dec 21, 2017

View reviewed changes

domenkozar force-pushed the haskell-outputs-bin-etc-lib branch from a0348e7 to e41e02a Compare December 23, 2017 21:56

domenkozar and others added 14 commits July 10, 2018 15:44

zsh-git-prompt: fix build for multiple outputs

f6c946d

haskell: add tests covering ghcWith{Packages,Hoogle}

23485c6

elmPackages: statically link haskell deps in executables

5d31589

Reduces closure size from 2.7GB to ~190MB (where most is python+nodejs) This optimization is now possible since bin is a separate output but we can build $lib so that other libraries can depend upon any elm component.

elmPackages: properly override the whole dep tree

35c51fd

haskell generic builder: remove references only for .conf in libdir

49af796

haskell generic builder: add missing closing quote

e62a5a8

haskellPackages.happy: fix circular reference caused by separate bin

3dc0648

haskell generic builder: make robust against missing lib directory

9027a69

Darwin builds always have a lib *output* but may not produce a lib *directory* inside of it when they are not library packages and the executable does not require dylibs for circumventing Sierra linker limits.

haskellPackages.hspec-{meta,discover}: resolve store reference cycles

5ebdc4e

haskellPackages.uuagc-bootstrap: don't haddock

9c9ff37

It is just uuagc itself but preprocessed by uuagc. Not haddocking should save some space and build time.

haskell generic builder: broaden datadir removal from lib

68cc2a4

haskell generic builder: avoid redundant lib output on darwin

709289b

The purpose here is to address the failure mode where $lib is not created. Additionally this reduces the number of store paths and nars.

haskell generic builder: use setup dependency libs

981458a

roberth force-pushed the haskell-outputs-bin-etc-lib branch from fb50809 to 981458a Compare July 11, 2018 14:58

GrahamcOfBorg added the 2.status: merge conflict label Jul 11, 2018

This was referenced Jul 15, 2018

Feature/mingw fixes #43559

Closed

pandoc - statically linked closure-size #34376

Closed

dezgeg reviewed Jul 23, 2018

View reviewed changes

domenkozar mentioned this pull request Sep 11, 2018

[Help request] Minimize the size of the nar files cachix/cachix#125

Closed

domenkozar closed this Mar 29, 2019

peti deleted the haskell-outputs-bin-etc-lib branch March 30, 2019 10:51

Haskell multiple outputs: bin etc lib (take three) #32629

Haskell multiple outputs: bin etc lib (take three) #32629

Conversation

domenkozar commented Dec 13, 2017 • edited

jb55 left a comment

Choose a reason for hiding this comment

peti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenkozar commented Dec 15, 2017

domenkozar commented Dec 15, 2017

grahamc commented Dec 15, 2017

domenkozar commented Dec 16, 2017 • edited

vcunat commented Dec 18, 2017

domenkozar commented Dec 18, 2017

vcunat commented Dec 18, 2017

domenkozar commented Dec 20, 2017

domenkozar commented Dec 21, 2017 • edited

domenkozar commented Dec 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shlevy commented Dec 22, 2017

domenkozar commented Dec 23, 2017

domenkozar commented Dec 23, 2017

shlevy commented Dec 24, 2017 • edited

bennofs commented Dec 25, 2017

shlevy commented Dec 25, 2017

ryantm commented Jul 22, 2018

domenkozar commented Jul 22, 2018

roberth commented Jul 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nh2 commented Nov 27, 2018

domenkozar commented Feb 6, 2019 • edited

domenkozar commented Mar 29, 2019

domenkozar commented Mar 29, 2019

domenkozar commented Dec 13, 2017 •

edited

domenkozar commented Dec 16, 2017 •

edited

domenkozar commented Dec 21, 2017 •

edited

domenkozar commented Dec 21, 2017 •

edited

shlevy commented Dec 24, 2017 •

edited

domenkozar commented Feb 6, 2019 •

edited