From d6ebd537e5d212995984152d57e16029b3726de5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 31 Oct 2021 19:10:13 +0100 Subject: [PATCH 1/6] lib/modules: Short-circuit unmatchedDefns when configs is empty --- lib/modules.nix | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 46ae3f136310..b2ae51c8e618 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -430,10 +430,16 @@ rec { # an attrset 'name' => list of unmatched definitions for 'name' unmatchedDefnsByName = - # Propagate all unmatched definitions from nested option sets - mapAttrs (n: v: v.unmatchedDefns) resultsByName - # Plus the definitions for the current prefix that don't have a matching option - // removeAttrs defnsByName' (attrNames matchedOptions); + if configs == [] + then + # When no config values exist, there can be no unmatched config, so + # we short circuit and avoid evaluating more _options_ than necessary. + {} + else + # Propagate all unmatched definitions from nested option sets + mapAttrs (n: v: v.unmatchedDefns) resultsByName + # Plus the definitions for the current prefix that don't have a matching option + // removeAttrs defnsByName' (attrNames matchedOptions); in { inherit matchedOptions; From bfaa9426c0e70b387f58bce6248b454b556018c2 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 3 Nov 2021 19:05:26 +0100 Subject: [PATCH 2/6] lib/modules: Short-circuit unmatchedDefns earlier --- lib/modules.nix | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index b2ae51c8e618..4bbf2947bc4e 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -430,26 +430,27 @@ rec { # an attrset 'name' => list of unmatched definitions for 'name' unmatchedDefnsByName = - if configs == [] - then - # When no config values exist, there can be no unmatched config, so - # we short circuit and avoid evaluating more _options_ than necessary. - {} - else - # Propagate all unmatched definitions from nested option sets - mapAttrs (n: v: v.unmatchedDefns) resultsByName - # Plus the definitions for the current prefix that don't have a matching option - // removeAttrs defnsByName' (attrNames matchedOptions); + # Propagate all unmatched definitions from nested option sets + mapAttrs (n: v: v.unmatchedDefns) resultsByName + # Plus the definitions for the current prefix that don't have a matching option + // removeAttrs defnsByName' (attrNames matchedOptions); in { inherit matchedOptions; # Transforms unmatchedDefnsByName into a list of definitions - unmatchedDefns = concatLists (mapAttrsToList (name: defs: - map (def: def // { - # Set this so we know when the definition first left unmatched territory - prefix = [name] ++ (def.prefix or []); - }) defs - ) unmatchedDefnsByName); + unmatchedDefns = + if configs == [] + then + # When no config values exist, there can be no unmatched config, so + # we short circuit and avoid evaluating more _options_ than necessary. + [] + else + concatLists (mapAttrsToList (name: defs: + map (def: def // { + # Set this so we know when the definition first left unmatched territory + prefix = [name] ++ (def.prefix or []); + }) defs + ) unmatchedDefnsByName); }; /* Merge multiple option declarations into a single declaration. In From e8d61a25fcb5e11da8af0792da27a896bf87ba65 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 3 Nov 2021 19:19:41 +0100 Subject: [PATCH 3/6] lib/tests/modules: Test non-strictness some more Doesn't seem to have been a problem actually, but now it won't regress. --- lib/tests/modules/declare-attrsOf.nix | 9 ++++++++- lib/tests/modules/freeform-nested.nix | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/tests/modules/declare-attrsOf.nix b/lib/tests/modules/declare-attrsOf.nix index b3999de7e5fb..d19964064b21 100644 --- a/lib/tests/modules/declare-attrsOf.nix +++ b/lib/tests/modules/declare-attrsOf.nix @@ -1,6 +1,13 @@ -{ lib, ... }: { +{ lib, ... }: +let + deathtrapArgs = lib.mapAttrs + (k: _: throw "The module system is too strict, accessing an unused option's ${k} mkOption-attribute.") + (lib.functionArgs lib.mkOption); +in +{ options.value = lib.mkOption { type = lib.types.attrsOf lib.types.str; default = {}; }; + options.testing-laziness-so-don't-read-me = lib.mkOption deathtrapArgs; } diff --git a/lib/tests/modules/freeform-nested.nix b/lib/tests/modules/freeform-nested.nix index 5da27f5a8b4f..b81fa7f0d222 100644 --- a/lib/tests/modules/freeform-nested.nix +++ b/lib/tests/modules/freeform-nested.nix @@ -1,7 +1,14 @@ -{ lib, ... }: { +{ lib, ... }: +let + deathtrapArgs = lib.mapAttrs + (k: _: throw "The module system is too strict, accessing an unused option's ${k} mkOption-attribute.") + (lib.functionArgs lib.mkOption); +in +{ options.nest.foo = lib.mkOption { type = lib.types.bool; default = false; }; + options.nest.unused = lib.mkOption deathtrapArgs; config.nest.bar = "bar"; } From 8b584158a5cf3f00ee87297030874dbe35373037 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 3 Nov 2021 19:34:27 +0100 Subject: [PATCH 4/6] lib/modules: Remove a lib.flip In hot code, the overhead (envs, applies) can matter. --- lib/modules.nix | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 4bbf2947bc4e..fd90c51b9425 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -13,7 +13,6 @@ let elem filter findFirst - flip foldl foldl' getAttrFromPath @@ -403,7 +402,7 @@ rec { [{ inherit (module) file; inherit value; }] ) configs; - resultsByName = flip mapAttrs declsByName (name: decls: + resultsByName = mapAttrs (name: decls: # We're descending into attribute ‘name’. let loc = prefix ++ [name]; @@ -424,7 +423,7 @@ rec { in throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'." else - mergeModules' loc decls defns); + mergeModules' loc decls defns) declsByName; matchedOptions = mapAttrs (n: v: v.matchedOptions) resultsByName; From 541ce53a3bfa9c769ce4aa755d1c452cb3bdc2dd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 3 Nov 2021 19:39:31 +0100 Subject: [PATCH 5/6] lib/modules: Fix import* comments Very confusing otherwise. --- lib/modules.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index fd90c51b9425..99f51d22dcd1 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -947,7 +947,7 @@ rec { /* Use this function to import a JSON file as NixOS configuration. - importJSON -> path -> attrs + modules.importJSON :: path -> attrs */ importJSON = file: { _file = file; @@ -956,7 +956,7 @@ rec { /* Use this function to import a TOML file as NixOS configuration. - importTOML -> path -> attrs + modules.importTOML :: path -> attrs */ importTOML = file: { _file = file; From 844a9e746f92ac7278ece51c2b7819d0e6c5c05d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 3 Nov 2021 19:45:06 +0100 Subject: [PATCH 6/6] lib/modules: Use strict fold' as recursiveUpdate is also strict recursiveUpdate does not produce an attrset until it has evaluated both its arguments to weak head normal form. nix-repl> lib.recursiveUpdate (throw "a") (throw "b") error: b nix-repl> lib.recursiveUpdate (throw "a") {} error: a --- lib/modules.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 99f51d22dcd1..e3abf846625d 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -13,7 +13,6 @@ let elem filter findFirst - foldl foldl' getAttrFromPath head @@ -863,7 +862,7 @@ rec { mkMergedOptionModule = from: to: mergeFn: { config, options, ... }: { - options = foldl recursiveUpdate {} (map (path: setAttrByPath path (mkOption { + options = foldl' recursiveUpdate {} (map (path: setAttrByPath path (mkOption { visible = false; # To use the value in mergeFn without triggering errors default = "_mkMergedOptionModule";