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

ghc: support building with integer-simple #22121

Closed
wants to merge 2 commits into from

Conversation

basvandijk
Copy link
Member

If the flag enableIntegerSimple is true GHC will be build with the GPL-free but slower integer-simple library instead of the faster but GPLed integer-gmp library.

@basvandijk basvandijk force-pushed the ghc-integer-simple branch 2 times, most recently from a6a23a3 to 2d309e3 Compare January 25, 2017 08:52
@basvandijk
Copy link
Member Author

I also added some documentation to the manual.

@basvandijk
Copy link
Member Author

A few questions:

  • Should we give names to the compilers build with integer-simple? Advantages are that we can then easily build them on hydra and users can more easily select those compilers without resorting to overriding. Maybe we can use a naming scheme like: pkgs.haskell.compiler.ghc802.integer-simple or pkgs.haskell.compiler.integer-simple.ghc802.

  • Secondly, should we also provide package sets for these compilers?

  • Finally, should we build these compilers and/or package sets on hydra or will this increase the load to much?

@peti any ideas?

If the flag enableIntegerSimple is true GHC will be build with the
GPL-free but slower integer-simple library instead of the faster but
GPLed integer-gmp library.
@basvandijk
Copy link
Member Author

I pushed another commit that answers my first and second questions. I provide convenient names ro get a GHC compiler build with integer-simple and to get a package set supporting integer-simple:

The attribute pkgs.haskell.compiler.integer-simple."${ghcVersion}" provides a GHC compiler build with integer-simple.

Similarly, the attribute pkgs.haskell.packages.integer-simple."${ghcVersion}" provides a package set supporting integer-simple.

The attribute `pkgs.haskell.compiler.integer-simple."${ghcVersion}"`
provides a GHC compiler build with `integer-simple`.

Similarly, the attribute `pkgs.haskell.packages.integer-simple."${ghcVersion}"`
provides a package set supporting `integer-simple`.
@peti
Copy link
Member

peti commented Jan 31, 2017

This is some fine work. Thank you very much for the contribution.

I've merged the patches to haskell-updates. This means that Hydra will test-build everything at http://hydra.nixos.org/jobset/nixpkgs/haskell-updates. I'll merge the patches to master in a day or two -- assuming that the changes don't break the compiler builds. :-)

@basvandijk
Copy link
Member Author

Thanks Peter.

Do you think it makes sense to set recurseForDerivations = true in pkgs.haskell.compiler.integer-simple causing the underlying compilers to be build by hydra?

Offtopic: I would also prefer to have a binary cache available for packages build with profiling (enableLibraryProfiling = true;). At LumiGuide we currently have a hydra job which builds all our Haskell packages and their dependencies with profiling enabled in case we quickly need to profile something.

@vcunat
Copy link
Member

vcunat commented Feb 1, 2017

The jobset status looks OK. Mergeable then?

(I'd like all larger rebuilds merged now to staging so it can be stabilized without too much rebuilding.)

@peti
Copy link
Member

peti commented Feb 1, 2017

Do you think it makes sense to set recurseForDerivations = true in pkgs.haskell.compiler.integer-simple causing the underlying compilers to be build by hydra?

I pushed a commit that does that in 47a604a41807dd4221a354a095715c7320008d6e. I think we'll use that for running test builds right now, but it's probably overkill to enable those builds on hydra.nixos.org for all GHC versions we have. Who needs GHC 7.2.2 anyway -- with or without gmp? GHC is relatively expensive to compile and to store (the resulting package is >1 GB), so I'd rather be cautious about enabling too many builds that we probably don't need anyway. I do think that it is appropriate to enable the gmp-less variant of the latest ghc in pkgs/top-level/release.nix though.

I would also prefer to have a binary cache available for packages build with profiling (enableLibraryProfiling = true;).

#22340

@peti
Copy link
Member

peti commented Feb 1, 2017

@vcunat, give us one more day, please. I have one minor change I'd like to see implemented before we commit.

@@ -1,6 +1,16 @@
{ pkgs, callPackage, stdenv, buildPlatform, targetPlatform }:

rec {
let # These are the GHC versions that support building with integer-simple.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list contains every compiler we have except 7.0.4, I believe. Assuming that newer versions released in the future will probably support this feature, too, would it make sense to invert this logic, i.e. to list those compilers that don't support a gmp-less build? That would make this list shorter and it would remove the need to do manual maintenance in the future because new compiler versions are implicitly supported the moment we add them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peti something like this:

LumiGuide@ae72fef

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool, thank you. Added to haskell-updates in a7fffbf808572d1afd40a10f644eb213116a3bad.

@vcunat
Copy link
Member

vcunat commented Feb 1, 2017

OK, in case Hydra gets idle, we can "build staging first" and eventually even exchange the merge order.

@peti
Copy link
Member

peti commented Feb 2, 2017

The integer-simple variant of GHC 7.6.3 does not compile: http://hydra.nixos.org/build/47389866/nixlog/1/raw. There is some code that expects functions like divInteger which appear to be missing.

@peti
Copy link
Member

peti commented Feb 2, 2017

@vcunat, dfcc9e2 and 75a4679 pushed to staging.

@basvandijk
Copy link
Member Author

@peti regarding GHC-7.6.3, maybe it's best to put "ghc763" in integerSimpleExcludes.

@vcunat vcunat closed this in dfcc9e2 Feb 4, 2017
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.

4 participants