From 8db712a6a2b5d7f56143a38da14fce85bc0bc97b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Na=C3=AFm=20Favier?= <n@monade.li>
Date: Wed, 23 Mar 2022 05:01:52 +0100
Subject: [PATCH] types: fix `dagOf` behaviour with `mkIf`

This makes definitions like

    home.activation.foo = mkIf false "bar"

work, where previously they would complain about
`home.activation.foobar.data` being used but not defined.

The crucial part is that we don't call `convertAllToDags` in
`dagOf.merge`, because we need to process `mkIf`/`mkMerge` properties
first. So we let `attrEquivalent.merge` do its job normally, but give
it a type `dagEntryOf` that does the conversion.

Ideally this shouldn't require so much boilerplate; I'd like to
implement something like

    types.changeInto dagContentType elemType dagEntryAnywhere

in Nixpkgs.
---
 modules/lib/types-dag.nix     | 43 +++++++++++++++++++----------------
 tests/lib/types/dag-merge.nix | 14 ++++++++----
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/modules/lib/types-dag.nix b/modules/lib/types-dag.nix
index 68a5826f..366cc2be 100644
--- a/modules/lib/types-dag.nix
+++ b/modules/lib/types-dag.nix
@@ -8,17 +8,27 @@ let
 
   isDagEntry = e: isAttrs e && (e ? data) && (e ? after) && (e ? before);
 
-  dagContentType = elemType:
-    types.submodule ({ name, ... }: {
-      options = {
-        data = mkOption { type = elemType; };
-        after = mkOption { type = with types; uniq (listOf str); };
-        before = mkOption { type = with types; uniq (listOf str); };
-      };
-      config = mkIf (elemType.name == "submodule") {
-        data._module.args.dagName = name;
-      };
-    });
+  dagEntryOf = elemType:
+    let
+      submoduleType = types.submodule ({ name, ... }: {
+        options = {
+          data = mkOption { type = elemType; };
+          after = mkOption { type = with types; uniq (listOf str); };
+          before = mkOption { type = with types; uniq (listOf str); };
+        };
+        config = mkIf (elemType.name == "submodule") {
+          data._module.args.dagName = name;
+        };
+      });
+      maybeConvert = v: if isDagEntry v then v else dag.entryAnywhere v;
+    in mkOptionType {
+      name = "dagEntryOf";
+      description = "DAG entry of ${elemType.description}";
+      # leave the checking to the submodule type
+      merge = loc: defs:
+        submoduleType.merge loc
+        (map (def: def // { value = maybeConvert def.value; }) defs);
+    };
 
 in rec {
   # A directed acyclic graph of some inner type.
@@ -29,21 +39,16 @@ in rec {
   # "actual" attribute name a new submodule argument is provided with
   # the name `dagName`.
   dagOf = elemType:
-    let
-      convertAllToDags = let
-        maybeConvert = n: v: if isDagEntry v then v else dag.entryAnywhere v;
-      in map (def: def // { value = mapAttrs maybeConvert def.value; });
-
-      attrEquivalent = types.attrsOf (dagContentType elemType);
+    let attrEquivalent = types.attrsOf (dagEntryOf elemType);
     in mkOptionType rec {
       name = "dagOf";
       description = "DAG of ${elemType.description}s";
-      check = isAttrs;
-      merge = loc: defs: attrEquivalent.merge loc (convertAllToDags defs);
+      inherit (attrEquivalent) check merge emptyValue;
       getSubOptions = prefix: elemType.getSubOptions (prefix ++ [ "<name>" ]);
       getSubModules = elemType.getSubModules;
       substSubModules = m: dagOf (elemType.substSubModules m);
       functor = (defaultFunctor name) // { wrapped = elemType; };
+      nestedTypes.elemType = elemType;
     };
 
   # A directed acyclic graph of some inner type OR a list of that
diff --git a/tests/lib/types/dag-merge.nix b/tests/lib/types/dag-merge.nix
index 5e361cd7..0b41ec8a 100644
--- a/tests/lib/types/dag-merge.nix
+++ b/tests/lib/types/dag-merge.nix
@@ -1,7 +1,7 @@
 { config, lib, pkgs, ... }:
 
 let
-  inherit (lib) concatStringsSep hm mkMerge mkOption types;
+  inherit (lib) concatStringsSep hm mkIf mkMerge mkOption types;
 
   dag = lib.hm.dag;
 
@@ -14,10 +14,14 @@ in {
   options.tested.dag = mkOption { type = hm.types.dagOf types.str; };
 
   config = {
-    tested = mkMerge [
-      { dag.after = "after"; }
-      { dag.before = dag.entryBefore [ "after" ] "before"; }
-      { dag.between = dag.entryBetween [ "after" ] [ "before" ] "between"; }
+    tested.dag = mkMerge [
+      { never = mkIf false "never"; }
+      { after = mkMerge [ "after" (mkIf false "neither") ]; }
+      { before = dag.entryBefore [ "after" ] (mkIf true "before"); }
+      {
+        between =
+          mkIf true (dag.entryBetween [ "after" ] [ "before" ] "between");
+      }
     ];
 
     home.file."result.txt".text = result;