Skip to content

fix: three silent drops in aspect-key handling (._ depth, provides mask, quirk multi-def) - #650

Merged
sini merged 4 commits into
mainfrom
fix/nested-globbing
Aug 7, 2026
Merged

fix: three silent drops in aspect-key handling (._ depth, provides mask, quirk multi-def)#650
sini merged 4 commits into
mainfrom
fix/nested-globbing

Conversation

@sini

@sini sini commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Three defects that shared a shape: content was declared, accepted, and then never reached the output — with no error, no warning, and no diagnostic.

._ stopped synthesizing below the second nesting level

den.aspects.a.b._ resolved; den.aspects.a.b.c._ threw attribute '_' missing. aspectContentType synthesized _ only on the wrapper for its own key, and the two annotators below it had already diverged — the single-def path walked one level, the multi-def path recursed, and neither carried _.

Collapsed into one recursive annotateChildren: every node that gets provenance also gets its own ._, at any depth. Include lists now map over the annotated children rather than the raw ones, which is what the multi-def path's own comment required — raw children get anon-renamed per inclusion path and double-emit class content.

provides/_ remain structural keys, so ._ still never collects provides children at any level.

Four tests in include-children: depth 3, depth 4, depth-3 class-key exclusion, depth-3 multi-def.

A provides child masked a same-named class key

An aspect declaring provides.user alongside user-class content emitted no user class at all:

den.aspects.svc = {
  provides.user.nixos.services.openssh.enable = true;
  user.description = "pinguino";   # silently dropped
};

__providesForwarded listed every provides child so classification would skip the forwarded copies, but the forwarded value only reaches the top level when the aspect does not define that name itself — the merge is providesChildren // merged, so a direct key wins. Masking it anyway left the surviving value classified as neither class nor nested key.

Now reports only children the aspect does not define directly, at both the root-aspect and nested-wrapper sites. host-to-users reads the same marker to decide entity-named sub-aspect fan-out, so it now matches its own stated rule ("not from provides").

Quirk keys defined twice on one aspect collapsed to one empty value

An aspect defined across several files, each contributing the same quirk key, reached consumers as a single entry carrying none of the emitted data — one attrset whose only attribute was imports.

unwrapContentValuesList folds multiple definitions into { imports = vals; }. Right for a class key, where the module system merges them back into one value; wrong for a quirk key, where nothing merges and the wrapper is the value the consumer reads.

Split the two readings: unwrapContentValuesAll returns one value per definition, used by pipe emission in both emit-classes and the deferred-include drain in resolve.nix. Class emission keeps the collapsing form. Assembly needed no change — it already concatenates entries per scope and per pipe.

One test in pipes, stating the designed behavior (quirks accumulate). It sorts before comparing, since merge order is not a contract.

Verification

  • nix develop -c just ci → 1084/1084, exit 0
  • nix develop -c just fmt → clean
  • just check minimal, just check default, just check example (each --override-input den .) → all checks passed

sini added 3 commits August 6, 2026 10:36
`den.aspects.a.b._` resolved, `den.aspects.a.b.c._` threw `attribute '_'
missing`. aspectContentType synthesized `_` only on the wrapper for its own
key; children below it were annotated with `__provider` and nothing else.

The two annotators had already diverged: the single-def path walked one level
and the multi-def path recursed, and neither carried `_`. Collapse them into
one recursive `annotateChildren` so every node that gets provenance also gets
its own `._`, at any depth. Include lists now map over the annotated children
rather than the raw ones, which is what the multi-def path's own comment
required — raw children get anon-renamed per inclusion path and double-emit
class content.

`provides`/`_` stay structural keys, so `._` still never collects provides
children at any level.
An aspect declaring `provides.user` alongside `user`-class content emitted no
user class at all — silently, with no error and no diagnostic.

`__providesForwarded` listed every provides child so classification would skip
the forwarded copies, but the forwarded value only reaches the top level when
the aspect does not define that name itself: the merge is `providesChildren //
merged`, so a direct key wins. Masking it anyway left the surviving value
classified as neither class nor nested key, and it fell out of the pipeline.

Report the forwarded names only for children the aspect does not define
directly, at both the root-aspect and nested-wrapper sites. `host-to-users`
reads the same marker to decide entity-named sub-aspect fan-out, so it now
matches its own stated rule too ("not from provides").
…y value

An aspect defined across several files, each contributing the same quirk key,
reached consumers as a single entry carrying none of the emitted data — the
consumer's pipe list held one attrset whose only attribute was `imports`.

`unwrapContentValuesList` folds multiple definitions into `{ imports = vals; }`.
That is right for a class key, where the module system merges the imports back
into one value, and wrong for a quirk key, where nothing merges: the wrapper
IS the value the consumer reads, and the definitions inside it are never seen.

Split the two readings. `unwrapContentValuesAll` returns one value per
definition; pipe emission uses it, class emission keeps the collapsing form.
Both the emit-classes pipe path and the deferred-include drain in resolve.nix
now emit one entry per definition, which is what assembly already expected —
it concatenates entries per scope and per pipe.
@sini
sini requested a review from vic as a code owner August 6, 2026 18:21
@sini
sini merged commit 6cc4153 into main Aug 7, 2026
15 checks passed
@sini
sini deleted the fix/nested-globbing branch August 7, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants