-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[8.6.0] Add bazel mod show_repo --output=streamed_proto and --output=streamed_jsonproto
#28207
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
[8.6.0] Add bazel mod show_repo --output=streamed_proto and --output=streamed_jsonproto
#28207
Conversation
…ut=streamed_jsonproto` Serialize repo definitions to the same `Target` proto that `bazel query` uses. A few more notable details: * Add back `_original_name`, even to the Starlark output. This was removed in bazelbuild#26493, but I believe it's still useful for debugging. * The output protos may contain a `$apparent_repo_name` or `$module_key` pseudo-attribute, which is the equivalent of the `## @repo_name` / `## module@version` line in the Starlark output. * Similar to the original Starlark output, the same (canonical) repo can be shown multiple times if the user explicitly specified the same repo in different ways: ```sh ❯ bazel-bin/src/bazel_nojdk mod show_repo @@rules_cc+ @rules_cc rules_cc ``` ```starlark ## @@rules_cc+: http_archive( name = "rules_cc+", ... ## @rules_cc: http_archive( name = "rules_cc+", ... ## [email protected]: http_archive( name = "rules_cc+", ... ``` ```sh ❯ bazel-bin/src/bazel_nojdk mod show_repo --output=streamed_jsonproto @@rules_cc+ @rules_cc rules_cc ``` ```js {"canonicalName":"rules_cc+", … {"canonicalName":"rules_cc+","apparentName":"@rules_cc", … {"canonicalName":"rules_cc+","moduleKey":"[email protected]", … ``` * Tighten up command argument validation so that `bazel mod show_{repo,extension} --output={graph,json}` now exits with an error, addressing a common source of user confusion. I decided not to add the same validation to all `bazel mod` subcommands since no one can possibly expect `bazel mod tidy --output=graph` to do something. Fixes bazelbuild#21617. Works towards bazelbuild#24692. Closes bazelbuild#27242.
This reverts commit 69970f1.
There was a problem hiding this 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 new protobuf-based output formats for bazel mod show_repo and enhances argument validation for several mod subcommands. The changes are well-implemented, with corresponding updates to documentation and tests. My review focuses on improving the robustness of I/O operations. I've pointed out a couple of instances where IOExceptions are silently ignored, which could result in incomplete output without notifying the user. I've suggested handling these exceptions to ensure more reliable behavior.
src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/RepoOutputFormatter.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutor.java
Show resolved
Hide resolved
| aspectResolver.computeAspectDependencies(target, dependencyFilter); | ||
| ImmutableMap.of(); | ||
| if (aspectResolver != null) { | ||
| aspectsDependencies = aspectResolver.computeAspectDependencies(target, dependencyFilter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already a null check for aspectResolver elsewhere in the file, so I thought this is warranted:
bazel/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
Lines 332 to 334 in 8c41dd9
| aspectResolver == null | |
| ? inputFile.getPackage().getOrComputeTransitivelyLoadedStarlarkFiles() | |
| : aspectResolver.computeBuildFileDependencies(inputFile.getPackage()); |
This PR reimplements #27242 in terms of
BzlmodRepoRuleValuefor Bazel 8.x, as 8.x doesn't include #26493.Serialize repo definitions to a new
Repositoryproto message.A few more notable details:
Similar to the original Starlark output, the same (canonical) repo can be shown multiple times if the user explicitly specified the same repo in different ways:
Tighten up command argument validation so that
bazel mod show_{repo,extension} --output={graph,json}now exits with an error, addressing a common source of user confusion. I decided not to add the same validation to allbazel modsubcommands since no one can possibly expectbazel mod tidy --output=graphto do something.Fixes #21617.
Works towards #24692.
Closes #27242.
Fixes #28009.