Skip to content

haskellPackages: improve LLVM handling for GHC#147260

Merged
sternenseemann merged 26 commits intomasterfrom
haskell-updates
Nov 28, 2021
Merged

haskellPackages: improve LLVM handling for GHC#147260
sternenseemann merged 26 commits intomasterfrom
haskell-updates

Conversation

@sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Nov 24, 2021

This Merge

This PR is the regular merge of the haskell-updates branch into master.

This branch is being continually built and tested by hydra at https://hydra.nixos.org/jobset/nixpkgs/haskell-updates. You may be able to find an up-to-date Hydra build report at cdepillabout/nix-haskell-updates-status.

We roughly aim to merge these haskell-updates PRs at least once every two weeks. See the @NixOS/haskell team calendar for who is currently in charge of this branch.

haskellPackages Workflow Summary

Our workflow is currently described in pkgs/development/haskell-modules/HACKING.md.

The short version is this:

  • We regularly update the Stackage and Hackage pins on haskell-updates (normally at the beginning of a merge window).
  • The community fixes builds of Haskell packages on that branch.
  • We aim at at least one merge of haskell-updates into master every two weeks.
  • We only do the merge if the mergeable job is succeeding on hydra.
  • If a maintained package is still broken at the time of merge, we will only merge if the maintainer has been pinged 7 days in advance. (If you care about a Haskell package, become a maintainer!)

This is the follow-up to #146621. Come to #haskell:nixos.org if you have any questions.

I'll focus on working on GHC in this iteration and maybe even skip bumping hackage. The things I'm looking into are:

  • Use latest supported LLVM version for every GHC
  • Simplify how llvmPackages is reexposed in the package set
  • Fix pkgsCross.*.buildPackages.ghc (is not a cross-compiler counter-intuitively)
  • Unify NCG-detection mechanism for withPackages and GHC derivations
  • Investigate if we can avoid propagating LLVM in the GHC derivations and instead add it to the wrapper
  • haskellPackages.{ghcWithPackages, ghcWithHoogle}: make overrideable  #146770

Fixes to existing packages / unbreaking packages are always welcome of course!

This means we only have to update the llvmPackages attribute in one
place now and should prevent situations like with 8.6.5 where different
versions would be used in the package set compared to the compiler
build.

Drop comments in the configuration-ghc-X.Y.x.nix files as well, since
LLVM version isn't tied to the compiler minor version at
all (e. g. 8.10.2 and 8.10.7 have different support ranges).
GHC 9.0.1 only supports LLVM 9 and spews a lot of warnings about LLVM 10
when using the LLVM backend atm.

See also: https://www.haskell.org/ghc/download_ghc_9_0_1.html
GHC 8.6.5 will always segfault on aarch64-linux and at this point
it's not realistic we'll ever fix this.
haskell.compiler: refactor LLVM handling, upgrade to latest LLVM version for each GHC version
@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Nov 24, 2021
@ofborg ofborg bot added the 8.has: clean-up This PR removes packages or removes other cruft label Nov 24, 2021
@ofborg ofborg bot requested review from domenkozar and prusnak November 24, 2021 14:13
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 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 Nov 24, 2021
Since we inherit the platform list from the bootstrap GHC, we get
differing lists depending on which platform we evaluate the platform
list on (depending on whether 8.10.2 or 8.6.5 is used). This leads to
Hydra thinking aarch64-linux is not supported as it evaluates on
x86_64-linux usually.
Copy link
Member

@prusnak prusnak left a comment

Choose a reason for hiding this comment

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

untested, but changes look good

@sternenseemann
Copy link
Member Author

Sadly upgrading LLVM to 12 for GHC 8.10.7 doesn't solve some instances of LLVM's opt failing during compilation for aarch64-darwin that were reported on Matrix. This seems to be happening when compiling haskellPackages.happy_1_19_12 which is required for ghcjs.

@ofborg ofborg bot requested a review from prusnak November 24, 2021 16:26
Copy the approach from the normal GHC derivations for adding an
`export PATH` into the scripts in `$out/bin` and use it to put the
specific LLVM version of the binary GHC into its PATH. This will
prevent the LLVM version of the GHC we are building later to take
precedence over the LLVM version this GHC needs.
This brings the binary GHCs on parity with the source built ones in
terms of the wrapper. The upshot of this is that compiling something
using the binary GHCs no longer depends on PATH being populated with
the tools included in stdenv at all. We can even test this by running
the installCheck with an empty environment (via `env -i`).
This will prevent freak accidents where the wrong tools are used because
they are in PATH by chance.
This has two main benefits:

* GHC will work reliably outside of stdenv, even when using -fllvm since
  everything it'll call at runtime will be provided in PATH via the
  wrapper scripts.

* LLVM will no longer leak into haskell packages' configure
  scripts. This was an issue with llvm-hs which fails to build if the
  LLVM version of the compiler since the propagatedBuildInputs of GHC
  take precedence over the nativeBuildInputs added in the derivation.
Since GHC now will have LLVM available when needed, we don't need to add
it in the wrapper anymore. It can still be added if NCG is available,
but -fllvm should be used (e. g. to work around an NCG bug).
aarch64-darwin NCG was added in 9.2.1 which makes it unnecessary to
include LLVM in the wrapper.
See the added comment in all-packages.nix for a more detailed
explanation. This makes the top-level GHC different from
haskellPackages.ghc (which is build->host and used for building the
package set), but more consistent with gcc, gnat etc.

Specifically, pkgsCross.${platform}.buildPackages.ghc will now be a
cross-compiler instead of a native build->build compiler.

Since this change has a slight chance of being disruptive, add a note to
the changelog.
…uitive-cross

ghc: make sure top level exposed GHC is always host->target
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Nov 25, 2021
@ofborg ofborg bot requested a review from guibou November 25, 2021 20:30
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. and removed 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 Nov 25, 2021
github-actions bot and others added 4 commits November 26, 2021 00:07
GHC can actually accept absolute paths for its runtime tools (except for
touch) at configure time which are then saved in
`$out/lib/ghc-${version}/settings`. This allows us to drop the wrapper
entirely if we assume that a POSIX compliant touch is in PATH when we
run GHC later.

The touch problem can presumably be fixed by either patching the
configure file of GHC (although we need to take care not to change the
touch GHC uses during its compilation) or messing with the settings file
after installation.

The rationale for dropping the wrapper PATH entry completely is that
it's always possible to invoke GHC via its library which will bypass the
wrapper completely, leading to subtly different behavior.

Binary GHCs are not touched in this commit, but ideally they'll get a
similar treatment as well, so they are more robust, although we
generally don't need to use them as a library.

Note that GHC 8.8.4 doesn't care about install_name_tool or otool, so
the respective environment variables are not set.
@zowoq
Copy link
Contributor

zowoq commented Nov 28, 2021

Sorry for asking here but matrix won't let me log in...

What's the best way of making sure that haskellPackages.ShellCheck always points to the latest version? IIRC there used to be a hack somewhere in the haskell tooling that did this but 0.8.0 has been out for three weeks and ShellCheck is still at 0.7.2?

@sternenseemann
Copy link
Member Author

@zowoq Seems like we used to delete the version constraint on ShellCheck coming from stackage. This was deleted in a later cleanup trimming down the sed expression.

Not having the latest version of something due to stackage is an annoyance with most packages that are “user facing”, so shellcheck, pandoc, xmonad etc. This should get better when we switch to stackage nightly again, but is an inherent flaw of our packaging approach.

Since we do have the latest version of every package available, there should be nothing stopping us from using them for the top level packages (except for XMonad which is a bit tricky) with a bit of manual effort.

These targets also have NCG support, but they are tested less (in fact
SPARC seems to be untested atm) and may have issues. In such cases being
able to fallback to -fllvm without rebuilding the compiler could be
useful. OTOH GHC will default to -fasm and the backends probably work
well enough in most cases.
@sternenseemann
Copy link
Member Author

Seems like everything worked out. Aside from some flaky test suites, there is only one regression, namely haskellPackages.risc386 on aarch64-linux which fails due to a crash (bug) in LLVM. This seems to be due to our upgrade to LLVM 12, not our changes to GHC's packaging which is great.

Some stuff I have in mind is still pending which I'll tackle some other time:

  • ghcWithPackages/ghcWithHoogle refactors ( haskellPackages.{ghcWithPackages, ghcWithHoogle}: make overrideable  #146770)
  • Set absolute paths in the binary GHCs' settings files as well (this is not urgent since our binary GHCs just need to be good enough to build the proper GHCs, but still nicer overall; since it's easy enough to test, it's just a matter of slipping in a set rebuild at some point (maybe even via staging?))
  • Have an absolute path for touch in settings (this is not supported by upstream at the moment, maybe we should try to get a patch in?)

Some of the changes here will be backported as #147404, but mostly the LLVM upgrades. 17b8b5a would be very cool to backport, but I'd like to see if there are any downstream issues first. Backporting after release has the issue that this is a subtle breaking change since LLVM would no longer be in PATH in derivations that use a GHC with LLVM, but not sure if anyone relies on this behavior.

@sternenseemann sternenseemann merged commit 2222809 into master Nov 28, 2021
@zowoq
Copy link
Contributor

zowoq commented Nov 28, 2021

Seems like we used to delete the version constraint on ShellCheck coming from stackage.

Knew it used to be around somewhere, thanks for finding it.

Since we do have the latest version of every package available, there should be nothing stopping us from using them for the top level packages (except for XMonad which is a bit tricky) with a bit of manual effort.

ShellCheck does have a top level package that installs justStaticExecutables haskellPackages.ShellCheck and docs/manpage.

PR: #147780

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

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants