diff --git a/nix/lib/entities/_types.nix b/nix/lib/entities/_types.nix index 5bf9277aa..3e8048ff5 100644 --- a/nix/lib/entities/_types.nix +++ b/nix/lib/entities/_types.nix @@ -64,11 +64,81 @@ let # Recursive merge without forcing leaf values. Unlike lib.types.anything this # does not inspect values deeply (no mapAttrsRecursiveCond), avoiding infinite # recursion when values reference other options (e.g. den.aspects). + # Concatenates lists rather than overwriting them, which `lib.recursiveUpdate` + # does because it treats a list as an opaque leaf. Two files each writing + # `den.hosts..includes` therefore kept one list and dropped the other in + # silence, while an attrset key under the same two definitions merged + # normally (measured: `users.alice` and `users.bob` both survive). + # + # Concatenation is the module system's own rule for a list-valued option + # (`listOf` merges by `concatLists`) and den's rule at the aspect tier + # (`aspectContentType`'s `deepMerge`), so this makes the entity registry + # agree with both rather than introduce a third behaviour. It matters most + # for the collection keys: `includes`, `excludes` and `classes` are the + # list-valued keys an entity carries, and all three accumulate everywhere + # else they appear. + # A value the module system would have merged by equality had the key been + # declared. Functions and derivations are excluded: `==` on two functions is + # always false, so comparing them would refuse two identical definitions. + isPlainScalar = + v: + builtins.elem (builtins.typeOf v) [ + "string" + "int" + "bool" + "float" + "null" + ]; + + # Values named by type, with the value itself only where rendering it is + # safe. `builtins.toJSON` on a derivation or a function throws, and one side + # of a conflict can be either, so a message that always rendered both would + # fail while reporting a failure. + show = + v: if isPlainScalar v then "`${builtins.toJSON v}`" else "a value of type ${builtins.typeOf v}"; + deepMergeAttrs = lib.mkOptionType { name = "deepMergeAttrs"; description = "recursively merged attribute set"; check = builtins.isAttrs; - merge = _loc: defs: builtins.foldl' (acc: def: lib.recursiveUpdate acc def.value) { } defs; + merge = + _loc: defs: + let + merge2 = + a: b: + a + // builtins.mapAttrs ( + bk: bv: + if !(a ? ${bk}) then + bv + else if builtins.isAttrs a.${bk} && builtins.isAttrs bv then + merge2 a.${bk} bv + else if builtins.isList a.${bk} && builtins.isList bv then + # `bv` first: defs reach this merge in reverse declaration order, + # so `a` holds the LATER definition. Measured, not assumed, and + # it is the same ordering that makes the scalar arm below read as + # first-declaration-wins. + bv ++ a.${bk} + else if + # Either side a plain scalar means the two are not both mergeable + # shapes, so reaching here with different values is a genuine + # conflict: two scalars that differ, or a type mismatch such as a + # list against a string. Both silently resolved to one definition + # before this. + (isPlainScalar a.${bk} || isPlainScalar bv) && a.${bk} != bv + then + throw '' + den: conflicting definitions for `${bk}` on an entity. + + ${show bv} and ${show a.${bk}} were both defined, and neither is a shape the other merges with. Attribute sets merge and lists concatenate; everything else has to agree. + + Remove one definition, or give the two a key each. + '' + else + bv + ) b; + in + builtins.foldl' (acc: def: merge2 acc def.value) { } defs; }; # Single shared production run: imports + per-scope path set from ONE fx.handle. diff --git a/templates/ci/flake.lock b/templates/ci/flake.lock index a16ee16f0..4f0fe8f68 100644 --- a/templates/ci/flake.lock +++ b/templates/ci/flake.lock @@ -64,11 +64,11 @@ ] }, "locked": { - "lastModified": 1789511422, - "narHash": "sha256-Bh8GzWUbWoYRX/m56ACocEZ3l7SKPLm2YNav7FRaKJY=", + "lastModified": 1789572515, + "narHash": "sha256-Vy6fYBwOiDokeyDvVy7lSqpjILG2Y7GrzK/YPP3zJTI=", "owner": "sini", "repo": "gen", - "rev": "0b6fbd8d5d3ec37d96b739b13d957373fb89178c", + "rev": "0004f3c9dfd5634f47ee72575e56c54883450bcf", "type": "github" }, "original": { @@ -112,11 +112,11 @@ ] }, "locked": { - "lastModified": 1789489469, - "narHash": "sha256-vnmLKXRvRxuaXW6V//lJc6kRDZbt67JUPgPs9/quZVE=", + "lastModified": 1789532954, + "narHash": "sha256-HRHozzS7YaCtFK3ZQJEMSiQ265Z44YRhFYIhkM3Getg=", "owner": "sini", "repo": "gen-aspects", - "rev": "f0d9d14c356210dfe1d20f918785a317bb82a549", + "rev": "085579937ab35c8b7978e54badd67f2f26dce776", "type": "github" }, "original": { @@ -142,17 +142,21 @@ }, "gen-bind": { "inputs": { + "gen-graph": [ + "gen", + "gen-graph" + ], "gen-prelude": [ "gen", "gen-prelude" ] }, "locked": { - "lastModified": 1789489422, - "narHash": "sha256-7iFjxTPsCiNYZbyD/ZuIvNp0Kk9m1dKKrazSVaoepfY=", + "lastModified": 1789560874, + "narHash": "sha256-g7WXpedTei3uaTFXbnw4pAXyO8kJ13P29TV6U5pOSGU=", "owner": "sini", "repo": "gen-bind", - "rev": "15262e1eb3ee4cdd6b7ef075ef2ceedd0b74ca09", + "rev": "27860c870ccbe6a69a72b9cf3bc1f9ee9a4e6843", "type": "github" }, "original": { @@ -286,11 +290,11 @@ ] }, "locked": { - "lastModified": 1789489617, - "narHash": "sha256-WDzF6yJNKJQvY2OXCxN6WYpy6s64EqFQnioMAkKZsrQ=", + "lastModified": 1789521260, + "narHash": "sha256-WqcZRmzyu0F8BydkJnlFyDV5DwP7UolTYwRUbj/8u7I=", "owner": "sini", "repo": "gen-link", - "rev": "365937e4ba8d896980a0afa521f60746fedcb742", + "rev": "475f47f21db5e9e32e7a0ac42da9a961eba3f88e", "type": "github" }, "original": { @@ -424,32 +428,11 @@ ] }, "locked": { - "lastModified": 1789510821, - "narHash": "sha256-55SiolgfGyaGv2tkn6OH//K7heGiVhilF2gjXzUXIi8=", - "owner": "sini", - "repo": "gen-schema", - "rev": "f8e0e171d45e0ba872afe4a78843ec59f76165e9", - "type": "github" - }, - "original": { - "owner": "sini", - "repo": "gen-schema", - "type": "github" - } - }, - "gen-schema_2": { - "inputs": { - "nixpkgs": [ - "provider", - "nixpkgs" - ] - }, - "locked": { - "lastModified": 1779986641, - "narHash": "sha256-KcZuS+hpaloICFcepNXNLpbehh6XoPjWPBteYpTqMRw=", + "lastModified": 1789571357, + "narHash": "sha256-5VC3aTFNruM+Qwutz4879gURmHafSGxYQinkQk2BK8E=", "owner": "sini", "repo": "gen-schema", - "rev": "4bd0f6eb1799bf3c38eb3707419157b1f70eb1f5", + "rev": "48cfb9928988c7de51f67087368d92d8e587040b", "type": "github" }, "original": { @@ -478,11 +461,11 @@ ] }, "locked": { - "lastModified": 1789491559, - "narHash": "sha256-7NyGoDre+w2A2k448WRkfsdnJUtExx0DzprMEkVUVzU=", + "lastModified": 1789558867, + "narHash": "sha256-uLFCUV8aQnujhR6qegxQ745LkvuOUyN33PRnTWHRDGA=", "owner": "sini", "repo": "gen-scope", - "rev": "d24e0d983f55312b1ddada68e5a4737ec91021bb", + "rev": "41c7d9f5ead24f273b9517ca2d4744710aaa8f7f", "type": "github" }, "original": { @@ -499,11 +482,11 @@ ] }, "locked": { - "lastModified": 1789490250, - "narHash": "sha256-G3OWSbw2rBcgQWG4h6HchZxtHcKvDY+NZk50bUEE75s=", + "lastModified": 1789560881, + "narHash": "sha256-PUfsaVeOS6ZCZ47vAvEWow8A8xWHzw04Mjikh8N7KMM=", "owner": "sini", "repo": "gen-select", - "rev": "093b3d1600717c43cc00ad9d3a933e0124035e19", + "rev": "fe75c443e8e6dd55f9d94fda86013249d507671b", "type": "github" }, "original": { @@ -594,11 +577,11 @@ ] }, "locked": { - "lastModified": 1789472671, - "narHash": "sha256-CpB55naQXApJ2hMdX2kGIfEI7AEFFmIQz3pztzfnzpc=", + "lastModified": 1789555847, + "narHash": "sha256-5vKM3gmsLur1DHgAIG2jCuCNMIMzMFSWg5a2OkWJa3Q=", "owner": "sini", "repo": "gen-view", - "rev": "2656d3cc383ceb00a812b579dfed37bbd7a04c33", + "rev": "eccb0d2a787f8601616d1debc2db86beab38b133", "type": "github" }, "original": { @@ -743,7 +726,6 @@ "den": [ "den" ], - "gen-schema": "gen-schema_2", "import-tree": [ "import-tree" ], diff --git a/templates/ci/modules/features/deadbugs/instance-collection-merge.nix b/templates/ci/modules/features/deadbugs/instance-collection-merge.nix new file mode 100644 index 000000000..b8d84f529 --- /dev/null +++ b/templates/ci/modules/features/deadbugs/instance-collection-merge.nix @@ -0,0 +1,216 @@ +# Reported on #678 by theutz: two files each writing +# `den.hosts...includes` kept one list and dropped the other, +# with no conflict warning. +# +# `den.hosts`' freeform type merged with `lib.recursiveUpdate`, which treats a +# list as an opaque leaf, so one definition overwrote the other. An attrset key +# under the same two definitions merged normally, which is what made it look +# like the collection simply had no effect. +# +# Latent until #663 made instance-level collections mean something. Before +# that they were read by nothing, so there was no way to notice which list +# survived. +{ denTest, ... }: +{ + flake.tests.deadbugs.instance-collection-merge = { + + test-includes-merge-across-definitions = denTest ( + { den, igloo, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.includes = [ den.aspects.one ]; } + { den.hosts.x86_64-linux.igloo.includes = [ den.aspects.two ]; } + ]; + den.hosts.x86_64-linux.igloo.users.tux = { }; + den.aspects.one.nixos.environment.etc."one".text = "y"; + den.aspects.two.nixos.environment.etc."two".text = "y"; + + expr = { + one = igloo.environment.etc ? "one"; + two = igloo.environment.etc ? "two"; + }; + expected = { + one = true; + two = true; + }; + } + ); + + # `excludes` shares the freeform type, so it shared the defect. Two files + # each excluding one policy must suppress both, not whichever list won. + test-excludes-merge-across-definitions = denTest ( + { den, igloo, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.excludes = [ den.policies.alpha ]; } + { den.hosts.x86_64-linux.igloo.excludes = [ den.policies.beta ]; } + ]; + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.policies.alpha = _: [ + (den.lib.policy.include { nixos.environment.etc."alpha".text = "y"; }) + ]; + den.policies.beta = _: [ + (den.lib.policy.include { nixos.environment.etc."beta".text = "y"; }) + ]; + den.schema.host.includes = [ + den.policies.alpha + den.policies.beta + ]; + + expr = { + alpha = igloo.environment.etc ? "alpha"; + beta = igloo.environment.etc ? "beta"; + }; + expected = { + alpha = false; + beta = false; + }; + } + ); + + # CONTROL for the excludes cell: the two policies do fire when nothing + # excludes them, so the absences above read as suppression rather than as + # policies that never delivered. + test-control-both-policies-fire = denTest ( + { den, igloo, ... }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.policies.alpha = _: [ + (den.lib.policy.include { nixos.environment.etc."alpha".text = "y"; }) + ]; + den.policies.beta = _: [ + (den.lib.policy.include { nixos.environment.etc."beta".text = "y"; }) + ]; + den.schema.host.includes = [ + den.policies.alpha + den.policies.beta + ]; + + expr = { + alpha = igloo.environment.etc ? "alpha"; + beta = igloo.environment.etc ? "beta"; + }; + expected = { + alpha = true; + beta = true; + }; + } + ); + + # Declaration order is preserved. Concatenating in either order makes both + # entries present, so the cells above pass under a reversed merge too; + # this is what pins the direction, since include order decides which + # aspect's content wins a conflict. + test-collection-merge-keeps-declaration-order = denTest ( + { den, config, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.includes = [ "A" ]; } + { den.hosts.x86_64-linux.igloo.includes = [ "B" ]; } + ]; + + expr = config.den.hosts.x86_64-linux.igloo.includes; + expected = [ + "A" + "B" + ]; + } + ); + + # The same silent resolution, one arm over: two definitions of a key that + # is neither a mergeable attrset nor a list took the first and dropped the + # other. Pre-existing too, and weaker than the module system, which errors + # on a DECLARED option with conflicting definitions. An entity's freeform + # keys bypassed that. + test-scalar-conflict-refuses = denTest ( + { den, config, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.description = "first"; } + { den.hosts.x86_64-linux.igloo.description = "second"; } + ]; + + expr = config.den.hosts.x86_64-linux.igloo.description; + expectedError = { + type = "ThrownError"; + msg = "den: conflicting definitions for `description`"; + }; + } + ); + + # A TYPE MISMATCH is the sharper case and the one a narrower predicate + # misses: requiring both sides to be scalars leaves a list against a + # string still resolving in silence. + test-type-mismatch-refuses = denTest ( + { den, config, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.tags = [ "a" ]; } + { den.hosts.x86_64-linux.igloo.tags = "scalar"; } + ]; + + expr = config.den.hosts.x86_64-linux.igloo.tags; + expectedError = { + type = "ThrownError"; + msg = "den: conflicting definitions for `tags`"; + }; + } + ); + + # CONTROL: two definitions AGREEING is not a conflict. Without this the + # cells above pass for a predicate that refuses every repeated key, + # which would break any configuration that sets one twice harmlessly. + test-agreeing-definitions-are-not-a-conflict = denTest ( + { den, config, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.description = "same"; } + { den.hosts.x86_64-linux.igloo.description = "same"; } + ]; + + expr = config.den.hosts.x86_64-linux.igloo.description; + expected = "same"; + } + ); + + # CONTROL: a function-valued key must not refuse. `==` on two functions is + # always false, so a predicate that compared them would reject two + # identical definitions of `instantiate` or a class module. + test-function-valued-keys-do-not-refuse = denTest ( + { den, igloo, ... }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + den.aspects.igloo.nixos = + { ... }: + { + environment.etc."fn".text = "y"; + }; + + expr = igloo.environment.etc ? "fn"; + expected = true; + } + ); + + # An attrset key merged correctly throughout, and still must. This is the + # arm that localised the defect to list leaves rather than to the + # collection keys or to the registry's recursion. + test-attrset-keys-still-merge = denTest ( + { den, config, ... }: + { + imports = [ + { den.hosts.x86_64-linux.igloo.users.alice = { }; } + { den.hosts.x86_64-linux.igloo.users.bob = { }; } + ]; + + expr = builtins.attrNames config.den.hosts.x86_64-linux.igloo.users; + expected = [ + "alice" + "bob" + ]; + } + ); + + }; +} diff --git a/templates/ci/provider/flake.lock b/templates/ci/provider/flake.lock index 9e7451297..16324fcc1 100644 --- a/templates/ci/provider/flake.lock +++ b/templates/ci/provider/flake.lock @@ -15,26 +15,6 @@ "type": "github" } }, - "gen-schema": { - "inputs": { - "nixpkgs": [ - "nixpkgs" - ] - }, - "locked": { - "lastModified": 1779986641, - "narHash": "sha256-KcZuS+hpaloICFcepNXNLpbehh6XoPjWPBteYpTqMRw=", - "owner": "sini", - "repo": "gen-schema", - "rev": "4bd0f6eb1799bf3c38eb3707419157b1f70eb1f5", - "type": "github" - }, - "original": { - "owner": "sini", - "repo": "gen-schema", - "type": "github" - } - }, "import-tree": { "locked": { "lastModified": 1778781969, @@ -66,7 +46,6 @@ "root": { "inputs": { "den": "den", - "gen-schema": "gen-schema", "import-tree": "import-tree", "nixpkgs": "nixpkgs" } diff --git a/templates/ci/provider/flake.nix b/templates/ci/provider/flake.nix index 5c682e574..2380d7309 100644 --- a/templates/ci/provider/flake.nix +++ b/templates/ci/provider/flake.nix @@ -10,7 +10,5 @@ nixpkgs.url = "https://channels.nixos.org/nixos-unstable/nixexprs.tar.xz"; import-tree.url = "github:vic/import-tree"; den.url = "github:denful/den"; - gen-schema.url = "github:sini/gen-schema"; - gen-schema.inputs.nixpkgs.follows = "nixpkgs"; }; }