fix: keep a provides forward marked across a re-merge - #684
Merged
Merged
Conversation
`__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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #683.
Summary
__providesForwardedintact when an already-merged aspect is merged a second time, so aprovideschild 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 aprovides.packageschild reached through an<angle/bracket>include arrived asflake.packages.<system>carrying fourteen of the child aspect's own fields —name,includes,meta,provides,_,__functor,homeManager,darwinamong them — which a derivation-typed package schema then fails on.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.mergeFunctions, site B) but not for the defect:d50f0fcalready carried the identical test atmergeWithAspectMetaandaspectContentType(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.packagesregistered as a class. The leak is not name-specific —provides.nixosreproduces it on stock den with no flake outputs imported — so un-reservingpackageswould address one spelling and leavenixos,darwin,homeManager,apps,checks,devShells,legacyPackages,os,user,hjem,maidandwslunchanged, while breakingpackages-merge.nix, the How to merge multiple packages attrsets from different aspects instead of letting the last include win ? #572 regression assertingden.aspects.<name>.packagesreachesflake.packages.<system>.Behaviour changes
nixos/darwin/homeManagercontent delivered twice, stops receiving them. That content was never the class content the key named.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 thepass + failidentity is not standing in for a truncated run.types.nixreverted: 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.aMarker/cMarkerread[ ]against[ "nixos" ]andaClassified/cClassifiedread[ "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.originalForwarded = [ "packages" ],remergedForwarded = [ ],remergedClasses.classKeys = [ "packages" ], and thirteen names inflake.packages.x86_64-linux.provides.nixosloses the marker and gainsclassKeys = [ "nixos" ]on re-merge with no flake output module imported at all, and aprovides.plainchildchild is reclassified as a nested key in the same run.appsclassifies its own content normally (classKeys = [ "nixos" ]), a nested key namedappsis read as class content (classKeys = [ "apps" ]) and is correctly reserved, and aprovideschild namedappsis exempt by marker (classKeys = [ ]). Only the third is this bug.nix develop -c just fmt: 577 files processed, 0 changed.