Skip to content

Conversation

@bazel-io
Copy link
Member

Fixes #27517 by checking Skyframe deps in batches that stop right before any dep that may cause a cycle if checked while previous deps are out-of-date.

This is accompanied by a restructuring of RepoRecordedInput that consolidates all Skyframe logic associated with the computation of the corresponding value exclusively within that class. This will also be helpful in adding support for dynamic inputs to the remote repo contents cache in future work.

Also made the entirety of RepositoryFetchFunction use skyframe workers, so that checking the up-to-dateness of local repo contents cache entries isn't quadratic.

Closes #28206.

Co-authored-by: Xudong Yang [email protected]
PiperOrigin-RevId: 855252657
Change-Id: Ica18760ae79da5155fc0f3d8cd4f24c52a034c86

Commit 72a25a9

Fixes bazelbuild#27517 by checking Skyframe deps in batches that stop right before any dep that may cause a cycle if checked while previous deps are out-of-date.

This is accompanied by a restructuring of `RepoRecordedInput` that consolidates all Skyframe logic associated with the computation of the corresponding value exclusively within that class. This will also be helpful in adding support for dynamic inputs to the remote repo contents cache in future work.

Also made the entirety of `RepositoryFetchFunction` use skyframe workers, so that checking the up-to-dateness of local repo contents cache entries isn't quadratic.

Closes bazelbuild#28206.

Co-authored-by: Xudong Yang <[email protected]>
PiperOrigin-RevId: 855252657
Change-Id: Ica18760ae79da5155fc0f3d8cd4f24c52a034c86
@bazel-io bazel-io added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Jan 12, 2026
@bazel-io bazel-io requested a review from a team as a code owner January 12, 2026 17:32
@bazel-io bazel-io added the awaiting-review PR is awaiting review from an assigned reviewer label Jan 12, 2026
@bazel-io bazel-io requested a review from Wyverald January 12, 2026 17:32
@Wyverald Wyverald enabled auto-merge January 12, 2026 17:35
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a robust fix for potential cycles in Skyframe when checking the local repository contents cache. The core change is to check dependencies in batches, separating dependencies that could cause cycles (like files in other external repositories) from those that cannot. This is implemented through a significant and well-executed refactoring of RepoRecordedInput, introducing concepts like canBeRequestedUnconditionally and splitIntoBatches.

Additionally, the pull request improves performance by refactoring RepositoryFetchFunction to use Skyframe workers, addressing a quadratic complexity issue. The code is also made cleaner and more maintainable by consolidating logic within RepoRecordedInput and simplifying related classes like DigestWriter and StarlarkBaseExternalContext.

The changes are well-tested, with new tests specifically targeting the cycle scenario that is now fixed. Overall, this is an excellent set of changes that improves both the correctness and performance of Bazel's repository fetching mechanism.

@Wyverald Wyverald added this pull request to the merge queue Jan 12, 2026
Merged via the queue into bazelbuild:release-9.0.0 with commit c4c4559 Jan 12, 2026
46 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 12, 2026
@Wyverald Wyverald deleted the cp28206-9.0.0 branch January 12, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants