about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRobert Hensing <robert@roberthensing.nl>2023-01-21 00:58:47 +0100
committerRobert Hensing <robert@roberthensing.nl>2023-03-01 15:03:44 +0100
commit118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907 (patch)
treecc6cf8beac44afd968048edc568bd2ffe9bcd5b7
parent82b4c24705340bceed0d463d3d897b6ebcbd7550 (diff)
downloadnixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar.gz
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar.bz2
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar.lz
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar.xz
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.tar.zst
nixlib-118bdf25a6c572dd2fd29d10b1ae2e4d9a95b907.zip
lib/modules: Allow an "anonymous" module with key in disabledModules
This makes the following work

    disabledModules = [ foo.nixosModules.bar ];

even if `bar` is not a path, but rather a module such as

    { key = "/path/to/foo#nixosModules.bar"; config = ...; }

By supporting this, the user will often be able to use the same syntax
for both importing and disabling a module. This is becoming more relevant
because flakes promote the use of attributes to reference modules. Not
all of these modules in flake attributes will be identifiable, but with
the help of a framework such as flake-parts, these attributes can be
guaranteed to be identifiable (by outPath + attribute path).
-rw-r--r--lib/modules.nix34
-rwxr-xr-xlib/tests/modules.sh12
-rw-r--r--lib/tests/modules/disable-module-bad-key.nix16
-rw-r--r--lib/tests/modules/disable-module-with-key.nix34
-rw-r--r--lib/tests/modules/disable-module-with-toString-key.nix34
-rw-r--r--lib/tests/modules/merge-module-with-key.nix49
-rw-r--r--nixos/doc/manual/development/replace-modules.section.md11
7 files changed, 183 insertions, 7 deletions
diff --git a/lib/modules.nix b/lib/modules.nix
index 5e6bee6aabe3..051dbe2ef896 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -21,6 +21,7 @@ let
     isBool
     isFunction
     isList
+    isPath
     isString
     length
     mapAttrs
@@ -45,6 +46,9 @@ let
     showOption
     unknownModule
     ;
+  inherit (lib.strings)
+    isConvertibleWithToString
+    ;
 
   showDeclPrefix = loc: decl: prefix:
     " - option(s) with prefix `${showOption (loc ++ [prefix])}' in module `${decl._file}'";
@@ -403,7 +407,7 @@ rec {
             key = module.key;
             module = module;
             modules = collectedImports.modules;
-            disabled = module.disabledModules ++ collectedImports.disabled;
+            disabled = (if module.disabledModules != [] then [{ file = module._file; disabled = module.disabledModules; }] else []) ++ collectedImports.disabled;
           }) initialModules);
 
       # filterModules :: String -> { disabled, modules } -> [ Module ]
