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.

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

haskell: Split boot libraries into individual outputs (take 2) #43070

Closed
wants to merge 6 commits into from

Conversation

@kirelagin
Copy link
Member

kirelagin commented Jul 5, 2018

This is like #43017 (fix for #42069), but:

  1. Rebased onto master (now with GHC 8.6)
  2. Does only the bare minimum, only moves packages to individual outputs.

I am still not sure what to do with ghcjs (and whether anything needs to be done at all).

@Ericson2314

@Ericson2314
Copy link
Member

Ericson2314 commented Jul 5, 2018

Great!! I did like your ghc -> conpiler change from the last PR, but we can do that later. GHCJS could benefit from this too IIRC, @ElvishJerricco can maybe help with that. (But that also can be a later PR.)

@kirelagin
Copy link
Member Author

kirelagin commented Jul 5, 2018

I did like your ghc -> conpiler change

Yep, I can do it later as a separate PR, because I would still like to somewhat clean up the way things are passed around there. But I need you to help me by answering my questions in the previous PR 🙂.

@Ericson2314
Copy link
Member

Ericson2314 commented Jul 5, 2018

I saw that after posting here, sorry, and just did a bit there.

@ElvishJerricco
Copy link
Contributor

ElvishJerricco commented Jul 5, 2018

Am I misunderstanding, or isn't this just going to make the issues with too many command line arguments even worse?

@kirelagin
Copy link
Member Author

kirelagin commented Jul 5, 2018

I don’t know 🤷‍♂️.

In any case, this change is orthogonal to those issues and even if it makes them worse, let’s hope they will get eventually resolved by fixing the root cause.

@Ericson2314
Copy link
Member

Ericson2314 commented Jul 5, 2018

@ElvishJerricco It should just add a few more.

Copy link
Member

peti left a comment

Wouldn't it be far simpler to clean out all undesirable package config files from the generated package.conf.d directory so that only those mentioned in setup-depends actually show up in there?

Copy link
Member

peti left a comment

ghc is the wrong place to fix #42069. The issue is caused by incorrect code in the generic builder and it ought to be fixed there, not in the compiler.

@kirelagin
Copy link
Member Author

kirelagin commented Jul 7, 2018

@peti All this story is not specific to Setup.hs and #42069 is essentially only about a regression introduced recently, but the issue is still present not only when building Setup.hs. The only reason Haskell packages can be compiled without this change is that ghc is being treated as a Haskell dependency; if we fix the generic builder to actually only add Haskell dependencies to pkgdb, none of the boot packages will get there because, as you know, they are null-ed out.

@peti
Copy link
Member

peti commented Jul 10, 2018

If we fix the generic builder to actually only add Haskell dependencies to pkgdb, none of the boot packages will get there because, as you know, they are null-ed out.

Yes, this is true. From that point of view, an ideal solution would be to have a derivation per core library and to adapt the overrides to point to them (rather than null).

@peti
Copy link
Member

peti commented Jul 11, 2018

How did you test this PR?

@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

I can’t say that I tested it a lot, as it was meant mostly as a PoC and RFC.

I built a couple of heavy packages (pandoc, stack) with the default GHC and only instantiated them with all other versions. I also checked a couple of things interactively in nix-shell (to persuade myself that things actually work as expected). Oh and I know for sure that this fixes the original issue it was intended to fix, i.e. our build (which is the source of all the issues I have been filing lately) succeeds with current master with this patch applied.

@@ -148,7 +158,9 @@ let
'';
});

in package-set { inherit pkgs stdenv callPackage; } self // {
in genAttrs (ghc.passthru.bootPackages or []) (name: ghc."${haskellLib.toOutputName name}") //

This comment has been minimized.

Copy link
@matthewbauer

matthewbauer Jul 11, 2018

Member

You should leave off the passthru part. It should just be ghc.bootPackages.

@matthewbauer
Copy link
Member

matthewbauer commented Jul 11, 2018

Does something like ghci still work with this? I would think that it wouldn't be able to find the right dependencies outside of cabal2nix. We may need to tell GHC at build time where the destination will be - otherwise you will only be able to use GHC base outside of a Nix builder with something like this:

haskellPackages.ghcWithPackages (self: [ self.base ])
@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

ghci works but your question made me think that this is actually a problem, because it is not supposed to. What I should do is regenerate the pkgdb cache after moving the config files away.

To improve UX with ghci we should probably make all ghc environments that are meant to be used interactively include base by default.

@peti
Copy link
Member

peti commented Jul 11, 2018

@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

@peti
callHackage works.
ghcWithPackages does not. The problem is that dependency propagation doesn’t work for boot packages, e.g. if a package I am adding depends on binary, binary will be propagated, but binary depends on containers and containers will not be, because we don’t know anything about dependencies between boot packages.

I think it might be time we stop pulling boot packages from ghc build and instead just grab the matching version from Hackage and compile it again... The number of boot packages that cannot be built this way is hopefully small and we’ll have to handle them separately. Any ideas?

@Ericson2314
Copy link
Member

Ericson2314 commented Jul 11, 2018

@kirelagin I'm all for compiling again: It could well be simpler, and is closer to what we want to end up doing (build GHC without libraries). But if need be, we can fake nix-support/propagated-build-inputs so things work anyways.

@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

FTR here is another bug:
rts is very special in that it doesn’t have a version so its config file rts.conf (without -version) doesn’t match the regex I currently use, so it doesn’t get copied and the rts output ends up being empty which, in turn, means that packages that depend on it won’t build as package.conf.d/*.conf used to populate pkgdb matches no files and thus fails.
Looks like rts needs to be special-cased.

@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

FTR I have just realised that GHC executable itself works only because of the stale pkgdb cache :/.

@kirelagin
Copy link
Member Author

kirelagin commented Jul 11, 2018

I think we should just leave those libraries that are built together with GHC alone as much as possible, just pretend that they are merely a part of the resulting compiler and don’t use them for anything else.

Then we’ll have real derivations for boot libraries, most of them will be just plain versions form Hackage and those that cannot be compiled will be like in #43300.

@Ericson2314
Copy link
Member

Ericson2314 commented Jul 11, 2018

@kirelagin I think that should move us away from a multiple output direction then? If you notice there's some wrapping with a -B that is how GHC finds itself. We can replace that, in the GHC shim derivation, with a different -B ... to a boot directory without any libraries.

We can also protype the shimming + fresh separately-built boot libraries with the prebuilt GHCs and prebuilt-GHC-built-hscolour, which will help with debug cycles too.

CC @matthewbauer

@mmahut
Copy link
Member

mmahut commented Aug 3, 2019

Any updates on this pull request?

@cdepillabout
Copy link
Member

cdepillabout commented May 5, 2020

Similar to #39735 (comment), this is quite old, and hasn't seen any updates recently, so I am going to go ahead and close it.

If anyone is interested in pursuing this, please feel free to open a new PR on the current haskell-updates branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.