Reparse options with repo mapping for bazel info#29018
Reparse options with repo mapping for bazel info#29018fmeum wants to merge 2 commits intobazelbuild:masterfrom
bazel info#29018Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
61f4bcb to
589b7c7
Compare
bazel info
…ernal repos bazel info --host_platform=@mod1//:host_platform failed with "Repository '@@mod1' is not defined" when mod1 was a local_path_override dependency. This happened because info has buildPhase = NONE, so BlazeCommandDispatcher skipped the repo mapping computation and option reparsing. Label-typed options were parsed via Label.parseCanonical() without the repo mapping, producing incorrect canonical names. Fix by lazily reparsing options with the main repo mapping inside InfoCommand's configuration supplier, preserving the lazy loading behavior for simple info queries that don't need configuration. Fixes bazelbuild#28954
589b7c7 to
043dab5
Compare
|
@fmeum Could you please take a look at the failing checks? Thanks! |
| return optionsParsingResult; | ||
| } | ||
|
|
||
| OptionsParser oldParser = (OptionsParser) optionsParsingResult; |
There was a problem hiding this comment.
can we do an instanceof check first? This cast doesn't look completely safe.
|
@fmeum Can you please fix the failures and push the changes? We need to ship 9.0.2 soon and hope this fix is included in it. |
Use getCanonicalForm() instead of getCommandLineForm() when replaying options in reparseWithRepoMapping. getCommandLineForm() preserves the original space-separated format (e.g. "--logging 6"), which fails when passed as a single element to OptionsParser.parse(). getCanonicalForm() always uses "=" syntax (e.g. "--logging=6"), which works as a single arg. Also add an instanceof check before the OptionsParser cast as suggested in review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I went over this again and think that it's too brittle. There is too much subtle logic in BlazeCommandDispatcher that this doesn't reproduce faithfully (e.g. |
Description
bazel info --host_platform=@mod1//:host_platformfailed with "Repository '@@mod1' is not defined" whenmod1was alocal_path_overridedependency.Root cause:
infohasbuildPhase = NONE, soBlazeCommandDispatcherskipped the repo mapping computation and option reparsing. Label-typed options were parsed viaLabel.parseCanonical()without the repo mapping, producing incorrect canonical names (e.g.,@@mod1instead of the actual Bzlmod canonical name).Fix: include
infoin the dispatcher's sync + repo mapping + reparsing path, the same path used bybuild/test/etc.Motivation
Fixes #28954
Build API Changes
No
Checklist
Release Notes
RELNOTES:
bazel infonow correctly resolves label-typed options (e.g.--host_platform) that reference external repositories.