Skip to content

fix: keep a provides forward marked across a re-merge - #684

Merged
sini merged 1 commit into
mainfrom
fix/683-remerge-provides-forwarded
Sep 19, 2026
Merged

sini merged 1 commit into
mainfrom
fix/683-remerge-provides-forwarded

Conversation

@sini

@sini sini commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Closes #683.

Summary

  • Keeps __providesForwarded intact when an already-merged aspect is merged a second time, so a provides child whose name matches a registered class stays a child instead of being reclassified as the aspect's own class content and emitted into that class. In the reporting configuration a provides.packages child reached through an <angle/bracket> include arrived as flake.packages.<system> carrying fourteen of the child aspect's own fields — name, includes, meta, provides, _, __functor, homeManager, darwin among them — which a derivation-typed package schema then fails on.
  • Routes the shadow test through mkUnderscore, which already holds both the aspect and the candidate set, rather than leaving a copy at each of the three construction sites. All three derived it from !(own ? k), which cannot tell a name the author wrote at top level from a name an earlier merge forwarded there; the earlier merge published exactly that distinction, so the predicate reads the marker instead of guessing from key presence.
  • Fixes all three sites, not the reported one. BUG: provides.packages leaks aspect fields into flake packages after #678 #683 bisects to fix: resolve identity by definition value and split the provenance field #678, which is correct for the path it hits (mergeFunctions, site B) but not for the defect: d50f0fc already carried the identical test at mergeWithAspectMeta and aspectContentType (lines 132 and 579), so aliasing a merged aspect to a root slot or to a nested freeform key lost the marker before fix: resolve identity by definition value and split the provenance field #678. fix: resolve identity by definition value and split the provenance field #678 extended the flaw to the include path.
  • Leaves packages registered as a class. The leak is not name-specific — provides.nixos reproduces it on stock den with no flake outputs imported — so un-reserving packages would address one spelling and leave nixos, darwin, homeManager, apps, checks, devShells, legacyPackages, os, user, hjem, maid and wsl unchanged, while breaking packages-merge.nix, the How to merge multiple packages attrsets from different aspects instead of letting the last include win ? #572 regression asserting den.aspects.<name>.packages reaches flake.packages.<system>.
  • Adds two cells: the marker and its classification at the two re-merge sites readable by marker, with the once-merged source as a live control in the same run; and the reporter's shape end to end, since the symptom is the child's own fields arriving as flake output content rather than a marker value.

Behaviour changes

  • A provides child sharing a name with a registered class is no longer emitted as that class's content when its aspect is re-merged. A configuration that was receiving the child aspect's fields in a flake output, or its nixos/darwin/homeManager content delivered twice, stops receiving them. That content was never the class content the key named.
  • A provides child with an ordinary name is no longer reclassified as a nested key on re-merge. Nested keys are not auto-walked, so this was silent.
  • A name the author genuinely writes at top level alongside a same-named provides child keeps its direct value and stays classified, unchanged. That case is what the shadow test exists for and the predicate still answers it.

Validation

  • nix develop -c just ci: 1238/1238, exit 0, zero ❌ and zero ☢️, counted off a capture with stderr merged. The summary numerator agrees with the counted passes, so the pass + fail identity is not standing in for a truncated run.
  • Ran the same suite on the tree with only types.nix reverted: 1238 collected, 1236 passing, exit 1, and the two failures are exactly the two new cells. Same population both runs, so nothing else moved in either direction. 1236 is also the count fix: keep unnameable context values out of scope identity #682 landed at, which is the independent check that the population is right.
  • Falsified both new cells against the unfixed tree on their actual symptom rather than on a mismatch: aMarker/cMarker read [ ] against [ "nixos" ] and aClassified/cClassified read [ "nixos" ] against [ ], while the once-merged control in the same cell stays green — so the cell measures the re-merge rather than a marker that vanished everywhere.
  • Reproduced BUG: provides.packages leaks aspect fields into flake packages after #678 #683 as filed before touching anything, against the local checkout rather than the pinned revision: originalForwarded = [ "packages" ], remergedForwarded = [ ], remergedClasses.classKeys = [ "packages" ], and thirteen names in flake.packages.x86_64-linux.
  • Measured the name-independence claim rather than reasoning to it: provides.nixos loses the marker and gains classKeys = [ "nixos" ] on re-merge with no flake output module imported at all, and a provides.plainchild child is reclassified as a nested key in the same run.
  • Measured the three key positions separately, because they do not share a verdict: a root aspect named apps classifies its own content normally (classKeys = [ "nixos" ]), a nested key named apps is read as class content (classKeys = [ "apps" ]) and is correctly reserved, and a provides child named apps is exempt by marker (classKeys = [ ]). Only the third is this bug.
  • nix develop -c just fmt: 577 files processed, 0 changed.

`__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 `<angle/bracket>` 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.
@sini
sini requested a review from vic as a code owner September 19, 2026 01:05
@github-actions github-actions Bot added the allow-ci allow all CI integration tests label Sep 19, 2026
@sini
sini merged commit 90c303b into main Sep 19, 2026
17 of 24 checks passed
@sini
sini deleted the fix/683-remerge-provides-forwarded branch September 19, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-ci allow all CI integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: provides.packages leaks aspect fields into flake packages after #678

1 participant