Skip to content

GHC 8.4: compiler and package fixes#34023

Merged
peti merged 1 commit intoNixOS:masterfrom
deepfire:x-ghc-8.4-master
Jan 19, 2018
Merged

GHC 8.4: compiler and package fixes#34023
peti merged 1 commit intoNixOS:masterfrom
deepfire:x-ghc-8.4-master

Conversation

@deepfire
Copy link
Copy Markdown
Contributor

Motivation for this change

This provides GHC 8.4.1-alpha in Nixpkgs, along with many necessary package overrides.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@deepfire deepfire requested a review from peti as a code owner January 18, 2018 20:51
@deepfire
Copy link
Copy Markdown
Contributor Author

cc @peti

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 18, 2018
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment makes me suspicious. :-) Did you actually check which core libraries GHC 8.4.x provides and adopt this list accordingly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@peti, pushed an attempt at this update. One question is -- what is bin-package-db -- did previous GHC's indeed used to have this package or not? I retained its entry, because it sounded magical enough..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This override is actually necessary???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've started with the ghcHEAD definition and extended that. So I didn't really check the inherited old entries.

What do you think is the best way to go forward?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The problem is that we end up having a ton of overrides with no documentation, no ticket URLs, not explanation of why they exist ... so who is ever going to clean that up?

Personally, I'd start with an empty configuration-ghc-8.4.x.nix file and add only those overrides that I find are actually required. It's also a good idea to add comments every now and then to document why you're adding the overrides you do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed.. I was planning to track a short one-liner explanation in the Elaborate Set of Bash Scripts, so that information is available.

Thank you!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't you use doJailbreak?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I use an Elaborate Set of Bash Scripts to generate the list of overrides, and this has consequences, that adding a line entry is easier than adding a wrapping set of parentheses..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just out of curiosity, how did those shell scripts know which version to pick for overriding blaze-markup etc.?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@peti, the figuring out was manual work, of course -- but the maintenance (hash updates, state changes) is supported by those scripts.

There is an angle to this, however -- the dataset is structured in such a way, that paring the set of overrides down automatically is not infeasible.

@deepfire deepfire force-pushed the x-ghc-8.4-master branch 4 times, most recently from 137ec1d to 9f0a492 Compare January 19, 2018 11:06
@peti
Copy link
Copy Markdown
Member

peti commented Jan 19, 2018 via email

Copy link
Copy Markdown
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

I would be happy to merge a version of this PR that adds the compiler plus an empty configuration-ghc-8.4.x.nix file that defines nothing but the core packages. The version with all those overrides defined, however, scares me because it's seems like a really bad idea totally intransparent where these settings come from and how they are going to be maintained in the future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't you use self.blaze-markup_0_8_2_0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oversight : -)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I really don't like these overrides because they unconditionally override whatever version hackage-packages.nix provides -- even if the version in there is actually newer than the one we hard-code here. I'd very much prefer to have these overrides import the relevant patches, because then we'd at least notice when they cease to apply to the upstream version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Completely agreed -- my plan was to submit early, and then pare down the Hackage-sourced overrides, as they're overtaken by the hackage-packages regeneration.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't seem like a good idea.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Whoops, thank you! It's a bit odd the resulting package set works here.. Hmm.

@deepfire
Copy link
Copy Markdown
Contributor Author

deepfire commented Jan 19, 2018

@peti, done -- the PR now has only the minimum required to provide the following experience:

deepfire@andromedae:~/nixpkgs$ nix-shell -p haskell.compiler.ghc841

[nix-shell:~/nixpkgs]$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.4.20180115

I hope to work with you to submit the rest in a subsequent PR.

@peti peti merged commit 766bfd6 into NixOS:master Jan 19, 2018
@peti
Copy link
Copy Markdown
Member

peti commented Jan 19, 2018

Yes, I am curious to play with 8.4.x, too! I'd say let's add overrides incrementally and make sure each of them is (a) necessary, (b) known to upstream, and (c) somewhat documented. :-) I'll enable a bunch of Hydra builds for 8.4.x at https://hydra.nixos.org/jobset/nixpkgs/haskell-updates so that we can get a feel for the general state of the package set. That jobset builds the haskell-updates branch of https://github.com/peti/nixpkgs where I usually push all changes for testing before they go to master.

# library instead of the faster but GPLed integer-gmp library.
enableIntegerSimple ? false, gmp ? null

, version ? "8.4.20180115"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nitpick, not sure how important this is, but shouldn't this be 8.4.0.20180115? (with the .0 between 8.4 and the date).

Copy link
Copy Markdown
Contributor Author

@deepfire deepfire Jan 23, 2018

Choose a reason for hiding this comment

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

I don't really have an opinion, having taken ghcHEAD (which currently carries "8.5.20180118") as basis..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The difference is that HEAD is going to be released as 8.6.1, so that new version number will be higher than 8.5.x no matter what "x" is. In case of the 8.4.1 pre-release, the final version is going to be 8.4.1, which will be a version that's lower than what we currently use for the alpha. Arguably, 8.4.0.x would be a better choice for the alpha.

Having said that, I don't really care about this issue, because I don't think it makes any difference in practice. :-) People who use the 8.4.1-alpha tend to know what they are doing and won't be confused by this issue, I think.

@deepfire deepfire mentioned this pull request Jan 30, 2018
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants