Allow module extensions to persist facts in the lockfile#26198
Allow module extensions to persist facts in the lockfile#26198fmeum wants to merge 13 commits intobazelbuild:masterfrom
Conversation
0e22b0c to
4584973
Compare
5cb1682 to
d68881c
Compare
0acf750 to
00c53b2
Compare
|
I will add freeform docs after the first review round. |
|
@bazel-io fork 8.4.0 |
**What type of PR is this?** Feature **What does this PR do? Why is it needed?** This moves the download of the "all versions" JSON, which can't hit the repository cache, from each individual `go_download_sdk` into the module extension. If the current version of Bazel supports facts, this information will also be persisted in the lockfile, allowing for truly airgapped builds assuming an up-to-date download (formerly repository) cache. See bazelbuild/bazel#26198 for the PR that adds facts support to Bazel. **Which issues(s) does this PR fix?** Fixes #3945 **Other notes for review**
meteorcloudy
left a comment
There was a problem hiding this comment.
Looks great! But please wait also for @Wyverald 's review.
I have only one question about when old facts are going to be deleted so that the lockfile won't grow forever. IIUC, the module extension will always pass new facts via extension_metadata which should refresh the lockfile, right?
| throw new SingleExtensionEvalFunctionException( | ||
| ExternalDepsException.withMessage( | ||
| Code.BAD_LOCKFILE, | ||
| "The module extension '%s' has changed its facts: %s != %s", |
There was a problem hiding this comment.
Should we also recommend what the solution could be?
I did miss this. I now added logic that deletes facts for extensions that are no longer used, mirroring what we already do for module extension entries. |
|
@Wyverald Friendly ping |
|
Is there still any chance for this to make into the 8.4 release? |
|
I'd benefit from this api immensely and would prevent me from inventing my own lock file for rules_distroless. |
|
@Wyverald Addressed the comments and also wrote some long-form docs |
Wyverald
left a comment
There was a problem hiding this comment.
hmm, basically all shell integration tests started failing with some MODULE.bazel syntax error. Could you please take a look?
Wyverald
left a comment
There was a problem hiding this comment.
looks like tests are going to pass this time. great!
|
As a rule author I want to leverage this feature but I also want to support multiple version of bazel. What is the expectation of the behaviour here?
|
|
Both 8.5.0 and current master will be on the same lockfile version after this change, so flags should be preserved between them. I haven't tested this yet, but that is the intended behavior. |
|
I think I misunderstood your setup: What does it mean for rules_python to include a lockfile generated by Bazel? Any |
|
OK, thanks for confirming this. |
RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in `module_ctx.extension_metadata(facts = ...)` and retrieve it back in future evaluations of the extension via `module_ctx.facts` without any invalidation taking place. See bazel-contrib/rules_go#4393 for a real-world application of this new feature. Fixes bazelbuild#24777 Closes bazelbuild#26198. PiperOrigin-RevId: 819640644 Change-Id: I281503c9d4d0e60f46b57e9a231c3d2a4658d486 (cherry picked from commit 330dc9f)
RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in `module_ctx.extension_metadata(facts = ...)` and retrieve it back in future evaluations of the extension via `module_ctx.facts` without any invalidation taking place. See bazel-contrib/rules_go#4393 for a real-world application of this new feature. Fixes bazelbuild#24777 Closes bazelbuild#26198. PiperOrigin-RevId: 819640644 Change-Id: I281503c9d4d0e60f46b57e9a231c3d2a4658d486 (cherry picked from commit 330dc9f)
) RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in `module_ctx.extension_metadata(facts = ...)` and retrieve it back in future evaluations of the extension via `module_ctx.facts` without any invalidation taking place. See bazel-contrib/rules_go#4393 for a real-world application of this new feature. Fixes #24777 Closes #26198. PiperOrigin-RevId: 819640644 Change-Id: I281503c9d4d0e60f46b57e9a231c3d2a4658d486 (cherry picked from commit 330dc9f)
RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in `module_ctx.extension_metadata(facts = ...)` and retrieve it back in future evaluations of the extension via `module_ctx.facts` without any invalidation taking place. See bazel-contrib/rules_go#4393 for a real-world application of this new feature. Fixes bazelbuild#24777 Closes bazelbuild#26198. PiperOrigin-RevId: 819640644 Change-Id: I281503c9d4d0e60f46b57e9a231c3d2a4658d486
) **What type of PR is this?** Feature **What does this PR do? Why is it needed?** This moves the download of the "all versions" JSON, which can't hit the repository cache, from each individual `go_download_sdk` into the module extension. If the current version of Bazel supports facts, this information will also be persisted in the lockfile, allowing for truly airgapped builds assuming an up-to-date download (formerly repository) cache. This is a reland of #4393 with the following improvements: * Adapted to the more limited facts API that actually got merged. * Prefetching of SDK hashes is now performed on a best-effort basis so that otherwise airgapped builds that provide SDK hashes do not result in failures. See bazelbuild/bazel#26198 for the PR that added facts support to Bazel. **Which issues(s) does this PR fix?** Fixes #3945 **Other notes for review** You can verify that this works by running `bazel mod show_repo @go_default_sdk` in the BCR test repo.
RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in
module_ctx.extension_metadata(facts = ...)and retrieve it back in future evaluations of the extension viamodule_ctx.factswithout any invalidation taking place.See bazel-contrib/rules_go#4393 for a real-world application of this new feature.
Fixes #24777