From 79441600c28c755a302f1e78eceb3c452c3bc7c9 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 27 Sep 2020 17:15:57 +0200 Subject: [PATCH 1/4] lib/tests: Add submodule file propagation test --- lib/tests/modules.sh | 4 ++++ lib/tests/modules/submoduleFiles.nix | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 lib/tests/modules/submoduleFiles.nix diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 487fcd93641b8..36af32ca89da0 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -194,6 +194,10 @@ checkConfigOutput '^"submodule"$' options.submodule.type.description ./declare-s ## Paths should be allowed as values and work as expected checkConfigOutput '^true$' config.submodule.enable ./declare-submoduleWith-path.nix +# Check the file location information is propagated into submodules +checkConfigOutput the-file.nix config.submodule.internalFiles.0 ./submoduleFiles.nix + + # Check that disabledModules works recursively and correctly checkConfigOutput '^true$' config.enable ./disable-recursive/main.nix checkConfigOutput '^true$' config.enable ./disable-recursive/{main.nix,disable-foo.nix} diff --git a/lib/tests/modules/submoduleFiles.nix b/lib/tests/modules/submoduleFiles.nix new file mode 100644 index 0000000000000..c0d9b2cef3e8d --- /dev/null +++ b/lib/tests/modules/submoduleFiles.nix @@ -0,0 +1,21 @@ +{ lib, ... }: { + options.submodule = lib.mkOption { + default = {}; + type = lib.types.submoduleWith { + modules = [ ({ options, ... }: { + options.value = lib.mkOption {}; + + options.internalFiles = lib.mkOption { + default = options.value.files; + }; + })]; + }; + }; + + imports = [ + { + _file = "the-file.nix"; + submodule.value = 10; + } + ]; +} From 907627f6563546c6304703cab94b8cc60b06a12d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 27 Sep 2020 16:39:57 +0200 Subject: [PATCH 2/4] lib/types: Simplify submoduleWith shorthandOnlyDefinesConfig handling The module system already uses the parent module's _file as a fallback, so we don't need to inject the file in a weird way --- lib/types.nix | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/types.nix b/lib/types.nix index 147b92f784c9d..977dd313cf86d 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -580,19 +580,10 @@ rec { let inherit (lib.modules) evalModules; - shorthandToModule = if shorthandOnlyDefinesConfig == false - then value: value - else value: { config = value; }; - allModules = defs: imap1 (n: { value, file }: - if isFunction value - then setFunctionArgs - (args: lib.modules.unifyModuleSyntax file "${toString file}-${toString (n + extensionOffset)}" (value args)) - (functionArgs value) - else if isAttrs value - then - lib.modules.unifyModuleSyntax file "${toString file}-${toString (n + extensionOffset)}" (shorthandToModule value) - else value + if isAttrs value && shorthandOnlyDefinesConfig + then { _file = file; config = value; } + else { _file = file; imports = [ value ]; } ) defs; base = evalModules { From d4a84aeecaa765022a03c3ec03d214b93f46e804 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 14 Jun 2022 17:09:05 +0200 Subject: [PATCH 3/4] lib/types: Use map instead of imap1 in submoduleWith --- lib/types.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/types.nix b/lib/types.nix index 977dd313cf86d..4af75a3d6423b 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -580,7 +580,7 @@ rec { let inherit (lib.modules) evalModules; - allModules = defs: imap1 (n: { value, file }: + allModules = defs: map ({ value, file }: if isAttrs value && shorthandOnlyDefinesConfig then { _file = file; config = value; } else { _file = file; imports = [ value ]; } From 9dead5565a9ce7e25d9dfb7230b885bdaf634177 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 14 Jun 2022 17:25:06 +0200 Subject: [PATCH 4/4] lib/types, lib/modules: Remove unused extensionOffset --- lib/modules.nix | 8 +------- lib/types.nix | 6 ------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 48ddb31508f25..d8ae497fb2d84 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -113,10 +113,6 @@ rec { args ? {} , # This would be remove in the future, Prefer _module.check option instead. check ? true - # Internal variable to avoid `_key` collisions regardless - # of `extendModules`. Used in `submoduleWith`. - # Test case: lib/tests/modules, "168767" - , extensionOffset ? 0 }: let withWarnings = x: @@ -345,17 +341,15 @@ rec { modules ? [], specialArgs ? {}, prefix ? [], - extensionOffset ? length modules, }: evalModules (evalModulesArgs // { modules = regularModules ++ modules; specialArgs = evalModulesArgs.specialArgs or {} // specialArgs; prefix = extendArgs.prefix or evalModulesArgs.prefix or []; - inherit extensionOffset; }); type = lib.types.submoduleWith { - inherit modules specialArgs extensionOffset; + inherit modules specialArgs; }; result = withWarnings { diff --git a/lib/types.nix b/lib/types.nix index 4af75a3d6423b..caaa6dccc6d41 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -571,11 +571,6 @@ rec { , specialArgs ? {} , shorthandOnlyDefinesConfig ? false , description ? null - - # Internal variable to avoid `_key` collisions regardless - # of `extendModules`. Wired through by `evalModules`. - # Test case: lib/tests/modules, "168767" - , extensionOffset ? 0 }@attrs: let inherit (lib.modules) evalModules; @@ -623,7 +618,6 @@ rec { (base.extendModules { modules = [ { _module.args.name = last loc; } ] ++ allModules defs; prefix = loc; - extensionOffset = extensionOffset + length defs; }).config; emptyValue = { value = {}; }; getSubOptions = prefix: (base.extendModules