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-generic-builder: disable library-for-ghci by default #59297

Merged
merged 1 commit into from Apr 11, 2019

Conversation

cdepillabout
Copy link
Member

This commit disables the library-for-ghci flag passed to Setup configure in the Haskell generic-builder.nix file.

This stops HSfoo.o files from being built. Building HSfoo.o files cause doctest to take an extremely long time to load dependencies when running.

This is a follow-up from #58743. There is more explanation on that PR.

Motivation for this change

It is debatable as to whether or not we should make this change.

The reasons in favor of making this change:

  • Doctest now works for large packages with many dependencies without taking a lot of time to run. This has been a problem for us at work. I've also run into this personally with the Haskell package Termonad: doctests hang when compiling with nix cdepillabout/termonad#15
  • It appears that both stack and cabal use --disable-library-for-ghci by default. It makes sense that nixpkgs would follow this.
  • Disabling library-for-ghci reduces the build times very slightly. The resulting output takes up slightly less disk space. (I don't think this is a particularly compelling reason though.)

The reasons against making this change:

  • Maybe someone is depending on --enable-library-for-ghci being used? Maybe disabling it would break their build/workflow somehow? I have done a bunch of searching around this issue trying to figure out what is going on here, and I haven't found any reference to anyone specifically enabling or disabling library-for-ghci, so I would doubt anyone is actively depending on it (although there is a possibility).
  • The GHC documentation claims that enabling library-for-ghci causes packages to be loaded into GHCi slightly quicker than when it is disabled: haskell-generic-builder: Add option to disable library-for-ghci #58743 (comment). However, in the case of doctest (which loads packages uses the GHCi API), this seems to not be the case. I don't know what is going on here.

My personal opinion is that we should merge this PR and disable library-for-ghci by default. I think it is more important that commonly used tools like doctest work well, than having libraries (sometimes?) load slightly quicker in GHCi.

At some point in the future, I imagine it would be best if we could figure out what's going on with doctest, and whether the bug lies in our nix stuff, doctest, Cabal, GHC, etc.

Pinging @peti.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This commit disables the library-for-ghci flag passed to
`Setup configure` in the Haskell generic-builder.nix file.

This stops the HSfoo.o file from being built.  Building this
HSfoo.o file caused doctest to take an extremely long time
to load dependencies when running.

This is a follow-up from NixOS#58743.
@cdepillabout
Copy link
Member Author

Locally, I tried building a bunch of Haskell packages with the change from this PR. Everything seems to build and run fine.

@peti peti merged this pull request into NixOS:haskell-updates Apr 11, 2019
@michaelpj
Copy link
Contributor

I think this might be linked to this GHC ticket: https://gitlab.haskell.org/ghc/ghc/issues/15524#note_193060

Just worth keeping on the radar that we should check whether we should revert this if that ticket gets fixed.

@cdepillabout
Copy link
Member Author

@michaelpj Thanks for the link to the GHC issue!

@cdepillabout cdepillabout deleted the disable-library-for-ghci branch April 11, 2019 09:17
@peti
Copy link
Member

peti commented Apr 11, 2019

With this patch applied, every single ghc compiler build on hydra.nixos.org has failed.

Let's hope that this was caused by some freak accident / issue on Hydra itself. I'll try restarting the builds ASAP, but this does look a little strange.

@cdepillabout
Copy link
Member Author

cdepillabout commented Apr 12, 2019

@peti, hmm, that's strange. I've tried it many times locally on both NixOS and Debian (with nix) and never run into a problem with building GHC.

Do you have a link to the hydra failure?

I'm not very familiar with how to use Hydra, but after poking around a little bit, it looks like this might be the evaluation you restarted:

https://hydra.nixos.org/eval/1513940

If I go to the "Queued jobs" tab and search for haskell.compiler, it looks like at least ghc822 has successfully been compiled. Hopefully the others compile successfully as well.

I guess another thing we could do is ask ofborg to rebuild something like haskell.compiler.ghc864, but I think I remember that ofborg isn't supposed to be used for building these types of large packages.


It looks like ghc822 has succeeded:

https://hydra.nixos.org/build/92045197

But ghc844 is hanging at the very end?

https://hydra.nixos.org/build/92045468

ghc822 only took about 2 hours to build, but ghc844 has taken over 5 hours so far. It looks like ghc844 is hanging at the very end, right after it is checking for references to /build/ in the docs package:

shrinking RPATHs of ELF executables and libraries in /nix/store/dznib7mbz15qkrfdfvkmdwi6cwa09948-ghc-8.4.4-doc
/nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip is /nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip
strip is /nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip
patching script interpreter paths in /nix/store/dznib7mbz15qkrfdfvkmdwi6cwa09948-ghc-8.4.4-doc
/nix/store/dznib7mbz15qkrfdfvkmdwi6cwa09948-ghc-8.4.4-doc/share/doc/ghc/html/libraries/gen_contents_index: interpreter directive changed from "/bin/sh" to "/nix/store/yjkch3aia9ny4dq42dbcjrdwqb1y8c33-bash-4.4-p23/bin/sh"
checking for references to /build/ in /nix/store/dznib7mbz15qkrfdfvkmdwi6cwa09948-ghc-8.4.4-doc...

Maybe this is indicative of hydra errors? The logs for the ghc822 build basically end in the same way, but this appears to succeed:

shrinking RPATHs of ELF executables and libraries in /nix/store/bx8ypz650y47xwys7f5qpvzmf9i0w8cf-ghc-8.2.2-doc
/nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip is /nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip
strip is /nix/store/1kl6ms8x56iyhylb2r83lq7j3jbnix7w-binutils-2.31.1/bin/strip
patching script interpreter paths in /nix/store/bx8ypz650y47xwys7f5qpvzmf9i0w8cf-ghc-8.2.2-doc
/nix/store/bx8ypz650y47xwys7f5qpvzmf9i0w8cf-ghc-8.2.2-doc/share/doc/ghc/html/libraries/gen_contents_index: interpreter directive changed from "/bin/sh" to "/nix/store/yjkch3aia9ny4dq42dbcjrdwqb1y8c33-bash-4.4-p23/bin/sh"
checking for references to /build/ in /nix/store/bx8ypz650y47xwys7f5qpvzmf9i0w8cf-ghc-8.2.2-doc...

@mpickering
Copy link
Contributor

mpickering commented Apr 12, 2019

I can confirm that this issue is what caused https://gitlab.haskell.org/ghc/ghc/issues/15524

I also managed to build GHC with this flag patched away. Thank you @michaelpj and @cdepillabout for finding this issue and pointing me to it.

@peti
Copy link
Member

peti commented Apr 12, 2019

The compiler builds don't seem to end. 20h and counting. :-( I have opened NixOS/hydra#650. This looks like an issue with Hydra. Maybe it got stuck while copying the build products into the cache.

@cdepillabout
Copy link
Member Author

@peti It looks like ghc822 and ghc864 have succeeded:

ghc822: https://hydra.nixos.org/build/92045197
ghc864: https://hydra.nixos.org/build/92047012

Hopefully we can get ghc844 (and, well, ghc865) succeeding soon too.

Although now that ghc864 has succeeded, hopefully a bunch of the Haskell packages will start succeeding now too.

@mpickering
Copy link
Contributor

The strange thing is that I can't reproduce this problem without nix so perhaps it's the interaction of this flag with another flag which causes the issue. I am trying various incantations of cabal install with options such as --enable-library-for-ghci and the library build products look the same as the nix build products but the libraries are still fast to load into ghci.

@mpickering
Copy link
Contributor

Indeed, I can now reproduce the problem without nix at all. You also need --enable-split-sections.

@mpickering
Copy link
Contributor

If anyone is interested by investigation and minimal reproduction are here

@peti
Copy link
Member

peti commented Apr 12, 2019

OK, the new compiler built successfully by now and I tested it too. Everything works as expected. The doctest issue is gone, too. 👍 I pushed the patch to master a minute ago.

@cdepillabout
Copy link
Member Author

@peti Thanks a lot for your help with this one!

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

Successfully merging this pull request may close these issues.

None yet

5 participants