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

[7.2.0] Let Label#debugPrint emit label strings in display form #22136

Closed
bazel-io opened this issue Apr 25, 2024 · 3 comments
Closed

[7.2.0] Let Label#debugPrint emit label strings in display form #22136

bazel-io opened this issue Apr 25, 2024 · 3 comments

Comments

@bazel-io
Copy link
Member

Forked from #21963

@bazel-io bazel-io added this to the 7.2.0 release blockers milestone Apr 25, 2024
@bazel-io
Copy link
Member Author

Cherry-pick was attempted but there were merge conflicts in the following file(s). Please resolve manually.

src/main/java/com/google/devtools/build/lib/analysis/BazelRuleAnalysisThreadContext.java
src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
src/main/java/com/google/devtools/build/lib/cmdline/BazelStarlarkContext.java
src/main/java/com/google/devtools/build/lib/cmdline/Label.java
src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
src/main/java/com/google/devtools/build/lib/packages/BzlInitThreadContext.java
src/main/java/com/google/devtools/build/lib/packages/Package.java
src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
src/main/java/com/google/devtools/build/lib/packages/StarlarkCallbackHelper.java
src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java
src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
src/test/java/com/google/devtools/build/lib/packages/PackageTest.java
src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
src/test/java/com/google/devtools/build/lib/starlark/util/BazelEvaluationTestCase.java

cc: @bazelbuild/triage

@keertk
Copy link
Member

keertk commented May 21, 2024

@Wyverald mind submitting a PR for this one?

@fmeum
Copy link
Collaborator

fmeum commented May 21, 2024

I sent #22460.

github-merge-queue bot pushed a commit that referenced this issue May 21, 2024
…2460)

This is a reland of 30b95e3 with a
different approach to emitting display form labels that avoids adding a
new `to_display_form()` method to `Label`:
* In action command lines, which are the most frequent use of labels in
rule implementation functions, labels are automatically emitted in
display form since 9d3a8b0.
* In module extensions and repository rules, if labels can be turned
into display form, the inverse of the repository mapping would need to
be tracked in lockfiles and marker files for correct incrementality.
Furthermore, allowing implementation functions to access apparent names
would allow them to "discriminate" against them, thus possibly breaking
the user's capability to map repos at will via `use_repo` and
`repo_name`. Similar to how providers on a target can't be enumerated,
it is thus safer to not provide this information to the implementation
functions directly.

This change thus implements `StarlarkValue#debugPrint` for `Label` to
allow ruleset authors to emit labels in display form in warnings and
error messages while ensuring that Starlark logic doesn't have access to
this information. `print("My message", label)` degrades gracefully with
older Bazel versions (it just prints a canonical label literal) and can
thus be adopted immediately without a need for feature detection.

This requires changing the signature of `StarlarkValue#debugPrint` to
receive the `StarlarkThread` instead of just the `StarlarkSemantics`.
Since `debugPrint` is meant for emitting potentially non-deterministic
information, it is safe to give it access to `StarlarkThread`.

Also improves the Bzlmod cycle reporter so that it prints helpful
information on a cycle encountered in a previous iteration of this PR.

Fixes #20486

RELNOTES: `Label` instances passed to `print` or `fail` as positional
arguments are now formatted with apparent repository names (optimized
for human readability).

Closes #21963.

PiperOrigin-RevId: 635589078
Change-Id: If60fdc632a59f19dad0cb02312690c15a0540c8e

Closes #22136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants