Enable multiple outputs for haskell packages. #27209

nc6 · 2017-07-07T14:24:46Z

Motivation for this change

Enable multiple outputs for Haskell packages. This should address issues with references to GHC even when building static binaries.

Since creating this PR, I've spotted #27196, which is going in the same direction. I've lifted the things done in that PR into this. Here I'm trying to go somewhat further in having a split binary output for executables, since when we produce statically linked executables we don't want references to GHC.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

mention-bot · 2017-07-07T14:24:50Z

@nc6, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peti, @cstrahan and @domenkozar to be potential reviewers.

0xABAB · 2017-07-07T14:52:21Z

For such a major feature, it would be nice to have a test.

vcunat · 2017-07-08T08:17:05Z

What :-) The issue has been open for almost two years and now we have two implementations in a single day?

peti · 2017-07-11T10:25:53Z

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

@@ -871,4 +871,9 @@ self: super: {
    postInstall = "rm $out/bin/mkReadme && rmdir $out/bin";
  });

+  # These claim to have executables, but don't


I don't believe that this is the right approach. Instead, that issue must be fixed in cabal2nix. The tool should simply not consider executables that have buildable = false when generating the appropriate Nix expression. I'll fix that ASAP and re-generate hackage-packages.nix. Anyhow, please don't include these changes in this PR.

domenkozar · 2017-07-11T11:54:24Z

I suggest we first merge #27196 and then take this to the next level :)

nc6 · 2017-07-11T14:00:52Z

Updated and rebased atop #27196 - I'll try to keep tracking that branch, unless @cleverca22 wants to incorporate these changes.

domenkozar · 2017-07-11T14:07:49Z

This should reduce the closure-size of happy executable (which we don't build yet separately statically since there is no benefit from it now).

domenkozar · 2017-07-13T08:45:18Z

Thinking about this change, we will probably need to pass bin output at least for buildTools (for example happy)

domenkozar · 2017-07-27T12:21:50Z

@nc6 the initial PR has been merged, so this can be rebased (and I left one comment I think will have to be addressed before we start testing).

Fuuzetsu · 2017-07-28T10:28:10Z

AFAIU we're not porting the other multiple outputs back to 17.09 so I'm unsure if this one should be in that milestone either seeing as it has that as dependency

globin · 2017-07-28T10:41:15Z

Not back to 17.03, 17.09 is correct :)

nc6 · 2017-07-28T16:04:43Z

@domenkozar @peti PTAL. I have hopefully addressed the issue with .e.g. happy, alex and hscolour.

peti · 2017-07-29T11:01:50Z

Test builds run at https://hydra.nixos.org/jobset/nixpkgs/pr-27209.

FRidh · 2017-07-29T11:32:42Z

@peti could you disable these builds or significantly reduce the amount of shares it has. As with the previous Haskell jobset it has 8 times the shares staging has thereby completely stalling staging.

peti · 2017-07-29T12:03:18Z

@FRidh, that jobset has approx. 1% of the available shares on hydra.nixos.org. I don't see how that could possibly "completely stall" any other jobset. Also, please note that this jobset runs only once every couple of days -- namely when I manually trigger it --, whereas staging is evaluated several times per day. Therefore, I think it's appropriate for this jobset to have a slightly higher priority.

FRidh · 2017-07-29T12:10:06Z

@peti
Most branches are stall, so just consider shares.

staging: 6000 (0.13% out of 4488072 shares)
pr-27209: 50000 (1.11% out of 4488072 shares)

Staging is evaluated only once per day. Yes, you trigger this one just for now, but yesterday or the day before there was another one. For several reasons staging has been stuck for a month. Right now it seems to be in a good state again but its not advancing when other jobs are taking the build capacity.

edit: also, trunk has only 1500 shares. With 50000 shares 8 out of 9 ~~jobs~~ shares are for the pr-27209 jobset.

peti · 2017-07-29T12:25:15Z

I think it's perfectly fine to allocate more resources to this jobset than to staging, because this jobset will run far less frequently. If anything, the number of shares allocated to staging should be raised because 0.13% isn't a lot, really. But that's something you should probably discuss with Eelco.

peti · 2017-07-30T08:26:10Z

Run jailbreak-cabal to lift version restrictions on build inputs.
/nix/store/5h0dx5q0dzkm6d8kbdr5flplb2rgmsd1-stdenv/setup: line 73: /nix/store/9gbjrbj775dgjqjalbprgf4zqhk9qjrm-jailbreak-cabal-1.3.2/bin/jailbreak-cabal: No such file or directory

peti

Builds that need jailbreak-cabal are broken.

peti · 2017-07-30T08:32:17Z

pkgs/development/compilers/ghc/7.10.2.nix

@@ -34,7 +34,7 @@ stdenv.mkDerivation rec {
    sha256 = "1x8m4rp2v7ydnrz6z9g8x7z3x3d3pxhv2pixy7i7hkbqbdsp7kal";
  };

-  buildInputs = [ ghc perl libxml2 libxslt docbook_xsl docbook_xml_dtd_45 docbook_xml_dtd_42 hscolour ];
+  buildInputs = [ ghc perl libxml2 libxslt docbook_xsl docbook_xml_dtd_45 docbook_xml_dtd_42 hscolour.bin ];


All those accesses to .bin should be replaced by use of getBin. These variables are function arguments, so we really don't know what kind of derivation will be passed there and cannot rely on the fact that the bin output actually exists. People might call this derivation passing hscolour.bin for all we know.

Use stdenv.lib.getBin instead of `.bin`

Use lib.makeBinPath.

It has files in /usr/share which reference the bin output.

This is not needed, since `hscolour` has no output to `/etc`, and it breaks certain old builds (see https://hydra.nixos.org/build/57925511) because the older cabal used does not understand this flag.

`hastache` has no binary output but for `mkReadme`, so disable the separate bin output and keep removing the `mkReadme`.

Finish overriding bin output for broken packages. Put stack bash completions in $bin.

nc6 · 2017-09-04T11:36:55Z

Rebased to address conflict.

nc6 · 2017-09-04T11:37:51Z

@zimbatm Nothing failing to build (that I can see) due to problems introduced by this PR.

peti · 2017-09-06T07:31:51Z

Um, did anyone test whether a ghcWithPackages environment constructed in this branch actually works? Because it certainly looks like it does not:

nix-shell -p "haskellPackages.ghcWithPackages (ps: [ps.mtl])" --run "ghc-pkg list mtl"
/nix/store/szz84j5k1dy3jdashis6ws28d8l8zxxb-ghc-8.0.2-with-packages/lib/ghc-8.0.2/package.conf.d
    (no packages)

MP2E · 2017-09-06T07:36:58Z

Just finished a rebuild off of master since this was merged, ghcWithPackages is broken here as well. My xmonad config will no longer compile, among other things.

This reverts commit dfb0f25, reversing changes made to 7f8ff02. These changes broke the ghcWithPackages wrapper: nix-shell -p "haskellPackages.ghcWithPackages (ps: [ps.mtl])" --run "ghc-pkg list mtl" /nix/store/szz84j5k1dy3jdashis6ws28d8l8zxxb-ghc-8.0.2-with-packages/lib/ghc-8.0.2/package.conf.d (no packages)

peti · 2017-09-06T08:27:18Z

Since the entire Haskell infrastructure is essentially broken in master, I've pushed a revert in 89f5d52 so that we can fix these remaining issues properly without undue time pressure. There's ghcWithPackages to worry about, and then there's ghcWithHoogle, too, which I reckon will be broken as well.

domenkozar · 2017-11-26T20:06:23Z

@nc6 do you plan to revisit this? It's a really good change, I'm willing to help.

…e-out"" This reverts commit 89f5d52.

nc6 mentioned this pull request Jul 7, 2017

enable split-output builds for all haskellPackages #27196

Closed

8 tasks

nc6 changed the title ~~Nc/haskell multiple out~~ Enable multiple outputs for haskell packages. Jul 7, 2017

domenkozar added 6.topic: haskell 6.topic: closure-size labels Jul 8, 2017

peti requested changes Jul 11, 2017

View reviewed changes

nc6 force-pushed the nc/haskell-multiple-out branch from 311d99f to be06d78 Compare July 11, 2017 13:59

domenkozar mentioned this pull request Jul 27, 2017

Multiple outputs for haskell ecosystem #4504

Closed

fpletz added this to the 17.09 milestone Jul 27, 2017

nc6 force-pushed the nc/haskell-multiple-out branch 3 times, most recently from a60fed0 to 5462cf3 Compare July 28, 2017 15:29

peti requested changes Jul 30, 2017

View reviewed changes

Fuuzetsu force-pushed the nc/haskell-multiple-out branch from 51d1b7d to 7dd2c81 Compare July 31, 2017 14:34

nc6 and others added 9 commits September 4, 2017 12:35

Enable multiple outputs for Haskell packages.

6763624

Provide hscolour, happy, alex bin to buildInputs.

8c642d5

Use stdenv.lib.getBin instead of `.bin`

Fix cabal2nix/stack2nix overrides

cd24744

Use lib.makeBinPath.

Don't build separate bin output for alex.

a1d78fa

It has files in /usr/share which reference the bin output.

Disable separate etc output for hscolour.

330d874

This is not needed, since `hscolour` has no output to `/etc`, and it breaks certain old builds (see https://hydra.nixos.org/build/57925511) because the older cabal used does not understand this flag.

Fix hastache build error.

77cfb96

`hastache` has no binary output but for `mkReadme`, so disable the separate bin output and keep removing the `mkReadme`.

Add specific overrides for a number of failing packages.

414a557

Finish overriding bin output for broken packages. Put stack bash completions in $bin.

Fix broken rebase.

be90e9a

Fix git-annex.

eb99d3a

nc6 force-pushed the nc/haskell-multiple-out branch from 6f563af to eb99d3a Compare September 4, 2017 11:36

peti approved these changes Sep 5, 2017

View reviewed changes

peti merged commit dfb0f25 into NixOS:master Sep 5, 2017

nc6 deleted the nc/haskell-multiple-out branch September 5, 2017 07:19

domenkozar mentioned this pull request Sep 5, 2017

Statically link executables for haskell packages by default #29011

Closed

domenkozar mentioned this pull request Nov 26, 2017

Bring back multiple output for binaries #32082

Closed

peti pushed a commit that referenced this pull request Dec 4, 2017

Revert "Revert "Merge pull request #27209 from nc6/nc/haskell-multipl…

d90165c

…e-out"" This reverts commit 89f5d52.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable multiple outputs for haskell packages. #27209

Enable multiple outputs for haskell packages. #27209

nc6 commented Jul 7, 2017

mention-bot commented Jul 7, 2017

0xABAB commented Jul 7, 2017

vcunat commented Jul 8, 2017

peti Jul 11, 2017

domenkozar commented Jul 11, 2017

nc6 commented Jul 11, 2017

domenkozar commented Jul 11, 2017

domenkozar commented Jul 13, 2017

domenkozar commented Jul 27, 2017

Fuuzetsu commented Jul 28, 2017

globin commented Jul 28, 2017

nc6 commented Jul 28, 2017

peti commented Jul 29, 2017

FRidh commented Jul 29, 2017 •

edited

peti commented Jul 29, 2017

FRidh commented Jul 29, 2017 •

edited

peti commented Jul 29, 2017 •

edited

peti commented Jul 30, 2017

peti left a comment

peti Jul 30, 2017

nc6 commented Sep 4, 2017

nc6 commented Sep 4, 2017

peti commented Sep 6, 2017

MP2E commented Sep 6, 2017

peti commented Sep 6, 2017

domenkozar commented Nov 26, 2017

Enable multiple outputs for haskell packages. #27209

Enable multiple outputs for haskell packages. #27209

Conversation

nc6 commented Jul 7, 2017

Motivation for this change

Things done

mention-bot commented Jul 7, 2017

0xABAB commented Jul 7, 2017

vcunat commented Jul 8, 2017

peti Jul 11, 2017

Choose a reason for hiding this comment

domenkozar commented Jul 11, 2017

nc6 commented Jul 11, 2017

domenkozar commented Jul 11, 2017

domenkozar commented Jul 13, 2017

domenkozar commented Jul 27, 2017

Fuuzetsu commented Jul 28, 2017

globin commented Jul 28, 2017

nc6 commented Jul 28, 2017

peti commented Jul 29, 2017

FRidh commented Jul 29, 2017 • edited

peti commented Jul 29, 2017

FRidh commented Jul 29, 2017 • edited

peti commented Jul 29, 2017 • edited

peti commented Jul 30, 2017

peti left a comment

Choose a reason for hiding this comment

peti Jul 30, 2017

Choose a reason for hiding this comment

nc6 commented Sep 4, 2017

nc6 commented Sep 4, 2017

peti commented Sep 6, 2017

MP2E commented Sep 6, 2017

peti commented Sep 6, 2017

domenkozar commented Nov 26, 2017

FRidh commented Jul 29, 2017 •

edited

FRidh commented Jul 29, 2017 •

edited

peti commented Jul 29, 2017 •

edited