nixos/nixpkgs: make nixpkgs.config mergeable

[Ma27]

Description of changes

Closes #194056
Alternative implementation to #80582

nixpkgs.config is now a freeform-type which imports the
<nixpkgs/pkgs/top-level/config.nix>-module. This brings the following
benefits:

• Proper merging (to a certain degree, more details later) of several
  nixpkgs.config declarations.

• allowUnfree and friends appear in the NixOS manual.

To be precise, this means that something like this is now possible:

{
  nixpkgs = mkMerge [
    { allowUnfree = false; }
    { allowUnfree = mkForce true; }
  ];
}

Previously this would lead to a bogus attr-set, namely

{ _type = "override"; content = true; priority = 50; }

since no merging for this option-type was implemented.

This however has certain limitations. For instance

nixpkgs.config.allowUnfree = mkForce false;

works fine whereas

nixpkgs.config.virtualbox.enableExtensionPack = mkForce true;

doesn't. This is because we use types.raw inside the config-module for
nixpkgs which doesn't merge undeclared attribute-sets (such as
virtualbox). This is because we don't know if any attribute-set is
actually mergeable, for further details see the previous discussion[1].

A basic type has been implemented for the (actually deprecated)
mechanisms packageOverrides/perlPackageOverrides. This one basically
applies the pkgs argument onto each definition of this function, so
e.g. the following expression

{
  nixpkgs.config = mkMerge [
    { packageOverrides = pkgs: { randomattr = 23; }; }
    { packageOverrides = pkgs: { randomattr = pkgs.randomattr + 19; }; }
  ];
}

adds an attribute randomattr of value 42 to _module.args.pkgs.

[1] #194207 (comment)

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:
  • 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/)
• 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.

[roberth]

This kinda looks good, but I think we should address a deeper issue, which is the (worse) duplication of the same logic from Nixpkgs. I think we could give nixpkgs.config a submodule type and imports pkgs/top-level/config.nix in it. That way, packageOverrides like any other option can be represented by nothing more than module system types.

[Ma27]

@roberth AFAIU pkgs/top-level/config.nix doesn't have a definition for packageOverrides, so there's (1) no logic duplicated and (2) the merging issue doesn't even exist here. Or am I misunderstanding you? :)

[roberth]

You are technically correct, but what I'm trying to achieve by suggesting this is to clean up the custom merging code, and move the responsibility for merging from two places to one.

The pkgs/top-level/config.nix module is more developed than the NixOS one, and it's also used for documentation, so I think it makes sense to use that one.

By representing the merging behavior with the module system, it can be understood more easily without having to dig through the custom code. Code of such complexity needs tests and I don't want to ask you to write tests for an unnecessary solution. And it does have at least two subtle problems because types.anything is stricter than it should be, and overlays (called "overrides" here) shouldn't be ordered arbitrarily by the module system.

[Ma27]

OK, I played around with it a little bit more: importing pkgs/top-level/config.nix is actually a nice idea because then we also have defined nixpkgs config-options inside the manual.

However:

• We cannot use lazyAttrsOf raw here: this would break definitions such as virtualbox.enableExtensionPack being defined multiple times (once with false and once with mkForce true for instance).
• I tried to use lib.types.anything instead to restore the merging-behavior in config.nix, but then we have the same problem with packageOverrides which can only defined once unless the module system "decides" how it will be merged together. Would it be okay to you to implement it (with tests) in pkgs/top-level/config.nix or do you have a nicer idea?

[roberth]

• We cannot use lazyAttrsOf raw

I agree raw is a bit excessive, but I'd rather have something unsupported than something that's subtly broken by default. We can't just assume that any attrset can be merged, because we have no idea what it is supposed to mean.
The real solution is to declare more options. That way both merge and the user (docs) know what's supposed to happen.

problem with packageOverrides which can only defined once unless the module system "decides" how it will be merged together. Would it be okay to you to implement it (with tests) in pkgs/top-level/config.nix or do you have a nicer idea?

We should have a type for overlays. That'd be something worth testing.
It could be quite fancy and report ambiguities, but let's start with a simple implementation instead.

[Ma27]

I agree raw is a bit excessive, but I'd rather have something unsupported than something that's subtly broken by default. We can't just assume that any attrset can be merged, because we have no idea what it is supposed to mean.

So, your suggestion is to just use raw to have some basic merging capabilities (e.g. for allowUnfree and freeform options that are not an attr-set)?

We should have a type for overlays

Is that related to this change? Overlays are IIRC not passed into config, but directly to <nixpkgs>.

[roberth]

So, your suggestion is to just use raw to have some basic merging capabilities

Yes. We can use raw where our knowledge of the options ends. When we don't know for what option an attribute in config is, we fall back to raw. This allows multiple definitions of config to each contribute their own "top level" config.<name> attributes, as long as the intersection of those attributes has options declared for them.

We should have a type for overlays

I guess technically packageOverrides isn't an overlay because it lacks the super parameter, but otherwise they're the same.

Overlays are IIRC not passed into config, but directly to .

Correct. The type for nixpkgs.overlays should be improved though.

[Ma27]

@roberth I pushed a different implementation which makes nixpkgs.config a freeform-type using pkgs/top-level/config.nix.

Also, I implemented a fairly simple option-type for packageOverrides (a similar approach may be used for nixpkgs.overlays at some point in the future), but I'm not really sure if this approach is actually a good idea, but I thought I'd push it anyways to get some feedback on this approach %)

[roberth]

This is starting to look good.
After adding a test for the type, I think this will be ready to merge.

Resolved

[Ma27]

@roberth anything else to get this merged? :)

[infinisil]

This changes the merging behavior from before. Most notably the old merging function didn't propagate changes from earlier overrides to later ones, there's only one pkgs that is reused as the input for all override functions.

[roberth]

Is that functionTo attrs then?

[infinisil]

Almost. I think not because of the optCall, which allows either functions or attribute sets directly

[roberth]

Ok, then it needs a coercedTo.

optCall can be replaced by lib.toFunction, but I guess we'll only need lib.const anyway.

[infinisil]

In addition, the perlPackageOverrides is super weird, in that it doesn't even take the perl packages as an argument, but rather the main pkgs package set!

❯ nix-instantiate '<nixpkgs>' --arg config '{ perlPackageOverrides = pkgs: builtins.trace (builtins.head (builtins.attrNames pkgs)) {}; }' -A perlPackages.FileFinder
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/sf3bji32b4xw77xjh7w452pj15jq8gh6-perl5.34.1-File-Finder-0.53.drv

(this shows that the first attribute in the passed pkgs is pkgs.AAAAAASomeThingsFailToEvaluate)

[roberth]

Yikes, but it also kinda makes sense, because how else would you access pkgs. Actually doing your own fixpoint might work, but not with features like pkgsCross.

Anyway, thanks @infinisil for catching this and sorry @Ma27 for making a wrong suggestion.

[Ma27]

no worries, I also missed that!

[Ma27]

Uhm... I obseve the same behavior on my local master (dfae1c1 currently):

❯ ~/Projects/nixpkgs master → nix-instantiate './.' --arg config '{ perlPackageOverrides = pkgs: builtins.trace (builtins.head (builtins.attrNames pkgs)) {}; }' -A perlPackages.FileFinder
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/f9qgccfq4p6n161jlkdf05qikmq4l9jj-perl5.36.0-File-Finder-0.53.drv

Am I missing something? oO

[infinisil]

That's as expected. This just demonstrates that perlPackageOverrides gets the pkgs as an argument and not pkgs.perlPackages, since there's only pkgs.AAAAAASomeThingsFailToEvaluate and no pkgs.perlPackages.AAAAAASomeThingsFailToEvaluate. Don't mind this specific attribute name, it's not related to this issue.

[roberth]

In addition, the perlPackageOverrides is super weird, in that it doesn't even take the perl packages as an argument, but rather the main pkgs package set!

It's funny. In a discussion today the inverse came up. Without a way to reference pkgs, mere two-function overlays are not good enough for distributing additions/changes to language-specific package sets, because those inevitably tend to need some things from pkgs... but which one?
So without a pkgs parameter it's not suitable for distributed use, and without a prev or self, it's not an overlay.
We need both! ... or do we? self is actually available in pkgs.
But yes, we do, because the location may be ambiguous. Is it haskellPackages or haskell.packages.ghc943?

[roberth]

We could change the Nixpkgs entrypoint to allow multiple config objects. It is itself backed by the module system, so it can easily merge the definitions on NixOS' behalf. For simplicity we might even pass modules, although that seems like unnecessary power.
type = deferredModule would make that easy on the NixOS side.
If we want to stop short of full modules, we could have a fairly simple custom type that preserves all the definitions as a list. Type-changing types.
The only downside (I think) of these solutions is that they're not readable on the NixOS side, although arguably such code could just read from pkgs.config instead.

[Ma27]

Actually, that's a pretty good idea and way better than effectively doing two rounds of evaluations of config (for nixpkgs), so I'm in favor 👍

The only downside (I think) of these solutions is that they're not readable on the NixOS side, although arguably such code could just read from pkgs.config instead.

It'd be nice to ensure that users get a somewhat usable eval error (or warning at least) when referencing config.nixpkgs.config rather than pkgs.config, though I'm not (yet) sure if that's doable in a reasonable manner (and if so, how).

Will close this PR for now. Adding this suggestion to my todo list, but it'll take a moment until I get to it, so feel free to go ahead if you have the motivation & capacity currently 😄