Skip to content

[haskell-updates] ghc: Backport compact unwind support#166571

Merged
sternenseemann merged 3 commits intoNixOS:haskell-updatesfrom
hercules-ci:ghc902-backport-compact-unwind
Apr 3, 2022
Merged

[haskell-updates] ghc: Backport compact unwind support#166571
sternenseemann merged 3 commits intoNixOS:haskell-updatesfrom
hercules-ci:ghc902-backport-compact-unwind

Conversation

@roberth
Copy link
Member

@roberth roberth commented Mar 31, 2022 •
edited
Loading

Motivation

  • Allow to fix C++ exception handling in GHC-linked programs.

  • Make sure inline-c-cpp actually works using its test suite

  • Solve a problem with hercules-ci-agent on darwin (tbd in upcoming release)

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin (already tested previously, just not these exact commits)
    • 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/)
  • 22.05 Release Notes (or backporting 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
    • (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 Mar 31, 2022
@roberth roberth mentioned this pull request Mar 31, 2022
Closed
13 tasks
@ofborg ofborg bot requested a review from guibou March 31, 2022 13:59
@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. labels Mar 31, 2022
@roberth roberth mentioned this pull request Mar 31, 2022
Merged
sternenseemann
sternenseemann reviewed Apr 1, 2022
View reviewed changes
@roberth roberth force-pushed the ghc902-backport-compact-unwind branch from 75ccbd1 to 8b78429 Compare April 1, 2022 14:00
@ofborg ofborg bot requested a review from sternenseemann April 1, 2022 14:14
@roberth roberth force-pushed the ghc902-backport-compact-unwind branch from 8b78429 to b9fa562 Compare April 1, 2022 14:55
@roberth roberth force-pushed the ghc902-backport-compact-unwind branch from b9fa562 to 35f137e Compare April 1, 2022 14:56
sternenseemann
sternenseemann reviewed Apr 2, 2022
View reviewed changes
Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

sternenseemann
sternenseemann reviewed Apr 2, 2022
View reviewed changes
pkgs/development/compilers/ghc/9.0.2.nix

patches = [
# Add flag that fixes C++ exception handling; opt-in. Merged in 9.4 and 9.2.2.
# https://gitlab.haskell.org/ghc/ghc/-/merge_requests/7423
Copy link
Member

@sternenseemann sternenseemann Apr 2, 2022

Choose a reason for hiding this comment

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

I left a comment on the ghc MR w.r.t. documentation, given that it's a full rebuild to update it, can you take a look at that?

Copy link
Member Author

@roberth roberth Apr 2, 2022

Choose a reason for hiding this comment

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

I've addressed that comment in an extra upstream commit, which is not part of the fetchpatch below, so no rebuild is required.

This technically leaves the feature undocumented in our build, but I don't expect any significant number of people to use the built documentation in the store, let alone a user who needs to use the feature.

@sternenseemann sternenseemann merged commit 038d0d8 into NixOS:haskell-updates Apr 3, 2022
@sternenseemann
Copy link
Member

sternenseemann commented Apr 3, 2022

@roberth Did you check that ghcHEAD is new enough to support the flag? If not we should probably take the opportunity to update it to current upstream HEAD.

@roberth
Copy link
Member Author

roberth commented Apr 4, 2022

@sternenseemann I actually assumed that it'd be new enough or that it'd be okay because it'd be updated somewhat frequently. That appears to have been a little presumptuous.

I've tried to build an updated ghcHEAD, but it fails to build jailbreak-cabal because of changes to the cabal file parser.
It's been moved to a seprate package, but also the api has changed.

[1 of 2] Compiling Main             ( Main.hs, dist/build/jailbreak-cabal/jailbreak-cabal-tmp/Main.o )

Main.hs:13:41: error:
    Variable not in scope:
      readGenericPackageDescription
        :: Verbosity -> String -> IO GenericPackageDescription
    Suggested fix:
      Perhaps use one of these:
        data constructor ‘GenericPackageDescription’ (imported from Distribution.PackageDescription),
        ‘ppGenericPackageDescription’ (imported from Distribution.PackageDescription.PrettyPrint),
        ‘showGenericPackageDescription’ (imported from Distribution.PackageDescription.PrettyPrint)
   |
13 | main = getArgs >>= mapM_ (\cabalFile -> readGenericPackageDescription silent cabalFile >>= writeGenericPackageDescription cabalFile . stripVersionRestrictions)
   |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I've made a start https://github.com/NixOS/nixpkgs/compare/ghcHEAD-update?expand=1 but it's not something I can actually carry out now, so I don't think a PR should have my name on it :)

@sternenseemann
Copy link
Member

sternenseemann commented Apr 4, 2022

Okay, so this is going to be a side quest where I have to make a new jailbreak-cabal release first, thanks for looking into it!

@sternenseemann sternenseemann mentioned this pull request Apr 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sternenseemann sternenseemann sternenseemann left review comments

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@maralorn maralorn Awaiting requested review from maralorn

@expipiplus1 expipiplus1 Awaiting requested review from expipiplus1

@guibou guibou Awaiting requested review from guibou

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@roberth @sternenseemann