Use debugPrint instead of str for fail arguments
#18818
Closed
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
changed the title
debugPrint instead of str for fail parametersdebugPrint instead of str for fail arguments
|
@brandjon Could you take a look at this? It's a very lightweight alternative to #17889, aiming to resolve #17375. Some change along these lines is necessary to make Bzlmod module extension error messages comprehensible - there is currently no way for them to point to the actual location that is the cause of a user error. CC @Wyverald |
github-actions
bot
added
the
awaiting-review
sgowroji
added
the
team-ExternalDeps
|
@sgowroji While this would be applied to Bzlmod, the change affects the Starlark interpreter only - could you assign it to the respective team? |
sgowroji
added
team-Starlark-Interpreter
|
@brandjon Friendly ping, on the spectrum of the incompatible changes this should be tiny, but still very useful for Bzlmod's ability to generate error messages. It would be great to improve the situation in some way for Bazel 7. In case you are busy, could @tetromino review this? |
sgowroji
added
the
team-ExternalDeps
tetromino
approved these changes
Aug 17, 2023
|
I pushed a commit to reuse the printer. |
|
Thanks! |
github-actions
bot
removed
the
awaiting-review
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
|
@bazel-io fork 6.4.0 |
bazel-io
removed
the
potential release blocker
bazel-io
pushed a commit
to bazel-io/bazel
that referenced
this pull request
Aug 18, 2023
This allows non-hermetic information (such as Starlark `Location`s) to be included in `fail` messages without allowing access to it from Starlark. It also aligns the content of `fail` output with that of `print`. This change has very mild implications for backwards compatibility: it only affects the contents of `fail` messages, which aren't accessible from Starlark; `debugPrint`'s default implementation is `str` and no built-in Starlark type overrides it; if a type overrides `debugPrint`, it would usually contain at least as detailed information as `str`; `debugPrint` is a "Bazelism" that isn't governed by the Starlark spec. Work towards bazelbuild#17375 Closes bazelbuild#18818. Co-authored-by: Alexandre Rostovtsev <arostovtsev@google.com> PiperOrigin-RevId: 557916312 Change-Id: I8f202cd8530bcebb2d99f57745289b3992d03cac
iancha1992
pushed a commit
that referenced
this pull request
Aug 18, 2023
This allows non-hermetic information (such as Starlark `Location`s) to be included in `fail` messages without allowing access to it from Starlark. It also aligns the content of `fail` output with that of `print`. This change has very mild implications for backwards compatibility: it only affects the contents of `fail` messages, which aren't accessible from Starlark; `debugPrint`'s default implementation is `str` and no built-in Starlark type overrides it; if a type overrides `debugPrint`, it would usually contain at least as detailed information as `str`; `debugPrint` is a "Bazelism" that isn't governed by the Starlark spec. Work towards #17375 Closes #18818. Co-authored-by: Alexandre Rostovtsev <arostovtsev@google.com> Commit 3abbf20 PiperOrigin-RevId: 557916312 Change-Id: I8f202cd8530bcebb2d99f57745289b3992d03cac Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im> Co-authored-by: Alexandre Rostovtsev <arostovtsev@google.com>
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.
This allows non-hermetic information (such as Starlark
Locations) to be included infailmessages without allowing access to it from Starlark. It also aligns the content offailoutput with that ofprint.This change has very mild implications for backwards compatibility: it only affects the contents of
failmessages, which aren't accessible from Starlark;debugPrint's default implementation isstrand no built-in Starlark type overrides it; if a type overridesdebugPrint, it would usually contain at least as detailed information asstr;debugPrintis a "Bazelism" that isn't governed by the Starlark spec.Work towards #17375