Skip to content

Comments

Add cabal-paths patch for ghc 9.2.x#184041

Closed
sloane-shark wants to merge 7 commits intoNixOS:masterfrom
sloane-shark:fix/ghc923-cabal-paths-patch
Closed

Add cabal-paths patch for ghc 9.2.x#184041
sloane-shark wants to merge 7 commits intoNixOS:masterfrom
sloane-shark:fix/ghc923-cabal-paths-patch

Conversation

@sloane-shark
Copy link
Contributor

@sloane-shark sloane-shark commented Jul 30, 2022

Description of changes

I tweaked the cabal-paths.patch file included for GHC builds (see #140774) to work on the updated Cabal PathsModule code that is used in GHC 9.2.3+.

Tested by building and running haskell.packages.ghc923.ghcid.bin and haskell.packages.ghc923.ormolu.

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/)
  • 22.11 Release Notes (or backporting 22.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 Jul 30, 2022
@ofborg ofborg bot requested review from expipiplus1 and guibou July 30, 2022 21:21
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jul 30, 2022
@sloane-shark
Copy link
Contributor Author

nixpkgs-review pr 184041 failed on the jacinda package, but i tested on master and it fails for the same reason. seems like this should be marked as broken on (at least) aarch64-darwin. would it be worth including that in this PR, or not since it's unrelated?

@sloane-shark
Copy link
Contributor Author

Result of nixpkgs-review pr 184041 run on aarch64-darwin 1

2 packages built:
  • haskell.compiler.ghc923
  • haskell.compiler.native-bignum.ghc923

@sternenseemann
Copy link
Member

nixpkgs-review pr 184041 failed on the jacinda package, but i tested on master and it fails for the same reason. seems like this should be marked as broken on (at least) aarch64-darwin. would it be worth including that in this PR, or not since it's unrelated?

Feel free to add a conditional markBroken for regex-rure (which is actually failing) in this PR. Be sure to link the actual issue in a comment as well: haskell/c2hs#33 (comment)

@sloane-shark
Copy link
Contributor Author

sloane-shark commented Jul 31, 2022

@sternenseemann Any recommendation for a cleaner method of doing this?

{
  # ...
  regex-rure = doDistribute ((if isDarwin && isAarch64 then x: x else markUnbroken) super.regex-rure);
  # ...
}

Seems to work fine, just looks a little confusing imo

@sternenseemann
Copy link
Member

Any recommendation for a cleaner method of doing this?

There's an override section in configuration-darwin.nix that only gets applied on aarch64-darwin (towards the bottom of the file).

@sloane-shark
Copy link
Contributor Author

This patch didn't seem to work correctly, trying to build .#jacinda failed with the original error instead of reporting regex-rure broken. Is the GHC configuration overriding the darwin configuration?

@sternenseemann
Copy link
Member

Don't remember the ordering precisely (you can check pkgs/development/haskell-modules/default.nix) in any case, you don't have to have any override in the regular configuration, if your patch only touches configuration-darwin.nix, it should wrok.

@sloane-shark
Copy link
Contributor Author

sloane-shark commented Aug 3, 2022

I can't work out exactly what's going on, but it seems like ...

  • regex-rure is generally marked as broken in hackage-packages.nix
  • configuration-darwin.nix "re"marks it as broken, which is ignored
  • configuration-9.2.x.nix marks it as unbroken, which is the final accepted setting

I can update the PR with this commit so someone can maybe take a look and see if I am missing something. Otherwise I could

  • leave the PR as-is, which correctly marks the package broken on aarch64-darwin
  • drop the broken-marking entirely, and leave that for another PR

edit: i realize i am somewhat assuming that the package will build properly on linux, but is that the case? Is gcc used for C2hs on linux? if not, maybe i could remove the override in configuration-9.2.x.nix to resolve this.

@sloane-shark sloane-shark force-pushed the fix/ghc923-cabal-paths-patch branch from 5c9f360 to 750e57e Compare August 6, 2022 22:22
@sloane-shark
Copy link
Contributor Author

@sternenseemann sorry for the ping, i've pushed a version that i can get to locally report regex-rure as broken, i was unable to get something working via the configuration-9.2.x.nix file. if it's better, i can just drop the version specific hacking and leave the broken packages as-is for now. i think this should be ready for review now, would it be fair to ping anyone? otherwise i can hold tight until someone has the bandwidth.

@sternenseemann
Copy link
Member

regex-rure is generally marked as broken in hackage-packages.nix

Ah, right I remember now. The problem is that regex-rure only builds with GHC 9.2 and above, so you can only expect haskell.packages.ghc924.regex-rure to work and it is marked as broken by default (since that follows the state in the default package set). Leaving the override as is is okay (albeit a bit ugly), there doesn't seem to be a better solution due to the overlay chaining.

@sternenseemann
Copy link
Member

Can you rebase on master? I think the regex-rure hack can be dropped now. The patch will need to be applied to 9.2.4 and 9.2.5 now.

Porting that stuff to 9.4.* can be done in a separate step then.

@srid
Copy link
Contributor

srid commented Jan 3, 2023

This patch should be applied for 9.0 (pkgs.haskellPackages) as well, no? Because of #194367 (comment)

@sternenseemann
Copy link
Member

This patch should be applied for 9.0 (pkgs.haskellPackages) as well, no? Because of #194367 (comment)

haskellPackages is 9.2 now, 9.0 had and has a patch for this already which no longer applies for Cabal 3.8 shipped with GHC 9.2.

@sloane-shark sloane-shark changed the title Add cabal-paths patch for ghc 9.2.3 Add cabal-paths patch for ghc 9.2.x Jan 4, 2023
@sloane-shark
Copy link
Contributor Author

sloane-shark commented Jan 4, 2023

Looks like the patch that got added to all builds for 9.2.x (https://github.com/haskell/cabal/commit/6c796218c92f93c95e94d5ec2d077f6956f68e98.patch) updates some of the same files as this patch, so I have to hardcode it to address those changes. I would say this should be fine, except for the note that says

Can be removed if the Cabal library included with ghc backports the linked fix

But I guess that could just be addressed with a revert of the appropriate commit?

@sloane-shark
Copy link
Contributor Author

I'm also still seeing the regex-rure failure show up in local testing, for what it's worth.

@sloane-shark sloane-shark force-pushed the fix/ghc923-cabal-paths-patch branch 3 times, most recently from 097e631 to 12a98a8 Compare January 4, 2023 23:26
@github-actions github-actions bot removed 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: stdenv Standard environment 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: vim Advanced text editor 8.has: documentation This PR adds or changes documentation 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. labels Feb 17, 2023
@sloane-shark
Copy link
Contributor Author

alright, i think this should be ready for another pass of reviewing. i can kick off nixpkgs-review, but as per comments above, is there anything i can do to more thoroughly verify things are working as expected? i can try running ghcid or ormolu as I did in my initial post

@flokli
Copy link
Member

flokli commented Feb 17, 2023

This mass-subscribed a bunch of people, who won't get unsubscribed from retrieving notifications.

Please open a new PR.

@flokli flokli closed this Feb 17, 2023
@NixOS NixOS locked and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Development

Successfully merging this pull request may close these issues.

5 participants