Add Nixpkgs invocation helper modules #231940

roberth · 2023-05-14T22:11:17Z

Description of changes

This makes a number of well tested and documented additions that lead up to

A reusable module that module system applications can use for the purpose of invoking the Nixpkgs function.
A much sought after flake-parts module.

... using the supporting additions and improvements:

lib.systems.equals function for comparing elaborated system strings.
lib.systems.toLosslessStringMaybe which is useful for converting elaborated systems to optional attribute names.
The lib.systems tests are now run by ofborg.

This PR contains commits that have been split into a separate PR

lib.systems.equals #237512

Things done

fricklerhandwerk

Just a superficial review. TBH the documentation doesn't explain to me who and what this thing is for exactly, and how it's supposed to be used. I could go into option documentation if you like, once the big picture is clear.

fricklerhandwerk · 2023-05-15T09:47:21Z

doc/using/configuration.chapter.md

@@ -189,6 +189,13 @@ The following attributes can be passed in [`config`](#chap-packageconfig).
 <include xmlns="http://www.w3.org/2001/XInclude" href="../doc-support/result/config-options.docbook.xml"/>
 ```

+## Calling Nixpkgs from the Module System {#sec-invoke-nixpkgs-by-module}
+
+For [module system](#module-system) application authors, Nixpkgs provides a reusable module for the purpose of invoking Nixpkgs. Most users should not have to be aware of this. The documentation for its options is meant to be re-exposed in the application's documentation and is therefore not repeated here.


By "module system application" you mean things like Home Manager? I'm not sure what you mean here.

That's what I meant.
I don't know how to say this without going on a tangent. I think going on a tangent to explain this would distract from the content. Most users will know to skip the clause or skip the paragraph.

Suggested change

For [module system](#module-system) application authors, Nixpkgs provides a reusable module for the purpose of invoking Nixpkgs. Most users should not have to be aware of this. The documentation for its options is meant to be re-exposed in the application's documentation and is therefore not repeated here.

For authors of applications based on the [module system](#module-system), Nixpkgs provides a reusable module for the purpose of invoking Nixpkgs. Most users should not have to be aware of this. The documentation for its options is meant to be re-exposed in the application's documentation and is therefore not repeated here.

I can do a documentation review pass when the implementation is done. Unsubscribing for now, ping me if needed.

doc/using/configuration.chapter.md

pkgs/top-level/module/module.nix

lib/systems/default.nix

pkgs/top-level/module/tests.nix

Ericson2314

I didn't re-review the docs; I trust @fricklerhandwerk on that.

I reviewed the rest, and it looks good modulo these small few things. Excited that we are getting started on the idea behind NixOS/rfcs#78 --- the grand module deduplication!!

flake.nix

A helper for module system applications. Naming compatible with NixOS/nix#6257 To be supported by nix flake check in NixOS/nix#8332 Concept is in the spirit of a proposed module based solution to [RFC 0078 System-agnostic configuration file generators](NixOS/rfcs#78).

This hack isn't needed anymore, and would be incompatible with having submodule at `nixpkgs.*`.

A feature flag warning was rightfully picked up by ofborg. This should solve it, and hopefully help out the next person who tries to render docs for an optional module.

doc/using/configuration.chapter.md

blaggacao · 2023-07-16T17:58:09Z

In the outer rings of context to this PR, I have attempted abstractions in a similar problem domain spanning beyond nixpkgs here.

Note that this comment has no bearing to this PR beyond situational awareness for interested parties.

Co-authored-by: David Arnold <dgx.arnold@gmail.com>

infinisil

Reviewed together with @roberth on a call

infinisil · 2023-08-03T16:30:10Z

nixos/modules/misc/nixpkgs/no-legacy.nix

+{
+  disabledModules = [
+    # This replaces the traditional nixpkgs module
+    ../nixpkgs.nix


That module has other imports that would also get removed:

nixpkgs/nixos/modules/misc/nixpkgs.nix

Lines 99 to 103 in 4f51d81

imports = [

./assertions.nix

./meta.nix

(mkRemovedOptionModule [ "nixpkgs" "initialSystem" ] "The NixOS options `nesting.clone` and `nesting.children` have been deleted, and replaced with named specialisation. Therefore `nixpgks.initialSystem` has no effect anymore.")

];

infinisil · 2023-08-03T16:39:30Z

nixos/modules/misc/nixpkgs/no-legacy.nix

+{
+  disabledModules = [
+    # This replaces the traditional nixpkgs module
+    ../nixpkgs.nix


Why does this have to be a separate module? How about updating the ../nixpkgs.nix one, and soft deprecating the localSystem and crossSystem options.

infinisil · 2023-08-03T16:47:42Z

pkgs/top-level/module/module.nix

+{
+  options = {
+    hostPlatform = mkOption {
+      type = types.coercedTo types.str lib.systems.elaborate types.attrs;


We shouldn't use types.attrs, has weird merging behavior, types.raw should be fine

Suggested change

type = types.coercedTo types.str lib.systems.elaborate types.attrs;

type = types.coercedTo types.str lib.systems.elaborate types.raw;

Same for the other options.

infinisil · 2023-08-03T16:51:49Z

pkgs/top-level/module/module.nix

+          { allowBroken = true; allowUnfree = true; }
+        '';
+      # FIXME: use a mergeable type, like NixOS but not a hack
+      type = types.unique { message = "nixpkgs/pkgs/top-level/module.nix: Merging is not supported for the Nixpkgs config option yet."; } types.attrs;


Suggested change

type = types.unique { message = "nixpkgs/pkgs/top-level/module.nix: Merging is not supported for the Nixpkgs config option yet."; } types.attrs;

type = types.unique { message = "nixpkgs/pkgs/top-level/module.nix: Merging is not supported for the Nixpkgs config option yet."; } types.raw;

Or attrsOf raw

infinisil · 2023-08-03T16:56:58Z

pkgs/top-level/module/module.nix

+          if lib.mapAttrs (_: _: null) (builtins.removeAttrs inputsSet ["hostPlatform"])
+             != { hostPlatform = null; }


Seems wrong, but could also be done with builtins.removeAttrs inputsSet [ "hostPlatform" ] != { }

infinisil · 2023-08-03T17:14:51Z

pkgs/top-level/module/module.nix

+  isCross = config.buildPlatform != config.hostPlatform;
+  systemArgs =


Let's have a comment here mentioning that this is a bit of a hack and should be moved into the pkgs entrypoint function definition

infinisil · 2023-08-03T17:18:02Z

pkgs/top-level/module/module.nix

+  # key are usable with _memoize implementations that are not aware of them,
+  # as long as these additions are unused.
+  inputsSet = lib.filterAttrs
+    (_: v: v != v.isDefined && v.highestPrio < optDefaultPrio)


This condition needs a bit of work, v != v.isDefined seems wrong (detected by @roberth himself!)

infinisil · 2023-08-03T17:22:35Z

flake.nix

+      pkgsMemoizer = { pkgs, inputsSet, ... }:
+        let system = lib.systems.toLosslessStringMaybe inputsSet.hostPlatform.value.system;
+        in
+        if lib.attrNames inputsSet == [ "hostPlatform" ] && system != null


See also the other comment

infinisil · 2023-08-03T17:23:02Z

flake.nix

@@ -12,6 +12,20 @@
      lib = import ./lib;

      forAllSystems = lib.genAttrs lib.systems.flakeExposed;
+
+      pkgsMemoizer = { pkgs, inputsSet, ... }:
+        let system = lib.systems.toLosslessStringMaybe inputsSet.hostPlatform.value.system;


Suggested change

let system = lib.systems.toLosslessStringMaybe inputsSet.hostPlatform.value.system;

let system = lib.systems.toLosslessStringMaybe inputsSet.hostPlatform.value.system;

Calling this on system sounds wrong 😆

roberth requested a review from Ericson2314 May 14, 2023 22:11

roberth requested review from fricklerhandwerk, alyssais, matthewbauer, edolstra and infinisil as code owners May 14, 2023 22:11

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation labels May 14, 2023

roberth force-pushed the nixpkgs-module-helper branch from 4dbad27 to fc99ccc Compare May 14, 2023 22:12

ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 15, 2023

fricklerhandwerk reviewed May 15, 2023

View reviewed changes