From 6d7cd23a546767e1d726a28a7198ccaa5f851650 Mon Sep 17 00:00:00 2001 From: Jason Bowman Date: Fri, 18 Sep 2026 18:04:25 -0700 Subject: [PATCH] fix: keep a provides forward marked across a re-merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `__providesForwarded` names the provides children a merge forwarded onto an aspect's own top level, so `classifyKeys` skips them rather than reading them as the aspect's own class or nested content. All three construction sites derived it from `!(own ? k)`, which cannot tell a name the author wrote at top level from a name an earlier merge forwarded there. Re-merging an already-merged aspect — an alias, a nested-key alias, an `` include — therefore read its own earlier forward as a direct definition and cleared the marker, and the child was classified. Where the child's name is a registered class, the child aspect itself was emitted as that class's content. Routes the shadow test through `mkUnderscore`, which already holds both the aspect and the set to filter, and reads the marker the earlier merge published instead of guessing from key presence. --- nix/lib/aspects/types.nix | 30 +++++-- .../issue-683-remerge-provides-forwarded.nix | 87 +++++++++++++++++++ 2 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 templates/ci/modules/features/deadbugs/issue-683-remerge-provides-forwarded.nix diff --git a/nix/lib/aspects/types.nix b/nix/lib/aspects/types.nix index 26b39b33..a2d88885 100644 --- a/nix/lib/aspects/types.nix +++ b/nix/lib/aspects/types.nix @@ -155,6 +155,15 @@ let # `den.reservedKeys` — both reachable through `._` exactly as the # declared options are. forwardable = lib.filterAttrs (k: _: !(isStructuralKey k)) providesChildren; + # A name held both as a provides child and as a direct key keeps its + # direct value and stays classified — it is the aspect's own content, and + # masking it would emit neither. `own ? k` alone cannot decide that: a + # re-merge (an alias, a nested-key alias, an `` include) + # sees the FORWARD an earlier merge wrote onto the top level and reads it + # as a direct definition, clearing the marker and classifying the child + # (#683). The earlier merge named its own forwards, so ask it. + priorForwards = lib.genAttrs (own.__providesForwarded or [ ]) (_: true); + isShadowed = k: (own ? ${k}) && !(priorForwards ? ${k}); childKeys = builtins.filter isChildKey (builtins.attrNames own); functor = { __functor = _self: _args: { @@ -164,7 +173,12 @@ let }; in { - inherit providesChildren forwardable functor; + inherit + providesChildren + forwardable + functor + isShadowed + ; syntheticProvides = providesChildren // functor; }; @@ -234,8 +248,8 @@ let # usable in an includes list: wrapChild sees a zero-arg functor and calls # it, so the aspect below is built only when _ is actually included. underscore = mkUnderscore merged ((typeCfg.chain or typeCfg.origin) ++ [ aspectName ]); - inherit (underscore) providesChildren; - unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren); + inherit (underscore) providesChildren isShadowed; + unshadowedProvides = builtins.filter (k: !(isShadowed k)) (builtins.attrNames providesChildren); in # __functor makes merged aspects callable (aspect { host = ...; }). # Explicit functors (e.g. den.batteries.forward) take priority. @@ -328,8 +342,8 @@ let normalizedFn = foldUnderscoreIntoProvides fn; aspectName = fn.name or (lib.last loc); underscore = mkUnderscore normalizedFn ((typeCfg.chain or typeCfg.origin) ++ [ aspectName ]); - inherit (underscore) forwardable; - unshadowedProvides = builtins.filter (k: !(normalizedFn ? ${k})) (builtins.attrNames forwardable); + inherit (underscore) forwardable isShadowed; + unshadowedProvides = builtins.filter (k: !(isShadowed k)) (builtins.attrNames forwardable); in forwardable // normalizedFn @@ -728,10 +742,8 @@ let # single-def path is unaffected because a raw attrset carries none of # these keys. topUnderscore = mkUnderscore annotatedMerged provider; - inherit (topUnderscore) forwardable; - unshadowedProvides = builtins.filter (k: !(annotatedMerged ? ${k})) ( - builtins.attrNames forwardable - ); + inherit (topUnderscore) forwardable isShadowed; + unshadowedProvides = builtins.filter (k: !(isShadowed k)) (builtins.attrNames forwardable); in forwardable // annotatedMerged diff --git a/templates/ci/modules/features/deadbugs/issue-683-remerge-provides-forwarded.nix b/templates/ci/modules/features/deadbugs/issue-683-remerge-provides-forwarded.nix new file mode 100644 index 00000000..af3a5b6b --- /dev/null +++ b/templates/ci/modules/features/deadbugs/issue-683-remerge-provides-forwarded.nix @@ -0,0 +1,87 @@ +# `__providesForwarded` marks the names a merge forwarded from `provides` onto +# an aspect's own top level, so `classifyKeys` skips them instead of reading +# them as the aspect's own class or nested content. All three construction +# sites in nix/lib/aspects/types.nix compute that marker with the same shadow +# test — `!(own ? k)` — which cannot tell a name the AUTHOR wrote at top level +# from a name a PREVIOUS merge forwarded there. So re-merging an already-merged +# aspect (an alias, a nested-key alias, an `` include) read its +# own earlier forward as a direct definition, cleared the marker, and let the +# child be classified. Where the child's name is a registered class, the child +# aspect itself was emitted as that class's content (#683: a `provides.packages` +# child surfaced as `flake.packages.`, fields and all). +{ denTest, inputs, ... }: +{ + flake.tests.deadbugs.issue-683-remerge-provides-forwarded = { + + # `src` is the live control: it is merged once and classifies correctly. + # The two aliases are the same value merged a second time, at the two + # sites a re-merge is directly readable from. + test-marker-survives-a-remerge = denTest ( + { den, ... }: + let + cls = den.lib.aspects.fx.keyClassification.classifyKeys null; + classified = a: (cls a).classKeys; + in + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.d683src.provides.child.provides.nixos.nixos.environment.etc."d683".text = "y"; + # site A — mergeWithAspectMeta: an already-merged aspect aliased to a root slot. + den.aspects.d683a = den.aspects.d683src.provides.child; + # site C — aspectContentType: the same value at a nested freeform key. + den.aspects.d683c.holder = den.aspects.d683src.provides.child; + + expr = { + srcMarker = den.aspects.d683src.provides.child.__providesForwarded or "MISSING"; + aMarker = den.aspects.d683a.__providesForwarded or "MISSING"; + cMarker = den.aspects.d683c.holder.__providesForwarded or "MISSING"; + srcClassified = classified den.aspects.d683src.provides.child; + aClassified = classified den.aspects.d683a; + cClassified = classified den.aspects.d683c.holder; + }; + expected = { + srcMarker = [ "nixos" ]; + aMarker = [ "nixos" ]; + cMarker = [ "nixos" ]; + srcClassified = [ ]; + aClassified = [ ]; + cClassified = [ ]; + }; + } + ); + + # Site B — mergeFunctions' functor-carrying attrset branch, reached by + # including an already-merged aspect. End-to-end rather than by marker, + # because the reported symptom is the child aspect's own fields arriving + # as flake output content. + test-provides-packages-does-not-reach-flake-packages = denTest ( + { + config, + den, + __findFile, + ... + }: + { + imports = [ inputs.den.flakeOutputs.packages ]; + _module.args.__findFile = den.lib.__findFile; + + den.hosts.x86_64-linux.igloo.users.tux = { }; + + den.aspects.igloo.includes = [ ]; + den.aspects.d683group.provides.tools = { + includes = [ ]; + provides.packages = { + homeManager = + { pkgs, ... }: + { + home.packages = [ ]; + }; + }; + }; + + expr = builtins.attrNames (config.flake.packages.x86_64-linux or { }); + expected = [ ]; + } + ); + }; +}