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

A way to cache reproducible extension results between bazel server restarts #24723

Open
jkurland-roku opened this issue Dec 17, 2024 · 6 comments · May be fixed by #24757
Open

A way to cache reproducible extension results between bazel server restarts #24723

jkurland-roku opened this issue Dec 17, 2024 · 6 comments · May be fixed by #24757
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@jkurland-roku
Copy link

Description of the feature request:

No response

Which category does this issue belong to?

Core, External Dependency

What underlying problem are you trying to solve with this feature?

Currently extensions marked as reproducible are rerun more than they need to since they aren't tracked in the lock file. Since some extensions are slow it would be useful to have a mechanism to skip running them.

Which operating system are you running Bazel on?

ubuntu

What is the output of bazel info release?

release 8.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@github-actions github-actions bot added team-Core Skyframe, bazel query, BEP, options parsing, bazelrc team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Dec 17, 2024
@fmeum
Copy link
Collaborator

fmeum commented Dec 17, 2024

https://docs.google.com/document/d/1TjA7-M5njkI1F38IC0pm305S9EOmxcUwaCIvaSmansg/edit?usp=drivesdk included an "always update" lockfile under the output base for reproducible extensions. We didn't implement it at the time simply because we didn't have examples of slow but reproducible extensions. Now that we do, I think that we should implement this.

@Wyverald @meteorcloudy What do you think?

@meteorcloudy
Copy link
Member

I think it's a good idea, I guess bazel clean --expunge should still nuke it?

@fmeum
Copy link
Collaborator

fmeum commented Dec 18, 2024

Yes, it would behave similar to the local action cache and be stored directly under the output base.

@meteorcloudy meteorcloudy removed the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Dec 18, 2024
@meteorcloudy
Copy link
Member

Happy to review a PR!

@Wyverald
Copy link
Member

SGTM!

Now that we do, I think that we should implement this.

Just for the record, what are some examples of slow but reproducible extensions?

@fmeum
Copy link
Collaborator

fmeum commented Dec 19, 2024

rules_rust's crate_universe is the example: https://bazelbuild.slack.com/archives/CSV56UT0F/p1734442667932509

copybara-service bot pushed a commit that referenced this issue Jan 6, 2025
* 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.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake extension (so far) isn't included in the lockfile.

Work towards #24723

Closes #24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2
fmeum added a commit to fmeum/bazel that referenced this issue Jan 7, 2025
* 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.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake extension (so far) isn't included in the lockfile.

Work towards bazelbuild#24723

Closes bazelbuild#24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2
(cherry picked from commit cfda178)
fmeum pushed a commit to fmeum/bazel that referenced this issue Jan 7, 2025
Currently `ModuleExtensionId#toString()` uses the default Java record repr, which isn't very user-friendly. There's really no reason not to use the `asTargetString()` method instead. This PR just renames `asTargetString()` to `toString()` and updates all call sites.

Closes bazelbuild#24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d
(cherry picked from commit d23421a)

Fix edge cases in lockfile handling

* 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.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake extension (so far) isn't included in the lockfile.

Work towards bazelbuild#24723

Closes bazelbuild#24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2
(cherry picked from commit cfda178)
@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) help wanted Someone outside the Bazel team could own this and removed untriaged labels Jan 7, 2025
meteorcloudy pushed a commit that referenced this issue Jan 8, 2025
… lockfile handling (#24845)

### Streamline `ModuleExtensionId#toString()`

Currently `ModuleExtensionId#toString()` uses the default Java record
repr, which isn't very user-friendly. There's really no reason not to
use the `asTargetString()` method instead. This PR just renames
`asTargetString()` to `toString()` and updates all call sites.

Closes #24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d 
(cherry picked from commit d23421a)

### Fix edge cases in lockfile handling

* 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.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake
extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake
extension (so far) isn't included in the lockfile.

Work towards #24723

Closes #24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2 
(cherry picked from commit cfda178)

Fixes #24754

Co-authored-by: Xdng Yng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants