haskell{,Packages}: use recurseIntoAttrs

[jopejoe1]

Split from #434501

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[wolfgangwalther]

I think this simplification should be alright, but "eval and build" is certainly wrong. This is, apart from one additional build, merely a refactor. We already build and eval haskell packages.

Also, I'll want to give @sternenseemann to look at this when he finds the time to do so (no urgency).

[maralorn]

I am completely out of my depth here, but would love to learn. The motivation is to simplify/unify packageset definitions?

What would this PR change? Would there be more or less packages available on search.nixos.org, in nix search. Would there be more or less builds on hydra?

[maralorn]

Ah, okay. Search and hydra will go through every attrset which has recurseIntoAttr. So this actually looks like it makes sense. Just makes me wonder why it hadn’t been done like that before.

[wolfgangwalther]

Yeah, so the intent is simplification, making it easier to reason about both search and hydra builds: If an attrset is recursed into, it will show up on both. That's putting this information in a single place and not spread across multiple.

This PR is working towards that goal (part of a bigger effort to make packages-config.nix smaller).

[sternenseemann]

it will show up on both.

Hydra may no longer need it due to a quirk of nix-eval-jobs: NixOS/hydra@d84ff32#r162272621.

Overall this change seems fine. It could hurt nix-env performance which is maybe worth testing since it could be problematic for unrelated downstream use.

[jopejoe1]

it will show up on both.

Hydra may no longer need it due to a quirk of nix-eval-jobs: NixOS/hydra@d84ff32#r162272621.

Would still be needed with that change as as we do some filtering before it reaches hydra.

nixpkgs/pkgs/top-level/release-lib.nix

Lines 208 to 219 in 89dcabe

	# Recursive for packages and apply a function to them
	recursiveMapPackages =
	f:
	mapAttrs (
	name: value:
	if isDerivation value then
	f value
	else if value.recurseForDerivations or false || value.recurseForRelease or false then
	recursiveMapPackages f value
	else
	[ ]
	);

[wolfgangwalther]

Overall this change seems fine. It could hurt nix-env performance which is maybe worth testing since it could be problematic for unrelated downstream use.

I looked at both Eval and the tarball, both of which heavily use nix-env. Both are exactly the same in this PR and a random different PR that was just merged to master, time-wise.

So I don't think this will make a difference.

Needs a rebase. And, as mentioned earlier, improved commit message, aka "refactor", not eval and build.

[wolfgangwalther]

Let's see how it goes.

[trofi]

Bisect says it somehow introduced an unevaluatable attribute. haskell.packages.native-bignum.recurseForDerivations used not to exist, but now it's an untraversable attribute:

$ nix-instantiate -A haskell.packages.native-bignum.recurseForDerivations --show-trace
error:
       … from call site
         at lib/attrsets.nix:1229:61:
         1228|   */
         1229|   genAttrs = names: f: genAttrs' names (n: nameValuePair n (f n));
             |                                                             ^
         1230|

       … while calling anonymous lambda
         at pkgs/top-level/haskell-packages.nix:686:11:
          685|         pkgs.lib.genAttrs nativeBignumGhcNames (
          686|           name:
             |           ^
          687|           packages.${name}.override {

       error: attribute 'recurseForDerivations' missing
       at pkgs/top-level/haskell-packages.nix:687:11:
          686|           name:
          687|           packages.${name}.override {
             |           ^
          688|             ghc = bh.compiler.native-bignum.${name};

[wolfgangwalther]

You are doing nix-instantiate -A haskell.packages.native-bignum.recurseForDerivations. I mean.. if that attribute doesn't exist, that is bound to fail. Could you elaborate in which other way this can trigger a problem?

[trofi]

You are doing nix-instantiate -A haskell.packages.native-bignum.recurseForDerivations. I mean.. if that attribute doesn't exist, that is bound to fail.

It does exist. It's value is an error:

$ nix repl -f '<nixpkgs>'
...
nix-repl> haskell.packages.native-bignum
{
  ghc90 = { ... };
  ghc902 = { ... };
  ghc910 = { ... };
  ghc9101 = { ... };
  ghc9102 = { ... };
  ghc912 = { ... };
  ghc9121 = { ... };
  ghc9122 = { ... };
  ghc92 = { ... };
  ghc928 = { ... };
  ghc94 = { ... };
  ghc947 = { ... };
  ghc948 = { ... };
  ghc96 = { ... };
  ghc963 = { ... };
  ghc964 = { ... };
  ghc965 = { ... };
  ghc966 = { ... };
  ghc967 = { ... };
  ghc98 = { ... };
  ghc981 = { ... };
  ghc982 = { ... };
  ghc983 = { ... };
  ghc984 = { ... };
  ghcHEAD = { ... };
  recurseForDerivations = «error: attribute 'recurseForDerivations' missing»;
}

[wolfgangwalther]

Ah, ok, that's odd :D