-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
haskellPackages: GHC 9.10 fix, infrastructure improvements #315167
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we would set GHC_PACKAGE_PATH after configure, the reasons being that 1. Setup.hs configure forbids this from being set since it can make a build fail that would otherwise succeed (since it influences how GHC behaves when invoked by Cabal). 2. Setting GHC_PACKAGE_PATH being set is sound in our case, since we set it precisely to the packages available to Cabal at configure time, so there should be no room for a mismatch. 3. Some test suites require GHC_PACKAGE_PATH or GHC_ENVIRONMENT to be set, so they can invoke GHC(i) with build dependencies available. Cabal >= 3.12 forbids GHC_PACKAGE_PATH from being set after <haskell/cabal@d6e38041a7c778fadf8f416>. Setting GHC_ENVIRONMENT would be possible, but is cumbersome without cabal-install (which has the handy cabal exec command which takes care of that). Additionally, it is not clear if it'll remain possible to do that: <haskell/cabal#7792>. Our solution to Cabal 3.12's change is to be more targeted about setting GHC_PACKAGE_PATH: We _just_ set it for the actual test suite executable. This can be achieved by using --test-wrapper which when given is invoked by Cabal to run the test suite. Here we can set any environment variables after Cabal has already done its environment checks. As long as we don't do anything stupid, this should be unproblematic. Users can also arbitrarily influence what GHC_PACKAGE_PATH will contain using the NIX_GHC_PACKAGE_PATH_FOR_TEST environment variable. This is un(der)documented for now, since I want to keep some wiggle room for changing stuff in the coming weeks. Also it's rarely necessary to actually touch this variable.
elfutils is used in the RTS (rts/Libdw.c), i.e. it will be used on the target platform. Tested via pkgsCross.gnu32.haskellPackages.ghc [1], though #304605 needs to be cherry-picked for elfutils to build. [1]: nix-shell -E 'with import ./. { crossSystem = "i686-linux"; }; mkShell { nativeBuildInputs = [haskellPackages.ghc ]; }'
Patches `crypton-x509-system` to use the full path to the `security` binary.
This makes `justStaticExecutables` error if the produced store path contains references to GHC. This is almost always erroneous and due to the generated `Paths_*` module being imported. This helps prevent `justStaticExecutables` from producing binaries with closure sizes in the gigabytes. See: #164630 Co-authored-by: sternenseemann <sternenseemann@systemli.org>
This change ensures that packages won't be rebuild compared to before the introduction of disallowedRequisites/disallowGhcReference unless they use one of those arguments. It may be nice to revert this in the future (via staging) for greater simplicity, but will help with initial regression testing.
github-actions
bot
added
6.topic: nixos
Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
6.topic: haskell
8.has: documentation
8.has: changelog
labels
May 27, 2024
I've rescheduled the remaining builds now. We'll have to keep an eye peeled for regressions. |
After 120f242, GHC_PACKAGE_PATH isn't set implicitly in installPhase anymore. Instead we achieve the same by telling the Makefile the exact ghc command line to use. As a benefit, we can now cleanly separate build and host in this case: We used to (implicitly) reuse the host package db. Now we can explicitly request the package db also used for building Setup.hs.
W.r.t. regressions affecting aarch64-darwin/ |
It is somewhat curious that it behaves differently exclusively here, but I don't think it is necessary to stop shipping a package due to floating point arithmetic error—it would be unreasonable to assume there were none… See ekmett/ad#113.
ofborg
bot
added
the
2.status: merge conflict
This PR has merge conflicts with the target branch
label
May 31, 2024
ofborg
bot
removed
the
2.status: merge conflict
This PR has merge conflicts with the target branch
label
May 31, 2024
This ports our infamous patch for Cabal that prevents certain parts of the Paths_* module from being generated in order to prevent unnecessary references on aarch64-darwin, to GHC >= 9.10. See also: - Original issues: #140774 - Patches - Original patch for GHC >= 8.10 && < 9.2 / Cabal >= 3.2 && < 3.6: b0dcd7f - Patch for GHC >= 9.2 && < 9.10 / Cabal >= 3.6 && < 3.12: #216857, f6f780f129f50df536fb30, …
Edit: See #318013. Regressions from #304352 on aarch64-darwin. |
Remove references to `hpack` and `distribution-nixpkgs` paths so that `cabal2nix` can build on macOS. See: #304352
I suspect that we'll be able to upgrade to 9.10.* for all three packages after the next haskell-language-server update. I'll leave that to maralorn.
Since emanote incurs a reference on GHC on all platforms this override is all but uselless.
This matches what we do for GHC 8.10.7 where we also can't build the 9.6 versions.
This just requires picking the right version of the package for all compiler versions.
elmPackages.elm: fix build failure on darwin
Fixes `cabal-install` to remove references to GHC, mostly through other libraries that are included in the binary. See: #304352 Co-authored-by: sternenseemann <sternenseemann@systemli.org>
All these references would (indirectly) incur a GHC requisite which is prevented by #304352 via justStaticExecutables. Consequently, we can stop setting disallowedReferences. As it turns out the references we were removing aren't currently created, so our life gets even easier.
Since it incurs a GHC reference anyways, we have to disable justStaticExecutables!
- Address some removals. - Add all missing packages that use justStaticExecutables.
11 tasks
This commit disables justStaticExecutables for packages on aarch64-darwin that incur a (usually incorrect) reference on GHC. justStaticExecutables now fails when a GHC reference remains in the output (#304352). Due to this reference justStaticExecutables (before these changes) would only marginally reduce closure size. In the future, we'll be able to re-introduce justStaticExecutables after (manually) removing the incorrect references stemming from the use of Paths_* modules. Tracking Issue: #318013
This was referenced Jun 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
6.topic: haskell
6.topic: nixos
Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
8.has: changelog
8.has: documentation
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+
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #312934.
No package updates in this one since we have some mass-rebuild changes that need testing before we can land them.
Currently the eval is mostly cancelled on Hydra as we have a lot of builds for 23.11/24.05 scheduled at the moment.Builds on haskell-updates.