diff --git a/nix/lib/aspects/fx/content-util.nix b/nix/lib/aspects/fx/content-util.nix index 1e07ff3a..980ed62f 100644 --- a/nix/lib/aspects/fx/content-util.nix +++ b/nix/lib/aspects/fx/content-util.nix @@ -1,17 +1,24 @@ { lib, ... }: let + # Definition values of a content wrapper, empty definitions dropped. + defValues = + rawValue: + builtins.filter (v: !(builtins.isAttrs v && v == { })) (map (d: d.value) rawValue.__contentValues); + # Unwrap to a list of values, with empty-set fallback to [{}]. # Used by aspect emitClassModules which needs per-element processing. # Matches source order: list check first, then __contentValues, then singleton. + # + # Several definitions of one CLASS key collapse into a single module: the + # module system merges them through `imports`, so the class sees one value. + # Quirk data has no such merge — see unwrapContentValuesAll. unwrapContentValuesList = rawValue: if builtins.isList rawValue then rawValue else if builtins.isAttrs rawValue && rawValue ? __contentValues then let - vals = builtins.filter (v: !(builtins.isAttrs v && v == { })) ( - map (d: d.value) rawValue.__contentValues - ); + vals = defValues rawValue; in if builtins.length vals == 0 then [ { } ] @@ -22,6 +29,22 @@ let else [ rawValue ]; + # Every definition, uncollapsed. Quirks accumulate rather than merge, so a + # pipe key defined by several modules must reach assembly as one entry per + # definition. Collapsing them into `{ imports = …; }` would hand consumers a + # single value carrying none of the emitted data. + unwrapContentValuesAll = + rawValue: + if builtins.isList rawValue then + rawValue + else if builtins.isAttrs rawValue && rawValue ? __contentValues then + let + vals = defValues rawValue; + in + if vals == [ ] then [ { } ] else vals + else + [ rawValue ]; + # Unwrap for key-classification inspection: merges attrset values # for sub-key detection, returns null for non-attrsets. unwrapContentValuesForClassification = @@ -59,6 +82,7 @@ in { inherit unwrapContentValuesList + unwrapContentValuesAll unwrapContentValuesForClassification applyProvide ; diff --git a/nix/lib/aspects/fx/handlers/emit-classes.nix b/nix/lib/aspects/fx/handlers/emit-classes.nix index d6dca19f..664be841 100644 --- a/nix/lib/aspects/fx/handlers/emit-classes.nix +++ b/nix/lib/aspects/fx/handlers/emit-classes.nix @@ -8,7 +8,7 @@ let inherit (den.lib) fx; inherit (den.lib.aspects.fx) identity; - inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList; + inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList unwrapContentValuesAll; inherit (den.lib.schemaUtil) schemaEntityKindsSet; inherit (den.lib.aspects.fx.aspect) ctxFromHandlers; @@ -105,10 +105,12 @@ let in fx.seq (lib.imap0 mkEntry modules); + # One entry per definition: quirk values accumulate at assembly, so several + # definitions of the same pipe key on one aspect must stay separate. emitPipeKey = aspect: ctx: contextDep: nodeIdentity: k: let - modules = unwrapContentValuesList aspect.${k}; + modules = unwrapContentValuesAll aspect.${k}; isMulti = builtins.length modules > 1; mkEntry = idx: module: diff --git a/nix/lib/aspects/fx/resolve.nix b/nix/lib/aspects/fx/resolve.nix index 0421a6a6..dfe66dee 100644 --- a/nix/lib/aspects/fx/resolve.nix +++ b/nix/lib/aspects/fx/resolve.nix @@ -694,7 +694,7 @@ let let allDeferred = (result.state.scopedDeferredIncludes or (_: { })) null; inherit (den.lib.aspects.fx.keyClassification) classifyKeys; - inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList; + inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList unwrapContentValuesAll; # Build enriched context for a scope by inheriting parent enrichment. # Walks up scopeParent to find enrichment keys not present in the # scope's own context. @@ -744,8 +744,10 @@ let lib.concatMap ( k: let - modules = unwrapContentValuesList child.${k}; isPipe = den.quirks ? ${k}; + # Pipe keys keep one entry per definition (quirks accumulate); + # class keys collapse into a single module (the module system merges). + modules = if isPipe then unwrapContentValuesAll child.${k} else unwrapContentValuesList child.${k}; in map ( module: diff --git a/nix/lib/aspects/types.nix b/nix/lib/aspects/types.nix index d2dd86d0..fd8f15e5 100644 --- a/nix/lib/aspects/types.nix +++ b/nix/lib/aspects/types.nix @@ -96,8 +96,14 @@ let # Forward provides children onto the merged aspect so # aspect.docker resolves to aspect.provides.docker. # provides-first so direct freeform keys on merged take priority. - # __providesForwarded tells the pipeline to skip these during classification. + # __providesForwarded tells the pipeline to skip these during + # classification — but only for names the aspect does not define + # itself. A name held by both loses the forwarded value to the direct + # key above, so masking it would classify neither: an aspect declaring + # provides.user alongside user-class content would silently emit no + # user class at all. providesChildren = builtins.removeAttrs (merged.provides or { }) [ "_module" ]; + unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren); # Child aspect keys for synthetic provides: freeform keys that are # not structural, internal, class, pipe, or forwarded-from-provides. classReg = den.classes or { }; @@ -132,7 +138,7 @@ let // merged // { __functor = if originalFunctor != null then originalFunctor else resolveAspectWith; - __providesForwarded = builtins.attrNames providesChildren; + __providesForwarded = unshadowedProvides; provides = syntheticProvides; _ = syntheticProvides; }; @@ -489,45 +495,17 @@ let bv ) b; subForwarded = builtins.foldl' deepMerge { } subAttrVals; - # Multi-def counterpart of annotatedMerged: tag forwarded - # children recursively with __provider, else navigation - # through a multi-def key yields raw children that get - # anon-renamed per inclusion path and double-emit class - # content. Recursive (unlike annotatedMerged) because - # subForwarded never re-enters aspectContentType per - # level. Name-based guards come first — forcing a - # registered class value mid-merge can re-enter the flake - # fixpoint (#580; see isNestedKey); only unregistered - # namespace keys (forced by navigation anyway) get WHNF'd. - annotateDeep = - provPath: attrs: - lib.mapAttrs ( - ck: cv: - let - childPath = provPath ++ [ ck ]; - in - if - !(lib.hasPrefix "__" ck) - && !(classReg ? ${ck}) - && !(pipeReg ? ${ck}) - && !(structuralKeysSet ? ${ck}) - && builtins.isAttrs cv - && !(cv ? __provider) - && !(cv ? __contentValues) - then - annotateDeep childPath cv // { __provider = childPath; } - else - cv - ) attrs; provBase = (typeCfg.providerPrefix or [ ]) ++ [ keyName k ]; + annotatedSub = annotateChildren provBase subForwarded; in - annotateDeep provBase subForwarded + annotatedSub // { __contentValues = defsForKey; __provider = provBase; + _ = underscoreAt provBase annotatedSub; } ); # Single-function content wrappers need __functor so the wrapper is @@ -542,46 +520,61 @@ let inherit (den.lib.aspects.fx.keyClassification) structuralKeysSet; # Forward provides children onto the wrapper so # aspect.child.monitoring resolves to aspect.child.provides.monitoring, - # matching mergeWithAspectMeta behavior for root aspects. + # matching mergeWithAspectMeta behavior for root aspects — including + # the rule that a name the wrapper defines itself keeps its own value + # and stays classified. providesChildren = builtins.removeAttrs (merged.provides or { }) [ "_module" ]; + unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren); provider = (typeCfg.providerPrefix or [ ]) ++ [ keyName ]; - childKeys = builtins.filter ( - k: !(structuralKeysSet ? ${k}) && !(lib.hasPrefix "__" k) && !(classReg ? ${k}) && !(pipeReg ? ${k}) - ) (builtins.attrNames merged); - aspectName = lib.concatStringsSep "." provider; - syntheticAspect = { - name = "${aspectName}._"; - includes = map (k: merged.${k}) childKeys; + # A key names a candidate child aspect when it is neither structural, + # internal, class nor pipe. Provides children are reached through + # `provides`/`_`, which are structural — ._ never collects them. + isChildKey = + k: + !(structuralKeysSet ? ${k}) && !(lib.hasPrefix "__" k) && !(classReg ? ${k}) && !(pipeReg ? ${k}); + # The synthetic aspect behind ._ at a given tree position. + underscoreAt = provPath: attrs: { + __functor = _self: _args: { + name = "${lib.concatStringsSep "." provPath}._"; + includes = map (k: attrs.${k}) (builtins.filter isChildKey (builtins.attrNames attrs)); + }; }; # Annotate nested attrset children with __provider so deeply nested - # aspects carry provenance for hasAspect resolution. - # Only annotate unregistered keys (potential nested aspects) — - # skip class keys, pipe keys, structural keys, and internal keys. - annotatedMerged = lib.mapAttrs ( - k: v: - if - builtins.isAttrs v - && !(v ? __provider) - && !(v ? __contentValues) - && !(lib.hasPrefix "__" k) - && !(classReg ? ${k}) - && !(pipeReg ? ${k}) - && !(structuralKeysSet ? ${k}) - then - v // { __provider = provider ++ [ k ]; } - else - v - ) merged; + # aspects carry provenance for hasAspect resolution, and give each + # one its own ._ so the shorthand holds at every depth rather than + # only at this wrapper. Without the recursion, navigation through a + # nested key also yields raw children that get anon-renamed per + # inclusion path and double-emit class content. + # Name-based guards come first — forcing a registered class value + # mid-merge can re-enter the flake fixpoint (#580; see isNestedKey); + # only unregistered namespace keys (forced by navigation anyway) get + # WHNF'd. + annotateChildren = + provPath: attrs: + lib.mapAttrs ( + k: v: + let + childPath = provPath ++ [ k ]; + sub = annotateChildren childPath v; + in + if isChildKey k && builtins.isAttrs v && !(v ? __provider) && !(v ? __contentValues) then + sub + // { + __provider = childPath; + _ = underscoreAt childPath sub; + } + else + v + ) attrs; + annotatedMerged = annotateChildren provider merged; in providesChildren // annotatedMerged // { __contentValues = flatDefs; __provider = provider; - __providesForwarded = builtins.attrNames providesChildren; - _ = { - __functor = _self: _args: syntheticAspect; - }; + __providesForwarded = unshadowedProvides; + _ = underscoreAt provider annotatedMerged; } // lib.optionalAttrs singleFn { __functor = _self: (builtins.head flatDefs).value; diff --git a/templates/ci/modules/public-api/include-children.nix b/templates/ci/modules/public-api/include-children.nix index 30b97a35..52d35610 100644 --- a/templates/ci/modules/public-api/include-children.nix +++ b/templates/ci/modules/public-api/include-children.nix @@ -348,6 +348,114 @@ } ); + # ._ at depth 3 — synthesis must not stop at the first nesting level + test-include-children-depth-three = denTest ( + { + den, + igloo, + ... + }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.a.b.c.x.nixos.services.openssh.enable = true; + den.aspects.a.b.c.y.nixos.networking.nameservers = [ "1.1.1.1" ]; + + den.aspects.igloo.includes = [ den.aspects.a.b.c._ ]; + + expr = { + ssh = igloo.services.openssh.enable; + dns = igloo.networking.nameservers; + }; + expected = { + ssh = true; + dns = [ "1.1.1.1" ]; + }; + } + ); + + # ._ at depth 4 — synthesis is depth-unbounded + test-include-children-depth-four = denTest ( + { + den, + igloo, + ... + }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.a.b.c.d.x.nixos.services.openssh.enable = true; + den.aspects.a.b.c.d.y.nixos.networking.nameservers = [ "1.1.1.1" ]; + + den.aspects.igloo.includes = [ den.aspects.a.b.c.d._ ]; + + expr = { + ssh = igloo.services.openssh.enable; + dns = igloo.networking.nameservers; + }; + expected = { + ssh = true; + dns = [ "1.1.1.1" ]; + }; + } + ); + + # Depth-3 ._ excludes class keys, same as the shallower levels + test-include-children-depth-three-skips-class-keys = denTest ( + { + den, + igloo, + ... + }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.a.b.c = { + nixos.networking.hostName = "should-not-leak"; + child.nixos.services.openssh.enable = true; + }; + + den.aspects.igloo.includes = [ den.aspects.a.b.c._ ]; + + expr = { + ssh = igloo.services.openssh.enable; + hostName = igloo.networking.hostName; + }; + expected = { + ssh = true; + hostName = "nixos"; + }; + } + ); + + # Depth-3 ._ collects children contributed by several definitions + test-include-children-depth-three-multi-def = denTest ( + { + den, + igloo, + ... + }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.a.b.c.x.nixos.services.openssh.enable = true; + den.aspects.a.b = { + c.y.nixos.networking.nameservers = [ "1.1.1.1" ]; + }; + + den.aspects.igloo.includes = [ den.aspects.a.b.c._ ]; + + expr = { + ssh = igloo.services.openssh.enable; + dns = igloo.networking.nameservers; + }; + expected = { + ssh = true; + dns = [ "1.1.1.1" ]; + }; + } + ); + # Nested ._ with parametric child aspects test-include-children-nested-parametric = denTest ( { diff --git a/templates/ci/modules/public-api/pipes.nix b/templates/ci/modules/public-api/pipes.nix index bf044977..dad4fca1 100644 --- a/templates/ci/modules/public-api/pipes.nix +++ b/templates/ci/modules/public-api/pipes.nix @@ -265,5 +265,45 @@ expected = "resolved"; } ); + + # Quirks accumulate: several definitions of one aspect (i.e. the same + # aspect name across several files) each contribute an entry, rather than + # collapsing into a single value the way class content does. + test-pipe-multi-def-same-aspect = denTest ( + { den, igloo, ... }: + { + imports = [ + (_: { + den.aspects.svc.firewall.ports = [ 80 ]; + }) + (_: { + den.aspects.svc.firewall.ports = [ 5432 ]; + }) + ]; + + den.hosts.x86_64-linux.igloo.users.tux = { }; + den.quirks.firewall = { + description = "Firewall port declarations"; + }; + + den.aspects.igloo.includes = [ + den.aspects.svc + den.aspects.consumer + ]; + + den.aspects.consumer.nixos = + { firewall, ... }: + { + networking.firewall.allowedTCPPorts = lib.concatMap (f: f.ports or [ ]) firewall; + }; + + # Merge order is not a contract — compare as a sorted set. + expr = lib.sort (a: b: a < b) igloo.networking.firewall.allowedTCPPorts; + expected = [ + 80 + 5432 + ]; + } + ); }; }