haskell generic-builder: fix issues with GHC_PACKAGE_PATH in checkPha…

[MangoIV]

Description of changes

#311024 , supersedes #311025 as @NorfairKing requested that.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 24.05 Release Notes (or backporting 23.05 and 23.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
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[MangoIV]

cc @sternenseemann @NorfairKing

[NorfairKing]

@MangoIV Thanks for taking this on! How could I try this out?

[MangoIV]

gonna be kinda problematic to try out for everything, I'm currently trying it out on th-abstraction because it doesn't have too many deps and I know it didn't work but this rebuilds the entire haskell world so I think the hydra will be the more reliable way of trying this out...

[NorfairKing]

@MangoIV If it works with one package that runs a check with that problematic cabal version then I'm happy.

[NorfairKing]

CC @locallycompact

[NorfairKing]

Would love to get this into the 24.05 release if we can, otherwise I won't be able to use GHC 9.10 for another 6 months.

[NorfairKing]

NorfairKing/validity#118 @MangoIV I'm failing to try it out because I'm getting an eval error somewhere else, which is really strange.

[MangoIV]

if you check out the branch and then nix build .#haskell.packages.ghc910.th-abstraction you should start building ghc 👀

[NorfairKing]

@MangoIV I'm trying but it's gonna take a while because I'm building GHC now :D

[MangoIV]

I'm already in stage 1, I will tell you what happens, don't waste the CPU cycles ;)

[MangoIV]

th-abstraction> Running phase: checkPhase
th-abstraction> Running 1 test suites...
th-abstraction> Test suite unit-tests: RUNNING...
th-abstraction> Test suite unit-tests: PASS
th-abstraction> Test suite logged to: dist/test/th-abstraction-0.7.0.0-unit-tests.log
th-abstraction> 1 of 1 test suites (1 of 1 test cases) passed.

looks like this worked...

[NorfairKing]

@MangoIV I also want to confirm that it was indeed broken before this commit.

[MangoIV]

you can try nix build github:nixos/nixpkgs/haskell-updates#haskell.packages.ghc910.th-abstraction

[NorfairKing]

@MangoIV That does indeed fail but for a different reason:

th-abstraction> Encountered missing or private dependencies:
th-abstraction>     containers >=0.4 && <0.7, template-haskell >=2.5 && <2.21
th-abstraction> CallStack (from HasCallStack):
th-abstraction>   dieWithException, called at libraries/Cabal/Cabal/src/Distribution/Simple/Configure.hs:1457:11 in Cabal-3.12.0.0-inplace:Distribution.Simple.Configure

[MangoIV]

my bad, it has to be _0_7_0_0

[NorfairKing]

@MangoIV Everything seems to be in order then :D
I can't approve this (I think?) but it's all good to go as far as I'm concerned!
Let's get this merged and into 24.05 !

[NorfairKing]

Can confirm.
nix build .#haskell.packages.ghc910.th-abstraction_0_7_0_0 fails before and succeeds after.

[NorfairKing]

@sternenseemann ready to merge :D

[sternenseemann]

Hm actually this whole GHC_PACKAGE_PATH stuff may no longer be necessary… let me try something.

[NorfairKing]

@sternenseemann I am all for improvements but can we make them after fixing this bug?

[maralorn]

Will the staging branch this PR is targeting reach 24.05? If no we need to discuss other approaches, if yes finding an alternative solution until that branch gets merged seems unproblematic.

[NorfairKing]

@maralorn I have no idea, I don't know the process and don't have commit access 🤷

[sternenseemann]

This fix, just like what I have in mind, requires a full rebuild of all Haskell packages. Given how close 24.05 is, we shouldn't unnecessarily cause these multi-day rebuilds to avoid delays.

I'm currently in the process of confirming that my change idea is sound. The quickest way for any of these changes to land is to batch it with the rebuild #312133 will cause either way (Hydra is kinda busy at the moment), so it won't necessarily go any quicker.

The problem is easy to work around, so I don't think it's too urgent.

