modules/nixpkgs: Make customizable & support multiple evaluations

[bb010g]

I would highly appreciate review & comments on this. My main concerns going in were, roughly in order of priority:

• Easily support overlaying Nixpkgs with self.overlays.default for use in pkgs
• Easily support config.allowUnfree for use in NixOS or packages that require unfree
• Keep merging & defaults simple & predictable
• Efficiently evaluate Nixpkgs, ideally no more than once for a Nixpkgs argument attrset (including localSystem)
• Easily support multiple Nixpkgs evaluations for use with (multiple) NixOS configurations

I feel like this patch's design addresses all of those concerns, with a relatively simple module to boot. I think the code is alright, but it could very well be better. I'm not confident.

Here's an example flake declaring its packages in a Nixpkgs overlay & consuming them via pkgs:

{
  description = "A program that produces a familiar, friendly greeting";

  inputs.flake-parts.url = "github:hercules-ci/flake-parts";
  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
  inputs.systems.flake = false;
  inputs.systems.url = "github:nix-systems/default";

  outputs = { ... } @ inputs: inputs.flake-parts.lib.mkFlake {
    inherit inputs;
  } ({ inputs, lib, self, ... }: {
    config.systems = import inputs.systems;
    config.flake = {
      overlays.default = pkgsFinal: pkgsPrev: {
        hello = pkgsFinal.callPackage ./nix/nixpkgs-pkgs/hello { };
      };
    };
    config.nixpkgs.overlays = [
      self.overlays.default
    ];
    config.perSystem = { config, pkgs, ... }: {
      config.devShells.default = pkgs.callPackage ({ mkShell
      , hello
      }: mkShell {
        inputsFrom = [ hello ];
      }) {
      };
      config.packages = {
        inherit (pkgs) hello;
      };
      config.pkgs.default = config.pkgs.hello;
    };
  });
}

All that was necessary to integrate self.overlays.default into pkgs was a simple value for the nixpkgs.overlays option, and these derivations from pkgs can be directly used for perSystem.packages.

Let's say that the author also wants an easy way to test building their package against the latest NixOS stable, without switching inputs.nixpkgs.url or using --override-input nixpkgs <flake-url>.

--- a/flake.nix
+++ b/flake.nix
@@ -4,2 +4,3 @@
   inputs.flake-parts.url = "github:bb010g/flake-parts/enhanced-nixpkgs";
+  inputs.nixos-stable.url = "github:NixOS/nixpkgs/nixos-22.11";
   inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
@@ -18,5 +19,6 @@
     config.nixpkgs.overlays = [ self.overlays.default ];
