Skip to content

Rebuild stale module-internal dependents when a dependency is re-added#5944

Draft
danielbotros wants to merge 7 commits intomainfrom
APP-16062-fix-module-internal-dep-chain
Draft

Rebuild stale module-internal dependents when a dependency is re-added#5944
danielbotros wants to merge 7 commits intomainfrom
APP-16062-fix-module-internal-dep-chain

Conversation

@danielbotros
Copy link
Copy Markdown
Member

No description provided.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Apr 16, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 16, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 16, 2026
@dgottlieb dgottlieb added the static-ignore-tests Build static binaries from PR and ignore tests label Apr 19, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 22, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 22, 2026
Comment thread module/resources.go
Comment on lines +435 to +449
// Record the new dependent pointer in our entry so internalDeps stays in sync
// with the collection. This keeps `toReconfig` accurate for any downstream code
// that reads it (e.g. removeResource's filter loop matches entries by pointer
// identity — if we left toReconfig at the pre-rebuild pointer, a subsequent
// remove of the newly-rebuilt dependent wouldn't clean up this entry).
if newDependentRes != nil {
dependentName := args.conf.ResourceName()
entries := m.internalDeps[newResName]
for j := range entries {
if entries[j].conf.ResourceName() == dependentName {
entries[j].toReconfig = newDependentRes
break
}
}
}
Copy link
Copy Markdown
Member Author

@danielbotros danielbotros Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The state management of the internalDeps here and below in removeResource is a overly complicated imo and can be simplified by turning it into a forward index but keeping this PR scoped.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 22, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone static-ignore-tests Build static binaries from PR and ignore tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants