Skip to content

haskell.compiler: use wrapper scripts over propagating#147361

Merged
sternenseemann merged 7 commits intoNixOS:haskell-updatesfrom
sternenseemann:wrap-ghc-more-tightly
Nov 25, 2021
Merged

haskell.compiler: use wrapper scripts over propagating#147361
sternenseemann merged 7 commits intoNixOS:haskell-updatesfrom
sternenseemann:wrap-ghc-more-tightly

Conversation

@sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Nov 25, 2021 •
edited
Loading

Motivation for this change

Currently the C compiler and LLVM binaries used by GHC are propagatedBuildInputs. This has several downsides:

  • If used outside of stdenv or nixpkgs, propagation is not respected and these binaries will not be in PATH — and GHC won't work.
  • The precedence rules of propagation may cause (especially) the wrong LLVM version to leak into a build environment:
    • while bootstrapping GHC, the build->target LLVM version takes precedence over the LLVM version propagated by
      the binary GHC, causing a lot of warnings about unsupported LLVM versions to show up.
    • The LLVM version used by GHC leaks into Haskell derivations with higher precedence than one passed directly. This causes e. g. llvm-hs 9.0.1 to fail on aarch64-linux when compiling with a GHC which uses the LLVM backend and a more recent LLVM version.

TODO:

  • sanity checking (ideally especially on aarch64-linux)
  • applying the changes to the other GHCs
  • Adjust ghcWithPackages wrapper for the fact that LLVM only needs to be injected after explicit request now
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Nov 25, 2021
@sternenseemann
Copy link
Member Author

sternenseemann commented Nov 25, 2021

@ofborg build haskell.compiler.ghc8107

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 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. labels Nov 25, 2021
@sternenseemann
haskell.compiler.ghc*Binary: don't propagate LLVM, use wrapper
579bc49 
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.
@sternenseemann
haskell.compiler.ghc*Binary: add all missing runtimeDeps to PATH
5384a35 
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`).
@sternenseemann
Copy link
Member Author

sternenseemann commented Nov 25, 2021

@ofborg build haskell.compiler.ghc865Binary haskell.compiler.ghc8102Binary haskell.compiler.ghc8107Binary pkgsMusl.haskell.compiler.gh8102Binary pkgsMusl.haskell.compiler.ghc8107Binary

@sternenseemann
haskell.compiler.*: prefix PATH with runtimeDeps
035f20b 
This will prevent freak accidents where the wrong tools are used because
they are in PATH by chance.
@sternenseemann
haskell.compiler.*: move propagatedBuildInputs into runtimeDeps
f5c3b65 
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.
@sternenseemann
haskell.compiler.*: use isScript over grepping for #!
a7c5645
@sternenseemann
Copy link
Member Author

sternenseemann commented Nov 25, 2021

@ofborg build haskell.packages.ghc901.hello

@sternenseemann sternenseemann marked this pull request as ready for review November 25, 2021 17:41
@sternenseemann
ghcWithPackages: don't wrap GHC with LLVM unnecessarily
50f256f 
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).
@sternenseemann
haskell.compiler.*: don't useLLVM if aarch64-darwin NCG is available
ef081bf 
aarch64-darwin NCG was added in 9.2.1 which makes it unnecessary to
include LLVM in the wrapper.
Ericson2314
Ericson2314 approved these changes Nov 25, 2021
View reviewed changes
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@sternenseemann sternenseemann merged commit 7ee7afe into NixOS:haskell-updates Nov 25, 2021
@sternenseemann sternenseemann deleted the wrap-ghc-more-tightly branch November 25, 2021 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ericson2314 Ericson2314 Ericson2314 approved these changes

@prusnak prusnak Awaiting requested review from prusnak

@guibou guibou Awaiting requested review from guibou

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@maralorn maralorn Awaiting requested review from maralorn

@domenkozar domenkozar Awaiting requested review from domenkozar

@expipiplus1 expipiplus1 Awaiting requested review from expipiplus1

Assignees

No one assigned

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sternenseemann @Ericson2314