Skip to content

lambdabot: apply upstream patches for GHC 9.0.2 support#165014

Merged
maralorn merged 5 commits intoNixOS:haskell-updatesfrom
ncfavier:lambdabot-patch
Mar 22, 2022
Merged

lambdabot: apply upstream patches for GHC 9.0.2 support#165014
maralorn merged 5 commits intoNixOS:haskell-updatesfrom
ncfavier:lambdabot-patch

Conversation

@ncfavier
Copy link
Member

@ncfavier ncfavier commented Mar 20, 2022

Tested by building lambdabot and running dice d10 (from the dice plugin) and yow (from the fortune plugin) at the prompt.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Mar 20, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 20, 2022
@maralorn
Copy link
Member

Thank you for your fixes, especially for creating the upstream PR!

  1. normally we apply overrides for the current default compiler in configuration-common.nix
  2. we prefer applying patches instead of switching to unstable, if possible. Especially in this case where the used commits are not even on upstream master.

Can you change the PR to accommodate that, or explain why that’s not feasible?

@ncfavier
Copy link
Member Author

ncfavier commented Mar 21, 2022

Done (and tested again). Wasn't sure where to put my changes in configuration-common.nix so I picked a random place.

@maralorn
Copy link
Member

Okay that’s already better.

Now, there is one thing I should have warned you about. We try to not accumulated patches in nixpkgs, but rather use pkgs.fetchpatch to pull the patch from github. There are various examples in configuration-common.nix.

Since you are patching different packages in the same commit, if I am not mistaken, this would be a bit finnicky with fetchpatch.
You would need a combination of the stripLen = 2, extraPrefix = "" and includes = [ ... ] or something like that. And be careful since this is a fixed output derivation you will need to invalidate the hash everytime you modify one of those parameters.

Do you want to try that? If it’s too annoying, I think I could be convinced to merge this as is …

@ncfavier
Copy link
Member Author

I managed for dice.

For misfortune the PR is based on master which has a few more commits than the latest hackage release, and the patch doesn't apply cleanly. We would need to fetch two more commits from master for the patch to apply. I also can't just excludes = [ "misfortune.cabal" ] because the patch adds a dependency to the cabal file.

For lambdabot, what you suggest doesn't seem possible because the includes logic happens after the stripLen logic for some reason. Unless we make our own fetchpatch with fetchurl + runCommand.

Note that in both cases (misfortune and lambdabot), the only commits on top of the latest release are adjustments to CI, documentation or cabal files, so maybe we could fetch the source from the PR as I did initially and keep the version as-is since there are no code changes.

@maralorn
Copy link
Member

Yeah, the stripLen/excludes order is really annoying.

Thank you for trying. I guess we should choose our battles wisely and not sink any more work into a working fix.

@ncfavier
Copy link
Member Author

I guess we should choose our battles wisely and not sink any more work into a working fix.

I am not a very wise person #165327.

(Let's not block this PR on that one though)

@maralorn maralorn merged commit 5d20cc6 into NixOS:haskell-updates Mar 22, 2022
@maralorn
Copy link
Member

thx for patching

@ncfavier ncfavier deleted the lambdabot-patch branch March 22, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants