Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a persistent lockfile to speed up reproducible extensions #24757

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 19, 2024

Reproducible extensions are not included in the regular lockfile in order to keep it small and less prone to merge conflicts. But reproducible extensions can still take significant time to evaluate, which currently happens on every server restart.

This commit makes it so that Bazel maintains an additional lockfile, stored under the output base and not visible to the user. It only records information that remain correct indefinitely, such as the results of reproducible extensions as well as hashes of module files from registries. See the comments on BazelLockFileValue for a detailed comparison of the contents of the two lockfiles.

Fixes #24723

* Don't run the core logic when `--lockfile_mode` is `off` or `error` but the command doesn't forward options to Skyframe.
* Honor `reproducible` per extension eval factor, not per extension.
@fmeum fmeum force-pushed the 24723-reproducible-lockfile branch 3 times, most recently from 5b6bc19 to 7d5f6ee Compare December 19, 2024 12:48
@fmeum fmeum force-pushed the 24723-reproducible-lockfile branch 2 times, most recently from 082e2fb to 02e2e2c Compare December 19, 2024 18:54
@fmeum fmeum force-pushed the 24723-reproducible-lockfile branch from b713852 to 7e67e07 Compare December 28, 2024 22:01
@fmeum fmeum changed the title 24723 reproducible lockfile Use a persistent lockfile to speed up reproducible extensions Dec 29, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 29, 2024

Notes for the review:

  • Stacked on Fix edge cases in lockfile handling #24754
  • I haven't added any new tests yet. I will do so after we have reached consensus on the implementation.
  • The failing test cases are caused by Skyframe node invalidation after the first bazel command in each test due to the creation of the new lockfile. This affects commands that operate on Skyframe state directly. These commands are inherently brittle, so I lean towards the simple workaround of adding an additional bazel build //... --nobuild invocation to the relevant tests instead of e.g. generating a default persistent lockfile for tests.

@fmeum fmeum marked this pull request as ready for review December 29, 2024 13:03
@fmeum fmeum removed request for a team, gregestren and fweikert December 29, 2024 13:03
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Documentation Documentation improvements that cannot be directly linked to other team labels awaiting-review PR is awaiting review from an assigned reviewer labels Dec 29, 2024
@fmeum fmeum removed the team-Configurability platforms, toolchains, cquery, select(), config transitions label Dec 29, 2024
@fmeum fmeum removed the team-Documentation Documentation improvements that cannot be directly linked to other team labels label Dec 29, 2024
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-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A way to cache reproducible extension results between bazel server restarts
1 participant