From a742767bafecf0644585309b744e24f90cdff207 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 16 Jun 2023 21:28:06 +0200 Subject: [PATCH 1/4] nixos/nixpkgs: Make default Nixpkgs lazy when overridden --- nixos/modules/misc/nixpkgs.nix | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/nixos/modules/misc/nixpkgs.nix b/nixos/modules/misc/nixpkgs.nix index 55ec08acf445..8a20e31d5137 100644 --- a/nixos/modules/misc/nixpkgs.nix +++ b/nixos/modules/misc/nixpkgs.nix @@ -337,7 +337,15 @@ in config = { _module.args = { - pkgs = finalPkgs.__splicedPackages; + pkgs = + # We explicitly set the default override priority, so that we do not need + # to evaluate finalPkgs in case an override is placed on `_module.args.pkgs`. + # After all, to determine a definition priority, we need to evaluate `._type`, + # which is somewhat costly for Nixpkgs. With an explicit priority, we only + # evaluate the wrapper to find out that the priority is lower, and then we + # don't need to evaluate `finalPkgs`. + lib.mkOverride lib.modules.defaultOverridePriority + finalPkgs.__splicedPackages; }; assertions = [ From 36ea2bbfe81ccf1118fd0ef66b13868fa3cc27e4 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 16 Jun 2023 21:43:12 +0200 Subject: [PATCH 2/4] lib.modules: Add mergeAttrDefinitionsWithPrio This will let us make assertions involving _module.args.pkgs, which is not an option but a value attribute, and therefore doesn't have its own highestPrio to inspect. The new function gives us that info. --- lib/modules.nix | 42 +++++++++++++++++++ lib/tests/modules.sh | 2 + .../test-mergeAttrDefinitionsWithPrio.nix | 21 ++++++++++ 3 files changed, 65 insertions(+) create mode 100644 lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix diff --git a/lib/modules.nix b/lib/modules.nix index 4dc8c663b2fe..c7dcccd7a1dc 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -910,6 +910,47 @@ let else opt // { type = opt.type.substSubModules opt.options; options = []; }; + /* + Merge an option's definitions in a way that preserves the priority of the + individual attributes in the option value. + + This does not account for all option semantics, such as readOnly. + + Type: + option -> attrsOf { highestPrio, value } + */ + mergeAttrDefinitionsWithPrio = opt: + let subAttrDefs = + lib.concatMap + ({ value, ... }@def: + map + (value: def // { inherit value; }) + (lib.pushDownProperties value) + ) + opt.definitionsWithLocations; + defsByAttr = + lib.zipAttrs ( + lib.concatLists ( + lib.concatMap + ({ value, ... }@def: + map + (lib.mapAttrsToList (k: value: { ${k} = def // { inherit value; }; })) + (lib.pushDownProperties value) + ) + opt.definitionsWithLocations + ) + ); + in + assert opt.type.name == "attrsOf" || opt.type.name == "lazyAttrsOf"; + lib.mapAttrs + (k: v: + let merging = lib.mergeDefinitions (opt.loc ++ [k]) opt.type.nestedTypes.elemType v; + in { + value = merging.mergedValue; + inherit (merging.defsFinal') highestPrio; + }) + defsByAttr; + /* Properties. */ mkIf = condition: content: @@ -1256,6 +1297,7 @@ private // importJSON importTOML mergeDefinitions + mergeAttrDefinitionsWithPrio mergeOptionDecls # should be private? mkAfter mkAliasAndWrapDefinitions diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 7aebba6b589e..a60228198fd7 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -61,6 +61,8 @@ checkConfigError() { # Shorthand meta attribute does not duplicate the config checkConfigOutput '^"one two"$' config.result ./shorthand-meta.nix +checkConfigOutput '^true$' config.result ./test-mergeAttrDefinitionsWithPrio.nix + # Check boolean option. checkConfigOutput '^false$' config.enable ./declare-enable.nix checkConfigError 'The option .* does not exist. Definition values:\n\s*- In .*: true' config.enable ./define-enable.nix diff --git a/lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix b/lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix new file mode 100644 index 000000000000..3233f4151368 --- /dev/null +++ b/lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix @@ -0,0 +1,21 @@ +{ lib, options, ... }: + +let + defs = lib.modules.mergeAttrDefinitionsWithPrio options._module.args; + assertLazy = pos: throw "${pos.file}:${toString pos.line}:${toString pos.column}: The test must not evaluate this the assertLazy thunk, but it did. Unexpected strictness leads to unexpected errors and performance problems."; +in + +{ + options.result = lib.mkOption { }; + config._module.args = { + default = lib.mkDefault (assertLazy __curPos); + regular = null; + force = lib.mkForce (assertLazy __curPos); + unused = assertLazy __curPos; + }; + config.result = + assert defs.default.highestPrio == (lib.mkDefault (assertLazy __curPos)).priority; + assert defs.regular.highestPrio == lib.modules.defaultOverridePriority; + assert defs.force.highestPrio == (lib.mkForce (assertLazy __curPos)).priority; + true; +} From 8f31bff7940ed8d8d3e250e6ab7e7bc5b6160f1d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 16 Jun 2023 21:45:09 +0200 Subject: [PATCH 3/4] nixos/nixpkgs: Don't check when _module.args.pkgs is set --- nixos/modules/misc/nixpkgs.nix | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/nixos/modules/misc/nixpkgs.nix b/nixos/modules/misc/nixpkgs.nix index 8a20e31d5137..f9d8bccea284 100644 --- a/nixos/modules/misc/nixpkgs.nix +++ b/nixos/modules/misc/nixpkgs.nix @@ -55,11 +55,6 @@ let description = "An evaluation of Nixpkgs; the top level attribute set of packages"; }; - # Whether `pkgs` was constructed by this module - not if nixpkgs.pkgs or - # _module.args.pkgs is set. However, determining whether _module.args.pkgs - # is defined elsewhere does not seem feasible. - constructedByMe = !opt.pkgs.isDefined; - hasBuildPlatform = opt.buildPlatform.highestPrio < (mkOptionDefault {}).priority; hasHostPlatform = opt.hostPlatform.isDefined; hasPlatform = hasHostPlatform || hasBuildPlatform; @@ -348,7 +343,17 @@ in finalPkgs.__splicedPackages; }; - assertions = [ + assertions = let + # Whether `pkgs` was constructed by this module. This is false when any of + # nixpkgs.pkgs or _module.args.pkgs is set. + constructedByMe = + # We set it with default priority and it can not be merged, so if the + # pkgs module argument has that priority, it's from us. + (lib.modules.mergeAttrDefinitionsWithPrio options._module.args).pkgs.highestPrio + == lib.modules.defaultOverridePriority + # Although, if nixpkgs.pkgs is set, we did forward it, but we did not construct it. + && !opt.pkgs.isDefined; + in [ ( let nixosExpectedSystem = From 895ac176341606688b864bc84312140c065db741 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 23 Jun 2023 18:10:49 +0200 Subject: [PATCH 4/4] lib/modules.nix: Clean up mergeAttrDefinitionsWithPrio impl --- lib/modules.nix | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index c7dcccd7a1dc..381675f611df 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -920,14 +920,7 @@ let option -> attrsOf { highestPrio, value } */ mergeAttrDefinitionsWithPrio = opt: - let subAttrDefs = - lib.concatMap - ({ value, ... }@def: - map - (value: def // { inherit value; }) - (lib.pushDownProperties value) - ) - opt.definitionsWithLocations; + let defsByAttr = lib.zipAttrs ( lib.concatLists ( @@ -935,7 +928,7 @@ let ({ value, ... }@def: map (lib.mapAttrsToList (k: value: { ${k} = def // { inherit value; }; })) - (lib.pushDownProperties value) + (pushDownProperties value) ) opt.definitionsWithLocations )