-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Treat DEBUG
events as progress-like.
#16762
Merged
ShreeM01
merged 2 commits into
bazelbuild:release-6.0.0
from
ShreeM01:ks/cherry-pick16721
Nov 14, 2022
Merged
Treat DEBUG
events as progress-like.
#16762
ShreeM01
merged 2 commits into
bazelbuild:release-6.0.0
from
ShreeM01:ks/cherry-pick16721
Nov 14, 2022
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
The primary source of `DEBUG` events is the starlark `print()` function. The motivation in making this change is to emit `print()` statements more intelligibly, including: * Not deduplicating multiple identical prints from the same line of code. * Not replaying prints when the corresponding skyframe node is cached. For example, we currently replay prints in a `BUILD` file even when the package is up to date and the starlark code is not executed. A secondary benefit is that we don't spend memory retaining `DEBUG` events in skyframe. This change makes `DEBUG` events match the semantics of `INFO` and `PROGRESS` events. See the documentation on `Reportable#storeForReplay`. There is some chance that this causes additional spam or breaks something relying on cached print statements in incremental build output, but per [style guide](https://bazel.build/rules/bzl-style#print-for-debugging), `print()` should not be used for production code anyway. There are a couple other minor sources of `DEBUG` events besides `print()`, and those semantics will be changed as well (for the better, in my opinion). Fixes #16721. RELNOTES: Starlark `print()` statements are now emitted iff the line of code is executed. They are no longer replayed on subsequent invocations unless the Starlark code is re-executed. Additionally, multiple identical `print()` statements (same string from the same line of code, e.g. from a loop) are all emitted and no longer deduplicated. PiperOrigin-RevId: 487645012 Change-Id: I8b13df67febc9f330c864930688bca31c3711276
justinhorvitz
approved these changes
Nov 14, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meteorcloudy fyi
jdai8
added a commit
to jdai8/bazel
that referenced
this pull request
Mar 7, 2023
Updates the docs to match bazelbuild#16762.
fweikert
pushed a commit
to fweikert/bazel
that referenced
this pull request
May 25, 2023
Updates the docs to match bazelbuild#16762. Closes bazelbuild#17674. PiperOrigin-RevId: 520537670 Change-Id: Ied1a8fe399007754805bc4ffb9d6bebfd50efe51
This was referenced May 30, 2023
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.
The primary source of
DEBUG
events is the starlarkprint()
function. The motivation in making this change is to emitprint()
statements more intelligibly, including:BUILD
file even when the package is up to date and the starlark code is not executed.A secondary benefit is that we don't spend memory retaining
DEBUG
events in skyframe.This change makes
DEBUG
events match the semantics ofINFO
andPROGRESS
events. See the documentation onReportable#storeForReplay
. There is some chance that this causes additional spam or breaks something relying on cached print statements in incremental build output, but per style guide,print()
should not be used for production code anyway.There are a couple other minor sources of
DEBUG
events besidesprint()
, and those semantics will be changed as well (for the better, in my opinion).Fixes #16721.
RELNOTES: Starlark
print()
statements are now emitted iff the line of code is executed. They are no longer replayed on subsequent invocations unless the Starlark code is re-executed. Additionally, multiple identicalprint()
statements (same string from the same line of code, e.g. from a loop) are all emitted and no longer deduplicated.PiperOrigin-RevId: 487645012
Change-Id: I8b13df67febc9f330c864930688bca31c3711276