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.

Sign up for GitHub

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

ghc: 8.10.4 -> 8.10.6 #126195

Merged
merged 2 commits into from
Aug 16, 2021
Merged

ghc: 8.10.4 -> 8.10.6 #126195

merged 2 commits into from
Aug 16, 2021

Conversation

sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Jun 8, 2021 •
edited
Loading

Motivation for this change

Release Notes

Main interest for us apart from general bug fixes is the ability to target aarch64-darwin which nixpkgs/NixOS supports in a limited capacity as of 21.05.

However, GHC 8.10.5 has an issue in RTS which breaks (presumably) haskell-language-server and doctest-based testsuites. Since we generally execute test suites, this causes severe regressions in haskellPackages. However, it looks like we can apply an upstream patch to remedy the situation.

The following things would need to be done to make it viable to upgrade our GHC:

  • Apply the RTS patch export allocateWrite OR upgrade to 8.10.6 when it gets released
  • Package binary GHC 8.10.5 (or 8.10.6) to be able to natively bootstrap aarch64-darwin GHC and enable that platform for haskellPackages.

In its current state the PR may allow cross compiling for aarch64-darwin (but I haven't tested this) which may be useful for interested parties. For nixpkgs itself this is not interesting since we don't rely on cross compilation to provide packages for that platform (except for the bootstrap).

I'm not sure how much I'll be work on GHC 8.10.5 in the near future — I have no stake in aarch64-darwin and no way of testing it, so it doesn't really make sense for me. If someone else wants to take point on this, I'll be happy to review PRs target towards haskell-updates-ghc-8.10.5.

When GHC 8.10.6 gets released, I'm interested in testing it on haskell-updates with the help of Hydra.

cc @TerrorJack @domenkozar @NixOS/haskell

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@domenkozar domenkozar marked this pull request as ready for review June 8, 2021 11:52
@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Jun 8, 2021
@ofborg ofborg bot requested review from MarcWeber, peti and kosmikus June 8, 2021 12:00
@sternenseemann sternenseemann mentioned this pull request Jun 8, 2021
Merged
7 tasks
@sternenseemann sternenseemann marked this pull request as draft June 8, 2021 13:29
@sternenseemann
Copy link
Member Author

With the patch applied we can build swagger2 which was failing previously due to doctest issues:

$ nix-build -A haskellPackages.swagger2
/nix/store/m4f1ppcmps8d5vmvm0iznrr93idr6a88-swagger2-2.6

@domenkozar domenkozar mentioned this pull request Jun 26, 2021
Closed
@lf-
Copy link
Member

lf- commented Jul 3, 2021

Would it be possible to release this and later add aarch64-darwin, or is there a concern about doing a big hydra run and not getting that while we're at it?

@maralorn
Copy link
Member

maralorn commented Jul 3, 2021

No, the big hydra run is generally not a blocker. And we don‘t need to wait for the aarch64 support.

But last time we tried ghc 8.10.5 it had some problems breaking some packages and it was said that 8.10.6 will come soon within a week or so. That’s why we decided to not push this further without a good reason.

Is there a special reason why you‘d want 8.10.5 soon?

@lf-
Copy link
Member

lf- commented Jul 3, 2021

No, the big hydra run is generally not a blocker. And we don‘t need to wait for the aarch64 support.

But last time we tried ghc 8.10.5 it had some problems breaking some packages and it was said that 8.10.6 will come soon within a week or so. That’s why we decided to not push this further without a good reason.

Is there a special reason why you‘d want 8.10.5 soon?

Our project is hit by a compilation perf bug that it fixes, an interaction with Yesod forms causes one of our files to take multiple minutes to compile, only when optimizations are on on affected versions. It's not, I guess, super critical, in that we can continue avoiding upgrading to 8.10 series and don't need any of the features immediately, so I'm not meaning to rush.

cc @gleachkr

@msfjarvis
Copy link
Contributor

Is there any work being done to package GHC 8.10.5 as binary or has that been deferred to the 8.10.6 release?

@maralorn
Copy link
Member

maralorn commented Jul 5, 2021

No one working on it as far as I know. Waiting for 8.10.6 makes sense. But I don‘t think it would be that hard.

@angerman
Copy link
Contributor

@bgamari's https://github.com/bgamari/nixpkgs/archive/faf319a81bf.tar.gz, contains a haskell.compiler.ghc8105Binary
for the really curious.

@domenkozar
Copy link
Member

Watch closely https://gitlab.haskell.org/ghc/ghc/-/issues/19987 how release progresses.

@sternenseemann sternenseemann changed the title ghc: 8.10.4 -> 8.10.5 ghc: 8.10.4 -> 8.10.6 Aug 14, 2021
@sternenseemann @TerrorJack
ghc: 8.10.4 -> 8.10.6
cb330ce 
The only big change is required for darwin since GHC 8.10.5 now
runs xattr in the install phase on darwin:

* https://gitlab.haskell.org/ghc/ghc/-/commit/11e1dcde0d3137a8ee5cd32697b2925ea4e84b95
* https://gitlab.haskell.org/ghc/ghc/-/commit/ec451cac39de919621c783fec809c26fb9f0d1c8

Unfortunately, it uses the host /usr/bin/xattr by default which is
present in the build due to a lack of sandboxing on darwin. That xattr
version however still requires Python 2.7 whereas Python 3.8 is in PATH
in our build. We solve this by setting the XATTR environment variable.

We can't use python3Packages.xattr since GHC expects Apple's fork of
xattr which provides some extra flags to utilize.

Co-authored-by: Cheng Shao <cheng.shao@tweag.io>
@sternenseemann sternenseemann marked this pull request as ready for review August 14, 2021 17:18
@sternenseemann
Copy link
Member Author

sternenseemann commented Aug 14, 2021 •
edited
Loading

Built 8.10.6 and quite a few packages on x86_64-linux and x86_64-darwin, so far so good.

If @maralorn is up for it, we could try doing a set rebuild on haskell-updates.

@maralorn
Copy link
Member

I have kinda delayed a bump. I think the best would be to merge the current branch and then do the set rebuild.

@maralorn
Copy link
Member

So I can merge this tomorrow.

SuperSandro2000
SuperSandro2000 reviewed Aug 14, 2021
View reviewed changes
pkgs/os-specific/darwin/xattr/default.nix
license = [ licenses.psfl licenses.mit ]; # see $doc/share/xattr/OpenSourceLicenses
maintainers = [ maintainers.sternenseemann ];
homepage = "https://opensource.apple.com/source/python_modules/";
platforms = lib.platforms.darwin;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
platforms = lib.platforms.darwin;
platforms = platforms.darwin;

@maralorn maralorn merged commit d5f8216 into haskell-updates Aug 16, 2021
@cdepillabout cdepillabout deleted the haskell-updates-ghc-8.10.5 branch August 17, 2021 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@guibou guibou guibou approved these changes

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@maralorn maralorn Awaiting requested review from maralorn

@MarcWeber MarcWeber Awaiting requested review from MarcWeber

@peti peti Awaiting requested review from peti

@kosmikus kosmikus Awaiting requested review from kosmikus

@expipiplus1 expipiplus1 Awaiting requested review from expipiplus1

Assignees
No one assigned
Labels
6.topic: haskell 8.has: clean-up 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sternenseemann @lf- @maralorn @msfjarvis @angerman @domenkozar @SuperSandro2000 @guibou