[MangoIV]

@sternenseemann can you please somehow link more details? I would love to help you in these endeavours, it would be nice to be able to build anything with ghc910, even better if the hydra looked at it briefly before.

Thank you!

[sternenseemann]

I believe

export GHC_PACKAGE_PATH="$packageConfDir:"

is the only time in a normal situation that that variable would be set at the moment since it isn't actually set before configurePhase. The idea would thus be to delete the unset workaround altogether.

Unfortunately, it is unclear from the git history why this workaround was introduced in the first place, so I've reached out to peti to ask. If I'm right, this is a relic from when this var was set by the GHC setup hook (which has been removed even before generic-builder.nix existed), but I can't be sure yet. That can be complemented by some test builds of course to rule out any obvious problems before we try this.

[MangoIV]

@sternenseemann have you tried building without the workaround?

[NorfairKing]

@sternenseemann I am all for improvements but can we make them after fixing this bug?

I don't mean to sound ungrateful, but this bug has now been "fixed" for more than three days and not merged because there could be another improvement.
This is a classic "perfect is the enemy of the good" example IMO.

I hope we can still get this fix into 24.05 while I don't care whether the improvement makes it in.

[sternenseemann]

@ofborg build haskellPackages.doctest

[maralorn]

@NorfairKing
There is still plenty of time to get things into 24.05 and we can even do backports, so don’t worry, we will get this in.
Branch management and merging shortly before a release is a bit intricate so please bare with us while we manage this.
The "other improvement" already exists here #312859.

[NorfairKing]

@maralorn Thanks! And great communicating, thank you!

[locallycompact]

Is this related to this bug that has started showing up for me? I tried both as it was and with #312859

       > /nix/store/5b754nfcq4r0nx6cd9ja0mhdwy3bwcac-primitive-0.9.0.0/nix-support:
       > propagated-build-inputs: /nix/store/ny78cq2afsibchban97zby8ja0acd5dw-assoc-1.1/nix-support/propagated-build-inputs
       > /nix/store/9b15ff3152pv5j255qda5c1lapq7ghg7-vector-stream-0.1.0.1/nix-support:
       > propagated-build-inputs: /nix/store/ny78cq2afsibchban97zby8ja0acd5dw-assoc-1.1/nix-support/propagated-build-inputs
       > /nix/store/9b15ff3152pv5j255qda5c1lapq7ghg7-vector-stream-0.1.0.1/nix-support:
       > propagated-build-inputs: /nix/store/ny78cq2afsibchban97zby8ja0acd5dw-assoc-1.1/nix-support/propagated-build-inputs
       > rm: cannot remove '/nix/store/a43qiq0vap2vzzcvdq72s4nasxa5x3j7-ghc-9.10-with-packages/lib/ghc-9.10/lib/package.conf.d/package.cache.lock': No such file or directory

From: https://gitlab.horizon-haskell.net/package-sets/horizon-core/-/jobs/1441142

reproduction:

nix develop 'git+https://gitlab.horizon-haskell.net/package-sets/horizon-core?ref=build-env-bug#lens.env'

[sternenseemann]

@locallycompact No that is the withPackages wrapper and your GHC disagreeing where the package db is located. It seems suspicious to me that your GHC has the version number 9.10 with nothing else. Likely that's causing the problem.

[yaitskov]

Cherry picking this onto master results into overlays are ignored.

[maralorn]

Cherry picking this onto master results into overlays are ignored.

Can you maybe double check that or maybe share an example with error message? That sounds quite implausible.

[yaitskov]

Cherry picking this onto master results into overlays are ignored.

Can you maybe double check that or maybe share an example with error message? That sounds quite implausible.

Following default.nix should fail because happy derivation has a typo, but it doesn't
nix-build behaves just like overlay is missing.
If you comment nixpkgs tar url and uncomment master one then nix-build fails as expected.
I know it sounds impossible because the patch just unset bash var and nix code is not affected.

