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

Jump to bottom

haskell-generic-builder: Add option to disable library-for-ghci #58743

Closed
wants to merge 8 commits into from
Closed

haskell-generic-builder: Add option to disable library-for-ghci #58743

wants to merge 8 commits into from

Conversation

cdepillabout
Copy link
Member

Motivation for this change

This PR adds an option to the Haskell generic-builder that can be used to disable the library-for-ghci option passed to Setup configure.

By default, the Haskell generic-buider enables library-for-ghci. This causes Setup build to build an additional HSfoo*.o file.

It looks like this --enable-library-for-ghci flag was added in d7beae3, but there doesn't seem to be any explanation of why it was added.

I'm seeing a problem when trying to use doctest to test a Haskell package:

cdepillabout/termonad#15

doctest uses the GHCi API to load each module in your package into GHCi, and then run tests written in your Haddocks. I'm seeing a problem using doctest to load files that require TemplateHaskell.

It looks like what happens is that in order for GHCi to typecheck and load the modules in a given package, it needs to byte-compile the TemplateHaskell splices. In order to do this, it needs to load all the required dependency packages. For some reason, when libraries have been compiled with --enable-library-for-ghci, this takes an extremely long time. The Termonad example from above takes about 3 hours to run doctest, even though running it through stack or cabal just takes a few seconds.

I'm not sure why it takes so long to load libraries when using --enable-library-for-ghci, but I have confirmed that using --disable-library-for-ghci only takes a few seconds.

We're seeing this at work with our own Haskell packages, so we'd really like an easy way to work around this. I'd also like to backport this change to 19.03, since that is what we're using at work.

This PR adds an option to disable library-for-ghci for the Haskell generic-builder.

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.

luispedro and others added 8 commits March 29, 2019 16:02
@luispedro
ghc: make disableLargeAddressSpace configurable
5dbb3eb 
Even on Linux/NixOS, it can be useful to have a compiler without the
large address space as it creates issues in some contexts.
@peti
hackage-packages.nix: automatic Haskell package set update
d3bb814 
This update was generated by hackage2nix v2.14.2-4-gd25a3c5 from Hackage revision
commercialhaskell/all-cabal-hashes@47088e8.
@peti
Merge pull request NixOS#58547 from luispedro/ghc_disableLargeAddress…
7f9e46e 
…Space

ghc: make disableLargeAddressSpace configurable
@peti
hackage-packages.nix: automatic Haskell package set update
e7db376 
This update was generated by hackage2nix v2.14.2-4-gd25a3c5 from Hackage revision
commercialhaskell/all-cabal-hashes@17a371c.
@peti
hackage-packages.nix: automatic Haskell package set update
7e28313 
This update was generated by hackage2nix v2.14.2-4-gd25a3c5 from Hackage revision
commercialhaskell/all-cabal-hashes@1d1f033.
@peti
hackage-packages.nix: automatic Haskell package set update
f9bbc35 
This update was generated by hackage2nix v2.14.2-4-gd25a3c5 from Hackage revision
commercialhaskell/all-cabal-hashes@ac4eded.
@peti
hackage-packages.nix: automatic Haskell package set update
af89d95 
This update was generated by hackage2nix v2.14.2-4-gd25a3c5 from Hackage revision
commercialhaskell/all-cabal-hashes@5fa6445.
@cdepillabout
Add option to disable library-for-ghci in the generic haskell builder.
b58ecf3
@cdepillabout
Copy link
Member Author

I'd like to give a little bit more of the information I've found out about this when digging into it.

GHC

The GHC documentation says the following about library-for-ghci and these .o files (paraphrased):

  • To load a package foo, GHCi can load its libHSfoo.a library directly.
  • However, it can also load a package in the form of a single pre-linked HSfoo.o file
  • Loading the .o file is slightly quicker, but compiling it and keeping it around takes time and disk space.
  • .o file is built by Cabal automatically; use --disable-library-for-ghci to disable it.

Cabal

The cabal documentation seems to say that --enable-library-for-ghci is enabled by default:

https://github.com/haskell/cabal/blob/a21ae5e3c1590a14e36eed2aff534233eda1d589/Cabal/doc/nix-local-build.rst#L1513-L1528

However, the actual logic in cabal for deciding whether to pass this flag or not seems somewhat complicated. I haven't done a deep dive into the cabal codebase, but from playing around with cabal-install it, it appears that cabal new-build passes --disable-library-for-ghci on the command line by default.

Stack

As far as I can tell, stack also doesn't build the HSfoo.o file.

@cdepillabout
Copy link
Member Author

If anyone wants to try to reproduce this, you can do so with the Termonad repository. You can run the following commands:

$ git clone git@github.com:cdepillabout/termonad.git
$ cd termonad
$ git checkout show-problem-with-doctest
$ nix-build

Running the doctests for the termonad derivation should take a few hours. Instead, if you build termonad with stack, the doctests should only take a few seconds to run.

@cdepillabout cdepillabout mentioned this pull request Apr 2, 2019
Closed
@peti
Copy link
Member

peti commented Apr 2, 2019

Have you actually tested whether compiling with --disable-library-for-ghci solves the doctest issue?

@peti peti force-pushed the haskell-updates branch 2 times, most recently from 2afaeb9 to 1da98bb Compare April 2, 2019 07:53
@cdepillabout
Copy link
Member Author

cdepillabout commented Apr 2, 2019 •
edited

@peti, thanks for the quick response.

Have you actually tested whether compiling with --disable-library-for-ghci solves the doctest issue?

Sorry, I should have made this clear. I have tested locally that compiling with --disable-library-for-ghci (or even just commenting out --enabling-library-for-ghci) solves the issue with doctest.

Hmm, it looks like something strange happened with the history of this PR when you force-pushed the haskell-updates branch. I can rebase this commit on top of the current haskell-updates branch if that helps.

@peti peti changed the base branch from haskell-updates to master April 2, 2019 09:30
@peti peti changed the base branch from master to haskell-updates April 2, 2019 09:30
@peti
Copy link
Member

peti commented Apr 2, 2019

Merged in 780f8057991cf5bd1e701632c4e5c0d2090700c3. It will go to master once the test builds have succeeded. Thank you very much!

Now, the next step is to decide whether to disable this feature by default. ❓

@peti peti closed this Apr 2, 2019
peti pushed a commit that referenced this pull request Apr 2, 2019
@cdepillabout @peti
Add option to disable library-for-ghci in the generic haskell builder.
a5b8522 
Closes #58743.
@cdepillabout cdepillabout mentioned this pull request Apr 8, 2019
Merged
10 tasks
@cdepillabout cdepillabout deleted the disable-library-for-ghci branch April 11, 2019 03:47
@cdepillabout cdepillabout mentioned this pull request Apr 11, 2019
Merged
10 tasks
peti pushed a commit that referenced this pull request Apr 11, 2019
@cdepillabout
haskell-generic-builder: disable library-for-ghci by default
0698b54 
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 #58743.
peti pushed a commit that referenced this pull request Apr 11, 2019
@cdepillabout @peti
haskell-generic-builder: disable library-for-ghci by default
bf78396 
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 #58743.
peti pushed a commit that referenced this pull request Apr 11, 2019
@cdepillabout @peti
haskell-generic-builder: disable library-for-ghci by default
e55c939 
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 #58743.
peti pushed a commit that referenced this pull request Apr 12, 2019
@cdepillabout
haskell-generic-builder: disable library-for-ghci by default
353e200 
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 #58743.

(cherry picked from commit 0698b54)
vdemeester pushed a commit to vdemeester/nixpkgs that referenced this pull request Apr 12, 2019
@cdepillabout @peti
haskell-generic-builder: disable library-for-ghci by default
757099e 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basvandijk basvandijk Awaiting requested review from basvandijk

@peti peti Awaiting requested review from peti

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cdepillabout @peti @GrahamcOfBorg @luispedro