From 7a787e122bb704cf5a889ccdb3e1fcd773480f49 Mon Sep 17 00:00:00 2001 From: 49016 <49016@duck.com> Date: Fri, 1 Aug 2025 21:33:33 +0200 Subject: [PATCH 1/6] lib.modules: Add hint when using `config` in `imports` --- lib/modules.nix | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/modules.nix b/lib/modules.nix index 36a2ee5c4181..437f8298a58e 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -637,6 +637,15 @@ let key: f: args@{ config, ... }: let + importHint = + name: + if name == "config" then + "\n\n" + + '' + … If you get an infinite recursion here, you probably reference `config` + in `imports`. This is not supported; consider using mkEnableOption.'' + else + ""; # Module arguments are resolved in a strict manner when attribute set # deconstruction is used. As the arguments are now defined with the # config._module.args option, the strictness used on the attribute @@ -649,7 +658,7 @@ let # a module will resolve strictly the attributes used as argument but # not their values. The values are forwarding the result of the # evaluation of the option. - context = name: ''while evaluating the module argument `${name}' in "${key}":''; + context = name: ''while evaluating the module argument `${name}' in "${key}":${importHint name}''; extraArgs = mapAttrs ( name: _: addErrorContext (context name) (args.${name} or config._module.args.${name}) ) (functionArgs f); From e28f3f0cd018d60a3793cb705b5a24f76b78dedd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 2 Aug 2025 10:40:00 +0200 Subject: [PATCH 2/6] lib.modules: Test infinite recursion hint We don't want it to occur in the trace of any unrelated errors. --- lib/tests/modules.sh | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index dac8b0e33676..8b021ab6980a 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -82,6 +82,30 @@ checkConfigOutput() { fi } +invertIfUnset() { + gate="$1" + shift + if [[ -n "${!gate:-}" ]]; then + "$@" + else + ! "$@" + fi +} + +globalErrorLogCheck() { + invertIfUnset "REQUIRE_INFINITE_RECURSION_HINT" \ + grep -i 'if you get an infinite recursion here' \ + <<<"$err" >/dev/null \ + || { + if [[ -n "${REQUIRE_INFINITE_RECURSION_HINT:-}" ]]; then + echo "Unexpected infinite recursion hint" + else + echo "Expected infinite recursion hint, but none found" + fi + return 1 + } +} + checkConfigError() { local errorContains=$1 local err="" @@ -94,6 +118,14 @@ checkConfigError() { logFailure logEndFailure else + if ! globalErrorLogCheck "$err"; then + logStartFailure + echo "LOG:" + reportFailure "$@" + echo "GLOBAL ERROR LOG CHECK FAILED" + logFailure + logEndFailure + fi if echo "$err" | grep -zP --silent "$errorContains" ; then ((++pass)) else @@ -488,7 +520,7 @@ checkConfigOutput '^"bar"$' config.nest.bar ./freeform-attrsOf.nix ./freeform-ne checkConfigOutput '^null$' config.foo ./freeform-attrsOf.nix ./freeform-str-dep-unstr.nix checkConfigOutput '^"24"$' config.foo ./freeform-attrsOf.nix ./freeform-str-dep-unstr.nix ./define-value-string.nix # Check whether an freeform-typed value can depend on a declared option, this can only work with lazyAttrsOf -checkConfigError 'infinite recursion encountered' config.foo ./freeform-attrsOf.nix ./freeform-unstr-dep-str.nix +REQUIRE_INFINITE_RECURSION_HINT=1 checkConfigError 'infinite recursion encountered' config.foo ./freeform-attrsOf.nix ./freeform-unstr-dep-str.nix checkConfigError 'The option .* was accessed but has no value defined. Try setting the option.' config.foo ./freeform-lazyAttrsOf.nix ./freeform-unstr-dep-str.nix checkConfigOutput '^"24"$' config.foo ./freeform-lazyAttrsOf.nix ./freeform-unstr-dep-str.nix ./define-value-string.nix # submodules in freeformTypes should have their locations annotated From 9dad048f2118dd8678b8fbf69b7355b9a399d3cf Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 2 Aug 2025 10:51:09 +0200 Subject: [PATCH 3/6] lib.modules: Generalize the import hint to _module.args --- lib/modules.nix | 14 +++----------- lib/tests/modules.sh | 4 ++-- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 437f8298a58e..d723bf1656f2 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -254,11 +254,12 @@ let inherit lib options - config specialArgs ; _class = class; _prefix = prefix; + config = builtins.addErrorContext "If you get an infinite recursion here, you probably reference `config` + in `imports`. This is not supported; consider using mkEnableOption." config; } // specialArgs ); @@ -637,15 +638,6 @@ let key: f: args@{ config, ... }: let - importHint = - name: - if name == "config" then - "\n\n" - + '' - … If you get an infinite recursion here, you probably reference `config` - in `imports`. This is not supported; consider using mkEnableOption.'' - else - ""; # Module arguments are resolved in a strict manner when attribute set # deconstruction is used. As the arguments are now defined with the # config._module.args option, the strictness used on the attribute @@ -658,7 +650,7 @@ let # a module will resolve strictly the attributes used as argument but # not their values. The values are forwarding the result of the # evaluation of the option. - context = name: ''while evaluating the module argument `${name}' in "${key}":${importHint name}''; + context = name: ''while evaluating the module argument `${name}' in "${key}":''; extraArgs = mapAttrs ( name: _: addErrorContext (context name) (args.${name} or config._module.args.${name}) ) (functionArgs f); diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index 8b021ab6980a..a4aa5201715c 100755 --- a/lib/tests/modules.sh +++ b/lib/tests/modules.sh @@ -315,8 +315,8 @@ checkConfigOutput '^true$' "$@" ./define-_module-args-custom.nix # Check that using _module.args on imports cause infinite recursions, with # the proper error context. set -- "$@" ./define-_module-args-custom.nix ./import-custom-arg.nix -checkConfigError 'while evaluating the module argument .*custom.* in .*import-custom-arg.nix.*:' "$@" -checkConfigError 'infinite recursion encountered' "$@" +REQUIRE_INFINITE_RECURSION_HINT=1 checkConfigError 'while evaluating the module argument .*custom.* in .*import-custom-arg.nix.*:' "$@" +REQUIRE_INFINITE_RECURSION_HINT=1 checkConfigError 'infinite recursion encountered' "$@" # Check _module.check. set -- config.enable ./declare-enable.nix ./define-enable.nix ./define-attrsOfSub-foo.nix From 3d15b12d8f7acd0764a3cf08a2b3b58db4449a26 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 2 Aug 2025 10:59:55 +0200 Subject: [PATCH 4/6] lib.modules: Make _module.args evaluation explicit in trace --- lib/modules.nix | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/modules.nix b/lib/modules.nix index d723bf1656f2..dc4513da8e0a 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -652,7 +652,13 @@ let # evaluation of the option. context = name: ''while evaluating the module argument `${name}' in "${key}":''; extraArgs = mapAttrs ( - name: _: addErrorContext (context name) (args.${name} or config._module.args.${name}) + name: _: + addErrorContext (context name) ( + args.${name} or (addErrorContext + "noting that argument `${name}` is not externally provided, so querying `_module.args` instead, requiring `config`" + config._module.args.${name} + ) + ) ) (functionArgs f); # Note: we append in the opposite order such that we can add an error From c34e08489ec87a23cd18dd4f6035c840071995b9 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 2 Aug 2025 11:01:14 +0200 Subject: [PATCH 5/6] lib.modules: Adjust error message - Lower case error trace for consistency - Be more explicit about the condition under which the hint applies, and the resolution. --- lib/modules.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index dc4513da8e0a..ad8a381e97f7 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -258,8 +258,7 @@ let ; _class = class; _prefix = prefix; - config = builtins.addErrorContext "If you get an infinite recursion here, you probably reference `config` - in `imports`. This is not supported; consider using mkEnableOption." config; + config = addErrorContext "if you get an infinite recursion here, you probably reference `config` in `imports`. This is not possible. If you are trying to achieve a conditional behavior dependent on `config`, consider importing unconditionally, and using `mkEnableOption` and `mkIf` to control its effect." config; } // specialArgs ); From 5620fc678ed474f28b5691fd7111ab856ed02e1b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 4 Aug 2025 10:05:26 +0200 Subject: [PATCH 6/6] lib.modules: Improve infinite recursion hint hsjobeki: Using config in imports is possible in general. But its not possible to do conditional imports where the condition depends on config. Thats two different statements. Co-authored-by: Johannes Kirschbauer --- lib/modules.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules.nix b/lib/modules.nix index ad8a381e97f7..c55d276d4970 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -258,7 +258,7 @@ let ; _class = class; _prefix = prefix; - config = addErrorContext "if you get an infinite recursion here, you probably reference `config` in `imports`. This is not possible. If you are trying to achieve a conditional behavior dependent on `config`, consider importing unconditionally, and using `mkEnableOption` and `mkIf` to control its effect." config; + config = addErrorContext "if you get an infinite recursion here, you probably reference `config` in `imports`. If you are trying to achieve a conditional import behavior dependent on `config`, consider importing unconditionally, and using `mkEnableOption` and `mkIf` to control its effect." config; } // specialArgs );