{ pkgs ? import (builtins.fetchTarball
  "https://github.com/yaitskov/nixpkgs/archive/3c0914d2240c88bf60c9474f0f2e15e5911e1678.tar.gz"
  #"https://github.com/NixOS/nixpkgs/archive/37de455272d619d1aab9b6de567de3b3aac32aa3.tar.gz"
) {
}
}:
let
  inherit (pkgs) lib;
  inherit (pkgs.haskell.lib) dontCheck dontHaddock ;

  my = drv: dontHaddock (dontCheck (drv.overrideAttrs(oa: { })));

hp = pkgs.haskell.packages.ghc98; 
  haskellOverlays = [
    (hf: hp: {
      happy = my hp.happyyy;
    })
  ];

  hp2 = hp.override(old: {
    overrides =
      builtins.foldl' pkgs.lib.composeExtensions old.overrides haskellOverlays;
  });

  sourceRegexes = [
    ".*"
  ];

  foo = (enableSharedExecutables
    (hp2.callCabal2nix "foo" (lib.sourceByRegex ./. sourceRegexes) {})).overrideAttrs(oa:
      {  }
    );
in
{ inherit foo;
}

[maralorn]

The commit you are comparing with is not the one on which you cherry-picked.

[yaitskov]

nixpkgs master branch

{ pkgs ? import (builtins.fetchTarball
    #"https://github.com/yaitskov/nixpkgs/archive/a9936a04bf9e3041ff972ddf69a47a519b1da6d9.tar.gz"
  "https://github.com/yaitskov/nixpkgs/archive/ac5cb0b650c56e0ca4e7427d5db221c11bd7190c.tar.gz"

) {
}
}:
let
  inherit (pkgs) lib;
  hp = pkgs.haskell.packages.ghc98; 
  haskellOverlays = [
    (hf: hp: {
      doctest = hp.dctest;
    })
  ];

  hp2 = hp.override(old: {
    overrides =
      builtins.foldl' pkgs.lib.composeExtensions old.overrides haskellOverlays;
  });

  sourceRegexes = [
    ".*"
  ];

  foo = (
    (hp2.callCabal2nix "foo" (lib.sourceByRegex ./. sourceRegexes) {})).overrideAttrs(oa:
      {  }
    );
in
{ inherit foo;
}

nix-build

   error: attribute 'dctest' missing

   at /home/dan/demo/nix/ghc9.10/default.nix:16:17:

       16|       doctest = hp.dctest;

nixpkgs master branch with cherry picked PR:

{ pkgs ? import (builtins.fetchTarball
    "https://github.com/yaitskov/nixpkgs/archive/a9936a04bf9e3041ff972ddf69a47a519b1da6d9.tar.gz"
) {
}
}:
let
  inherit (pkgs) lib;
  hp = pkgs.haskell.packages.ghc98; 
  haskellOverlays = [
    (hf: hp: {
      doctest = hp.dctest;
    })
  ];

  hp2 = hp.override(old: {
    overrides =
      builtins.foldl' pkgs.lib.composeExtensions old.overrides haskellOverlays;
  });

  sourceRegexes = [
    ".*"
  ];

  foo = (
    (hp2.callCabal2nix "foo" (lib.sourceByRegex ./. sourceRegexes) {})).overrideAttrs(oa:
      {  }
    );
in
{ inherit foo;
}

nix-build

Running phase: buildPhase
Preprocessing library for doctest-0.22.2..
Building library for doctest-0.22.2..
[ 1 of 21] Compiling Imports ( src/Imports.hs, dist/build/Imports.o, dist/build/Imports.dyn_o )
[ 2 of 21] Compiling GhcUtil ( src/GhcUtil.hs, dist/build/GhcUtil.o, dist/build/GhcUtil.dyn_o )
[ 3 of 21] Compiling Language.Haskell.GhciWrapper ( src/Language/Haskell/GhciWrapper.hs, dist/build/Language/Haskell/GhciWrapper.o, dist/build/Language/Haskell/GhciWrapper.dyn_o )

[maralorn]

Resolved via: #312859