-    config.perSystem = { pkgs, ... }: {
+    config.nixpkgs.evals.nixos-stable.input = inputs.nixos-stable;
+    config.perSystem = { nixpkgs, pkgs, ... }: {
       config.devShells.default = pkgs.callPackage ({ mkShell
       , hello
       }: mkShell {
@@ -26,3 +28,4 @@
       config.packages = {
         hello = pkgs.hello;
+        hello-nixos-stable = nixpkgs.nixos-stable.hello;
       };

Only 3 lines added and 1 line changed, which I'm pretty happy with. nixpkgs.evals has now been introduced, but the flake author knows that they're dealing with multiple Nixpkgs instantiations by this point, and they don't have to write Nixpkgs application boilerplate to do it. This example is contrived, but you can imagine this approach instead used so that a development NixOS machine can be using unstable Nixpkgs while a server is using stable Nixpkgs, maybe plucking a package or two from the same unstable as the development machine.

I'm imagining a lot of nixpkgs.config usage will just look like config.nixpkgs.config.allowUnstable = true;. I haven't bothered replicating the Nixpkgs config module here because I think it'd be more bother than it's worth. As such, values of nixpkgs.config are types.raw and they'll eagerly fail to merge. This may be annoying for flake authors but it prevents unintended merge behavior.

By utilizing perSystem, all Nixpkgs evaluations can be shared between modules and the nixpkgs and pkgs module arguments won't force extra evaluations (to my knowledge). I don't know if perSystem.nixpkgs.evals.<name>.output should be hidden / internal, but I leaned towards public for now, partially so it's clear to flake authors what's going on. The documentation for this option should probably be improved to reflect that.

I debated about using RFC 42–style nixpkgs.settings, nixpkgs.evals.<name>.settings, & perSystem.nixpkgs.<eval>.settings options, to be used like nixpkgs.settings.overlays, but I don't know if the flexibility provided there justifies the extra verbosity. These options would likely be submodules with config._module.freeformType = types.lazyAttrsOf types.raw; if they did exist, so that options like nixpkgs.settings.overlays can be documented & specified to merge. This would also be more future-proof, but if Nixpkgs does accept this upstream, then that flexibility would not an issue.

This patch does not configure transposition or otherwise expose any option config data under flake.<system>. Sharing of values (e.g. via flake output) for config.nixpkgs.config or any other option is left up to the user. I personally wouldn't mind placing some of this data in the output, but that's a larger proposal that I want to avoid here.

I don't really know where _file is and is not inferred in flake-parts modules. Documentation on this for module authors somewhere would be nice.

[bb010g]

I refactored the top-level nixpkgs module to have a freeform nixpkgs.settings submodule. Options no longer need to be re-declared in nixpkgs.evals.<name>.settings. perSystem.nixpkgs.evals.<name>.settings has been added to make defaulting config.localSystem less magical and to allow specifying config.crossSystem.

A downside of this arrangement is that to set config in a module, you have to use nixpkgs.settings.config.config because nixpkgs.settings is a submodule via types.submoduleWith { modules = …; }. Is this an acceptable place to deviate from the flake-parts standard (as far as I could tell) and use shorthandOnlyDefinesConfig = true;?

[bb010g]

Short turnaround, but I changed the nixpkgs.settings submodule to use shorthandOnlyDefinesConfig. nixpkgs.settings.config.allowUnfree = true; really should just work, and shouldn't require nixpkgs.settings.config.config.allowUnfree = true;. Also, settings.config.config would probably be a problem in future attempts to upstream this module to Nixpkgs and move it out of flake-parts. This deviation from project style is explained in comments in the module source.

[bb010g]

This is a big module. That's not necessarily bad, but not good for everyone. Flake-parts is meant to be an unopinionated core, so this module should, as suggested in nixpkgs.nix, not be merged. Instead, users should choose which module to use.

If that's the case, would the current Nixpkgs module be moved to extra? I'm worried about incompatibility if other flake modules decide to build off this. (I've been concurrently working on a perSystemNixosConfigurations module on top of this module, and it meshes really nicely. Sharing a minimal amount of Nixpkgs evaluations between a whole set of machines is great, and it falls out naturally from this module's design.)

That said, of course I appreciate your effort, and I would like your module to be listed on flake.parts, so here's a review. Also, would you mind if I merge just some of your maintenance-oriented commits?

If you want to start merging maintenance comments now, go for it. I tacked on a few more commits extending the dev debugging tools & evaluation test suite while exploring an existing flake-parts regression that I thought was due to new code. (I think flakes really need to pass sourceInfo alongside self to flake.outputs for error messages involving the flake's path to not infinitely recurse. We can't do anything here besides suggest manually specifying _file = ./flake.nix; or flake-utils.lib.mkFlake { inputs = { sourceInfo.outPath = ./.; } // inputs; }.)

As a side question, when do you recommend manually specifying key in flake modules?

[bb010g]

@roberth Reading https://github.com/divnix/call-flake/commit/5828e083daac39efb098dc719502379f2bf2ed8a, it looks like both outPath and sourceInfo would have to be inputs to outputs that don't force self in order to consistently provide proper error messages when evaluation of self errors?