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 #22460

Merged

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented May 21, 2024

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

@fmeum fmeum requested a review from a team as a code owner May 21, 2024 07:43
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Performance Issues for Performance teams team-Rules-CPP Issues for C++ rules labels May 21, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented May 21, 2024

This was a pretty non-trivial cherry pick, hope I got it right.

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 bazelbuild#20486

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

Closes bazelbuild#21963.

PiperOrigin-RevId: 635589078
Change-Id: If60fdc632a59f19dad0cb02312690c15a0540c8e
@fmeum fmeum force-pushed the 22136-cherry-pick-label-print branch from 9f12868 to e8eb194 Compare May 21, 2024 07:49
@keertk keertk enabled auto-merge May 21, 2024 12:06
@keertk keertk requested a review from Wyverald May 21, 2024 12:06
@keertk keertk changed the title Let Label#debugPrint emit label strings in display form [7.2.0] Let Label#debugPrint emit label strings in display form May 21, 2024
@iancha1992 iancha1992 removed team-Performance Issues for Performance teams team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-CPP Issues for C++ rules labels May 21, 2024
@keertk keertk added this pull request to the merge queue May 21, 2024
Merged via the queue into bazelbuild:release-7.2.0 with commit b1fb70e May 21, 2024
32 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label May 21, 2024
@fmeum fmeum deleted the 22136-cherry-pick-label-print branch May 22, 2024 07:56
keertk pushed a commit that referenced this pull request Jun 10, 2024
Release Notes:

Configurability:
+ aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (#22135)
+ Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see #22080 for more details. (#22203)
+ Starlark command-line flags can now be referred to through `alias` targets. (#22212)

ExternalDeps:
+ bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (#22137)
+ Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (#22204)
+ Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (#22261)
+ `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (#22263)
+ The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (#22371)
+ `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (#22460)
+ Changes to environment variables read via `getenv` now correctly invalidate module extensions. (#22541)
+ Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (#22650)

OSS:
+ Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (#21817)

Performance:
+ Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (#22407)

Remote-Exec:
+ The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (#22327)
+ The compact and full execution logs now contain start times for spawns (if available). (#22341)

Rules-CPP:
+ The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (#22369)

Starlark-Interpreter:
+ Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (#21960)

Acknowledgements:

This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
copybara-service bot pushed a commit that referenced this pull request Jun 10, 2024
Release Notes:

Configurability:
+ aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (#22135)
+ Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see #22080 for more details. (#22203)
+ Starlark command-line flags can now be referred to through `alias` targets. (#22212)

ExternalDeps:
+ bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (#22137)
+ Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (#22204)
+ Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (#22261)
+ `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (#22263)
+ The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (#22371)
+ `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (#22460)
+ Changes to environment variables read via `getenv` now correctly invalidate module extensions. (#22541)
+ Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (#22650)

OSS:
+ Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (#21817)

Performance:
+ Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (#22407)

Remote-Exec:
+ The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (#22327)
+ The compact and full execution logs now contain start times for spawns (if available). (#22341)

Rules-CPP:
+ The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (#22369)

Starlark-Interpreter:
+ Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (#21960)

Acknowledgements:

This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
rdeushane pushed a commit to SibrosTech/bazel that referenced this pull request Jun 19, 2024
Release Notes:

Configurability:
+ aquery: `//foo:bar` now means "all configured targets with label `//foo:bar`" instead of "choose an arbitrary configured target with label `//foo:bar`". This is in line with cquery behavior. (bazelbuild#22135)
+ Added a new flag `--incompatible_disable_native_repo_rules` to disable native repo rule usage in WORKSPACE. All native repo rules now have a Starlark counterpart that can be used in both WORKSPACE and Bzlmod; see bazelbuild#22080 for more details. (bazelbuild#22203)
+ Starlark command-line flags can now be referred to through `alias` targets. (bazelbuild#22212)

ExternalDeps:
+ bzlmod `git_repository` now accepts the `strip_prefix` arg and passes it to the underlying `git_repository` call. (bazelbuild#22137)
+ Added a new `include()` directive to `MODULE.bazel` files, which allows the root module file to be divided into multiple segments. (bazelbuild#22204)
+ Fixed certain deadlocks in repo fetching with worker threads (`--experimental_worker_for_repo_fetching=auto`). (bazelbuild#22261)
+ `print` statements in module files are now only executed for the root module and modules subject to non-registry overrides (e.g. `local_path_override`). (bazelbuild#22263)
+ The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. (bazelbuild#22371)
+ `Label` instances passed to `print` or `fail` as positional arguments are now formatted with apparent repository names (optimized for human readability). (bazelbuild#22460)
+ Changes to environment variables read via `getenv` now correctly invalidate module extensions. (bazelbuild#22541)
+ Git merge conflicts in `MODULE.bazel.lock` files can be resolved automatically. See https://bazel.build/external/lockfile#automatic-resolution for the required setup. (bazelbuild#22650)

OSS:
+ Bazel on Linux and BSD now respects the XDG_CACHE_HOME environment variable instead of assuming that ~/.cache/bazel is writable. (bazelbuild#21817)

Performance:
+ Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. (bazelbuild#22407)

Remote-Exec:
+ The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. (bazelbuild#22327)
+ The compact and full execution logs now contain start times for spawns (if available). (bazelbuild#22341)

Rules-CPP:
+ The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`. (bazelbuild#22369)

Starlark-Interpreter:
+ Starlark `min` and `max` buitins now allow a `key` callback, similarly to `sorted`. (bazelbuild#21960)

Acknowledgements:

This release contains contributions from many people at Google, as well as bazel.build machine account, Brentley Jones, Cameron Martin, Daniel Wagner-Hall, Douglas Thor, Fabian Meumertzheim, George Gensure, hvd, Isaac Torres, Keith Smiley, Mark Elliot, oquenchil, Romain Chossart, Son Luong Ngoc, Spencer Putt, Thomas Weischuh, Xdng Yng, Xùdōng Yáng, Zheng Wei Tan.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants