-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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.
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
Enable multiple outputs for haskell packages. #27209
Conversation
@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. |
For such a major feature, it would be nice to have a test. |
What :-) The issue has been open for almost two years and now we have two implementations in a single day? |
@@ -871,4 +871,9 @@ self: super: { | |||
postInstall = "rm $out/bin/mkReadme && rmdir $out/bin"; | |||
}); | |||
|
|||
# These claim to have executables, but don't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
I suggest we first merge #27196 and then take this to the next level :) |
311d99f
to
be06d78
Compare
Updated and rebased atop #27196 - I'll try to keep tracking that branch, unless @cleverca22 wants to incorporate these changes. |
This should reduce the closure-size of |
Thinking about this change, we will probably need to pass |
@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). |
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 |
Not back to 17.03, 17.09 is correct :) |
a60fed0
to
5462cf3
Compare
@domenkozar @peti PTAL. I have hopefully addressed the issue with .e.g. happy, alex and hscolour. |
Test builds run at https://hydra.nixos.org/jobset/nixpkgs/pr-27209. |
@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. |
@FRidh, that jobset has approx. 1% of the available shares on |
@peti
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 |
I think it's perfectly fine to allocate more resources to this jobset than to |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builds that need jailbreak-cabal
are broken.
@@ -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 ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
51d1b7d
to
7dd2c81
Compare
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.
6f563af
to
eb99d3a
Compare
Rebased to address conflict. |
@zimbatm Nothing failing to build (that I can see) due to problems introduced by this PR. |
Um, did anyone test whether a
|
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)
Since the entire Haskell infrastructure is essentially broken in |
@nc6 do you plan to revisit this? It's a really good change, I'm willing to help. |
…e-out"" This reverts commit 89f5d52.
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
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)