Add Label.to_display_form()
#21179
Closed
Add Label.to_display_form()
#21179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmeum
requested review from
gregestren
and removed request for
a team and
gregestren
github-actions
bot
added
team-Performance
fmeum
removed
team-Performance
Wyverald
reviewed
Feb 2, 2024
src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java
Show resolved
Hide resolved
| throw result.getError().getException(); | ||
| } | ||
| assertThat(result.get(skyKey).getModule().getGlobal("data")) | ||
| .isEqualTo("foo://:foo bar:@@data_repo~1.0//:bar baz:@@canonical_name//:baz"); |
There was a problem hiding this comment.
huh... why is this not bar:@data_repo//:bar?
There was a problem hiding this comment.
This verifies that to_display_form() doesn't return an apparent name in the context of a module extension, where if it did we would need to track that information in the lockfile.
There was a problem hiding this comment.
I see, because of BzlLoadFunction.java:1003.
There was a problem hiding this comment.
We got some test failures while importing this change: https://buildkite.com/bazel/google-bazel-presubmit/builds/77054
The source is at https://bazel-review.googlesource.com/q/2098361e4a924e0e09dcd48c50235b38c5c5fd84
/cc @iancha1992 @fmeum Can you please check what's different?
There was a problem hiding this comment.
The tests need to be updated now that my other PR has been merged. Will do that shortly and then you can reimport.
There was a problem hiding this comment.
@meteorcloudy Updated!
There was a problem hiding this comment.
Thanks! @iancha1992 Please re-import!
|
@bazel-io fork 7.1.0 |
Wyverald
added
awaiting-PR-merge
Fixes bazelbuild#20486 Also use `to_display_form()` to generate Clang module names in the correct form for external repositories. `java_*` rules require more delicate handling and will be migrated in a follow-up change. RELNOTES: The `to_display_form()` method on `Label` returns a string representation of a label optimized for readability by humans.
fmeum
force-pushed
the
20486-to-display-name
branch
from
7872172
to
b3e272c
Compare
fmeum
force-pushed
the
20486-to-display-name
branch
from
b3e272c
to
660bd8d
Compare
github-actions
bot
removed
the
awaiting-PR-merge
Wyverald
pushed a commit
that referenced
this pull request
Feb 12, 2024
Fixes #20486 While `to_display_form()` can be called in all contexts, it only returns apparent names for BUILD threads, which are the most common use case for display form labels. Support for module extensions can be added later, but requires explicit tracking of inverse mappings in the lockfile. Also use `to_display_form()` to generate Clang module names in the correct form for external repositories. `java_*` rules require more delicate handling and will be migrated in a follow-up change. RELNOTES: The `to_display_form()` method on `Label` returns a string representation of a label optimized for readability by humans. Closes #21179. PiperOrigin-RevId: 606330539 Change-Id: Id6a4ad79fd3d50319320789b88c618aad28db28b
Wyverald
pushed a commit
that referenced
this pull request
Feb 13, 2024
Fixes #20486 While `to_display_form()` can be called in all contexts, it only returns apparent names for BUILD threads, which are the most common use case for display form labels. Support for module extensions can be added later, but requires explicit tracking of inverse mappings in the lockfile. Also use `to_display_form()` to generate Clang module names in the correct form for external repositories. `java_*` rules require more delicate handling and will be migrated in a follow-up change. RELNOTES: The `to_display_form()` method on `Label` returns a string representation of a label optimized for readability by humans. Closes #21179. PiperOrigin-RevId: 606330539 Change-Id: Id6a4ad79fd3d50319320789b88c618aad28db28b
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 13, 2024
Fixes #20486 While `to_display_form()` can be called in all contexts, it only returns apparent names for BUILD threads, which are the most common use case for display form labels. Support for module extensions can be added later, but requires explicit tracking of inverse mappings in the lockfile. Also use `to_display_form()` to generate Clang module names in the correct form for external repositories. `java_*` rules require more delicate handling and will be migrated in a follow-up change. RELNOTES: The `to_display_form()` method on `Label` returns a string representation of a label optimized for readability by humans. Closes #21179. PiperOrigin-RevId: 606330539 Change-Id: Id6a4ad79fd3d50319320789b88c618aad28db28b Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #20486
While
to_display_form()can be called in all contexts, it only returns apparent names for BUILD threads, which are the most common use case for display form labels. Support for module extensions can be added later, but requires explicit tracking of inverse mappings in the lockfile.Also use
to_display_form()to generate Clang module names in the correct form for external repositories.java_*rules require more delicate handling and will be migrated in a follow-up change.RELNOTES: The
to_display_form()method onLabelreturns a string representation of a label optimized for readability by humans.