Skip to content

Fix stale resource_set usage#29282

Open
keith wants to merge 1 commit intobazelbuild:masterfrom
keith:ks/fix-stale-resource_set-usage
Open

Fix stale resource_set usage#29282
keith wants to merge 1 commit intobazelbuild:masterfrom
keith:ks/fix-stale-resource_set-usage

Conversation

@keith
Copy link
Copy Markdown
Member

@keith keith commented Apr 13, 2026

Previously this function was interned using the location, not including
the contents. This meant if you were iterating on the contents while
developing a ruleset, the values required a bazel server restart to be
reflected.

Previously this function was interned using the location, not including
the contents. This meant if you were iterating on the contents while
developing a ruleset, the values required a bazel server restart to be
reflected.
@keith keith requested a review from a team as a code owner April 13, 2026 22:15
@keith keith requested review from dabanki and removed request for a team April 13, 2026 22:15
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols awaiting-review PR is awaiting review from an assigned reviewer labels Apr 13, 2026
}
return Objects.equal(fn, that.fn)
// Use identity comparison for fn to avoid interning stale callbacks across builds.
// StarlarkFunction.equals() is based on GlobalSymbol identity (label + name), which
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's a pretty bad gotcha. I wouldn't be surprised if this caused issues elsewhere, e.g. in Args.

@brandjon @tetromino Would it make sense to allow Starlark equality to go through a different method then equals to avoid this class of bugs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i didn't dig to why this isn't being hit elsewhere, but I assume it must be unique to this somehow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants