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.

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

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

Conversation

roberth
Copy link
Member

@roberth roberth commented Mar 31, 2022

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.

@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
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.

@@ -182,6 +183,17 @@ stdenv.mkDerivation (rec {

outputs = [ "out" "doc" ];

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

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

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

@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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants