From a6b455bbaecb42679e5d1ade51433d62ee21bd03 Mon Sep 17 00:00:00 2001
From: "Nicolas B. Pierron" <nicolas.b.pierron@gmail.com>
Date: Wed, 13 May 2015 22:44:04 +0200
Subject: NixOS modules: Add error context on module arguments evaluation.

---
 lib/modules.nix                                     | 19 ++++++++++++++-----
 lib/tests/modules.sh                                | 12 ++++++++++--
 lib/tests/modules/custom-arg-define-enable.nix      |  8 --------
 lib/tests/modules/define-_module-args-custom.nix    |  7 +++++++
 lib/tests/modules/define-enable-with-custom-arg.nix |  7 +++++++
 lib/tests/modules/import-custom-arg.nix             |  6 ++++++
 6 files changed, 44 insertions(+), 15 deletions(-)
 delete mode 100644 lib/tests/modules/custom-arg-define-enable.nix
 create mode 100644 lib/tests/modules/define-_module-args-custom.nix
 create mode 100644 lib/tests/modules/define-enable-with-custom-arg.nix
 create mode 100644 lib/tests/modules/import-custom-arg.nix

diff --git a/lib/modules.nix b/lib/modules.nix
index 73dbdd371f6..ea600127617 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -91,9 +91,11 @@ rec {
     let
       toClosureList = file: parentKey: imap (n: x:
         if isAttrs x || isFunction x then
-          unifyModuleSyntax file "${parentKey}:anon-${toString n}" (unpackSubmodule applyIfFunction x args)
+          let key = "${parentKey}:anon-${toString n}"; in
+          unifyModuleSyntax file key (unpackSubmodule (applyIfFunction key) x args)
         else
-          unifyModuleSyntax (toString x) (toString x) (applyIfFunction (import x) args));
+          let file = toString x; key = toString x; in
+          unifyModuleSyntax file key (applyIfFunction key (import x) args));
     in
       builtins.genericClosure {
         startSet = toClosureList unknownModule "" modules;
@@ -122,7 +124,7 @@ rec {
         config = removeAttrs m ["key" "_file" "require" "imports"];
       };
 
-  applyIfFunction = f: arg@{ config, options, lib, ... }: if isFunction f then
+  applyIfFunction = key: f: args@{ config, options, lib, ... }: if isFunction f then
     let
       # Module arguments are resolved in a strict manner when attribute set
       # deconstruction is used.  As the arguments are now defined with the
@@ -137,11 +139,18 @@ rec {
       # not their values.  The values are forwarding the result of the
       # evaluation of the option.
       requiredArgs = builtins.attrNames (builtins.functionArgs f);
+      context = name: ''while evaluating the module argument `${name}' in "${key}":'';
       extraArgs = builtins.listToAttrs (map (name: {
         inherit name;
-        value = config._module.args.${name};
+        value = addErrorContext (context name)
+          (args.${name} or config._module.args.${name});
       }) requiredArgs);
-    in f (extraArgs // arg)
+
+      # Note: we append in the opposite order such that we can add an error
+      # context on the explicited arguments of "args" too. This update
+      # operator is used to make the "args@{ ... }: with args.lib;" notation
+      # works.
+    in f (args // extraArgs)
   else
     f;
 
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index 66c6f560fbe..65de8e378c7 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -12,7 +12,7 @@ evalConfig() {
     local attr=$1
     shift;
     local script="import ./default.nix { modules = [ $@ ];}"
-    nix-instantiate --timeout 1 -E "$script" -A "$attr" --eval-only
+    nix-instantiate --timeout 1 -E "$script" -A "$attr" --eval-only --show-trace
 }
 
 reportFailure() {
@@ -100,7 +100,15 @@ checkConfigOutput 'true' "$@" ./define-enable.nix ./define-loaOfSub-foo-if-enabl
 checkConfigOutput 'true' "$@" ./define-enable.nix ./define-loaOfSub-foo-enable-if.nix
 
 # Check _module.args.
-checkConfigOutput "true" config.enable ./declare-enable.nix ./custom-arg-define-enable.nix
+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.*:' "$@"
+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' "$@"
 
 # Check _module.check.
 set -- config.enable ./declare-enable.nix ./define-enable.nix ./define-loaOfSub-foo.nix
diff --git a/lib/tests/modules/custom-arg-define-enable.nix b/lib/tests/modules/custom-arg-define-enable.nix
deleted file mode 100644
index f04d30dd9b9..00000000000
--- a/lib/tests/modules/custom-arg-define-enable.nix
+++ /dev/null
@@ -1,8 +0,0 @@
-{ lib, custom, ... }:
-
-{
-  config = {
-    _module.args.custom = true;
-    enable = custom;
-  };
-}
diff --git a/lib/tests/modules/define-_module-args-custom.nix b/lib/tests/modules/define-_module-args-custom.nix
new file mode 100644
index 00000000000..e565fd215a5
--- /dev/null
+++ b/lib/tests/modules/define-_module-args-custom.nix
@@ -0,0 +1,7 @@
+{ lib, ... }:
+
+{
+  config = {
+    _module.args.custom = true;
+  };
+}
diff --git a/lib/tests/modules/define-enable-with-custom-arg.nix b/lib/tests/modules/define-enable-with-custom-arg.nix
new file mode 100644
index 00000000000..7da74671d14
--- /dev/null
+++ b/lib/tests/modules/define-enable-with-custom-arg.nix
@@ -0,0 +1,7 @@
+{ lib, custom, ... }:
+
+{
+  config = {
+    enable = custom;
+  };
+}
diff --git a/lib/tests/modules/import-custom-arg.nix b/lib/tests/modules/import-custom-arg.nix
new file mode 100644
index 00000000000..3e687b661c1
--- /dev/null
+++ b/lib/tests/modules/import-custom-arg.nix
@@ -0,0 +1,6 @@
+{ lib, custom, ... }:
+
+{
+  imports = []
+  ++ lib.optional custom ./define-enable-force.nix;
+}
-- 
cgit 1.4.1