@@ -412,10 +416,30 @@ rec {
       # modules recursively. It returns the final list of unique-by-key modules
       filterModules = modulesPath: { disabled, modules }:
         let
-          moduleKey = m: if isString m && (builtins.substring 0 1 m != "/")
-            then toString modulesPath + "/" + m
-            else toString m;
-          disabledKeys = map moduleKey disabled;
+          moduleKey = file: m:
+            if isString m
+            then
+              if builtins.substring 0 1 m == "/"
+              then m
+              else toString modulesPath + "/" + m
+
+            else if isConvertibleWithToString m
+            then
+              if m?key && m.key != toString m
+              then
+                throw "Module `${file}` contains a disabledModules item that is an attribute set that can be converted to a string (${toString m}) but also has a `.key` attribute (${m.key}) with a different value. This makes it ambiguous which module should be disabled."
+              else
+                toString m
+
+            else if m?key
+            then
+              m.key
+
+            else if isAttrs m
+            then throw "Module `${file}` contains a disabledModules item that is an attribute set, presumably a module, that does not have a `key` attribute. This means that the module system doesn't have any means to identify the module that should be disabled. Make sure that you've put the correct value in disabledModules: a string path relative to modulesPath, a path value, or an attribute set with a `key` attribute."
+            else throw "Each disabledModules item must be a path, string, or a attribute set with a key attribute, or a value supported by toString. However, one of the disabledModules items in `${toString file}` is none of that, but is of type ${builtins.typeOf m}.";
+
+          disabledKeys = concatMap ({ file, disabled }: map (moduleKey file) disabled) disabled;
           keyFilter = filter (attrs: ! elem attrs.key disabledKeys);
         in map (attrs: attrs.module) (builtins.genericClosure {
           startSet = keyFilter modules;
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index cde4da643937..8081b186a2f9 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -141,6 +141,14 @@ checkConfigError "The option .*enable.* does not exist. Definition values:\n\s*-
 checkConfigError "attribute .*enable.* in selection path .*config.enable.* not found" "$@" ./disable-define-enable.nix ./disable-declare-enable.nix
 checkConfigError "attribute .*enable.* in selection path .*config.enable.* not found" "$@" ./disable-enable-modules.nix
 
+checkConfigOutput '^true$' 'config.positive.enable' ./disable-module-with-key.nix
+checkConfigOutput '^false$' 'config.negative.enable' ./disable-module-with-key.nix
+checkConfigError 'Module ..*disable-module-bad-key.nix. contains a disabledModules item that is an attribute set, presumably a module, that does not have a .key. attribute. .*' 'config.enable' ./disable-module-bad-key.nix
+
+# Not sure if we want to keep supporting module keys that aren't strings, paths or v?key, but we shouldn't remove support accidentally.
+checkConfigOutput '^true$' 'config.positive.enable' ./disable-module-with-toString-key.nix
+checkConfigOutput '^false$' 'config.negative.enable' ./disable-module-with-toString-key.nix
+
 # Check _module.args.
 set -- config.enable ./declare-enable.nix ./define-enable-with-custom-arg.nix
 checkConfigError 'while evaluating the module argument .*custom.* in .*define-enable-with-custom-arg.nix.*:' "$@"
@@ -358,6 +366,10 @@ checkConfigOutput '^"The option `a\.b. defined in `.*/doRename-warnings\.nix. ha
   config.result \
   ./doRename-warnings.nix
 
+# Anonymous modules get deduplicated by key
+checkConfigOutput '^"pear"$' config.once.raw ./merge-module-with-key.nix
+checkConfigOutput '^"pear\\npear"$' config.twice.raw ./merge-module-with-key.nix
+
 cat <<EOF
 ====== module tests ======
 $pass Pass
diff --git a/lib/tests/modules/disable-module-bad-key.nix b/lib/tests/modules/disable-module-bad-key.nix
new file mode 100644
index 000000000000..f50d06f2f03c
--- /dev/null
+++ b/lib/tests/modules/disable-module-bad-key.nix
@@ -0,0 +1,16 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+
+  moduleWithKey = { config, ... }: {
+    config = {
+      enable = true;
+    };
+  };
+in
+{
+  imports = [
+    ./declare-enable.nix
+  ];
+  disabledModules = [ { } ];
+}
diff --git a/lib/tests/modules/disable-module-with-key.nix b/lib/tests/modules/disable-module-with-key.nix
new file mode 100644
index 000000000000..ea2a60aa832d
--- /dev/null
+++ b/lib/tests/modules/disable-module-with-key.nix
@@ -0,0 +1,34 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+
+  moduleWithKey = {
+    key = "disable-module-with-key.nix#moduleWithKey";
+    config = {
+      enable = true;
+    };
+  };
+in
+{
+  options = {
+    positive = mkOption {
+      type = types.submodule {
+        imports = [
+          ./declare-enable.nix
+          moduleWithKey
+        ];
+      };
+      default = {};
+    };
+    negative = mkOption {
+      type = types.submodule {
+        imports = [
+          ./declare-enable.nix
+          moduleWithKey
+        ];
+        disabledModules = [ moduleWithKey ];
+      };
+      default = {};
+    };
+  };
+}
diff --git a/lib/tests/modules/disable-module-with-toString-key.nix b/lib/tests/modules/disable-module-with-toString-key.nix
new file mode 100644
index 000000000000..3f8c81904ce6
--- /dev/null
+++ b/lib/tests/modules/disable-module-with-toString-key.nix
@@ -0,0 +1,34 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+
+  moduleWithKey = {
+    key = 123;
+    config = {
+      enable = true;
+    };
+  };
+in
+{
+  options = {
+    positive = mkOption {
+      type = types.submodule {
+        imports = [
+          ./declare-enable.nix
+          moduleWithKey
+        ];
+      };
+      default = {};
+    };
+    negative = mkOption {
+      type = types.submodule {
+        imports = [
+          ./declare-enable.nix
+          moduleWithKey
+        ];
+        disabledModules = [ 123 ];
+      };
+      default = {};
+    };
+  };
+}
diff --git a/lib/tests/modules/merge-module-with-key.nix b/lib/tests/modules/merge-module-with-key.nix
new file mode 100644
index 000000000000..21f00e6ef976
--- /dev/null
+++ b/lib/tests/modules/merge-module-with-key.nix
@@ -0,0 +1,49 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+
+  moduleWithoutKey = {
+    config = {
+      raw = "pear";
+    };
+  };
+
+  moduleWithKey = {
+    key = __curPos.file + "#moduleWithKey";
+    config = {
+      raw = "pear";
+    };
+  };
+
+  decl = {
+    options = {
+      raw = mkOption {
+        type = types.lines;
+      };
+    };
+  };
+in
+{
+  options = {
+    once = mkOption {
+      type = types.submodule {
+        imports = [
+          decl
+          moduleWithKey
+          moduleWithKey
+        ];
+      };
+      default = {};
+    };
+    twice = mkOption {
+      type = types.submodule {
+        imports = [
+          decl
+          moduleWithoutKey
+          moduleWithoutKey
+        ];
+      };
+      default = {};
+    };
+  };
+}
diff --git a/nixos/doc/manual/development/replace-modules.section.md b/nixos/doc/manual/development/replace-modules.section.md
index 0c0d6a7ac2f1..ac9f5adbaf98 100644
--- a/nixos/doc/manual/development/replace-modules.section.md
+++ b/nixos/doc/manual/development/replace-modules.section.md
@@ -8,8 +8,15 @@ the system on a stable release.
 
 `disabledModules` is a top level attribute like `imports`, `options` and
 `config`. It contains a list of modules that will be disabled. This can
-either be the full path to the module or a string with the filename
-relative to the modules path (eg. \<nixpkgs/nixos/modules> for nixos).
+either be:
+ - the full path to the module,
+ - or a string with the filename relative to the modules path (eg. \<nixpkgs/nixos/modules> for nixos),
+ - or an attribute set containing a specific `key` attribute.
+
+The latter allows some modules to be disabled, despite them being distributed
+via attributes instead of file paths. The `key` should be globally unique, so
+it is recommended to include a file path in it, or rely on a framework to do it
+for you.
 
 This example will replace the existing postgresql module with the
 version defined in the nixos-unstable channel while keeping the rest of