Skip to content

Commit 6e945e8

Browse files
justinhorvitzcopybara-github
authored andcommitted
Treat DEBUG events as progress-like.
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 bazelbuild#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
1 parent 6c77c5a commit 6e945e8

File tree

5 files changed

+41
-3
lines changed

5 files changed

+41
-3
lines changed

src/main/java/com/google/devtools/build/lib/events/Event.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public void reportTo(ExtendedEventHandler handler) {
7373

7474
@Override
7575
public boolean storeForReplay() {
76-
return kind != EventKind.PROGRESS && kind != EventKind.INFO;
76+
return kind != EventKind.PROGRESS && kind != EventKind.INFO && kind != EventKind.DEBUG;
7777
}
7878

7979
public EventKind getKind() {

src/main/java/com/google/devtools/build/lib/events/Reportable.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ public interface Reportable {
4242
*
4343
* <p>Evaluations may disable all event storage and replay by using a custom {@link
4444
* com.google.devtools.build.skyframe.EventFilter}, in which case this method is only used to
45-
* fulfill the semantics of {@link
46-
* com.google.devtools.build.skyframe.SkyFunction.Environment#reportEvent}.
45+
* fulfill the semantics described at {@link
46+
* com.google.devtools.build.skyframe.SkyFunction.Environment#getListener}.
4747
*
4848
* <p>This method is not relevant for events which do not originate from {@link
4949
* com.google.devtools.build.skyframe.SkyFunction} evaluation.

src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java

+32
Original file line numberDiff line numberDiff line change
@@ -3780,4 +3780,36 @@ public void nonExecutableStarlarkRuleReturningTestEnvironmentProducesAWarning()
37803780
+ " non-test target has no effect",
37813781
ImmutableSet.of(EventKind.WARNING));
37823782
}
3783+
3784+
@Test
3785+
public void identicalPrintStatementsOnSameLineNotDeduplicated_buildFileLoop() throws Exception {
3786+
scratch.file("foo/BUILD", "[print('this is a print statement') for _ in range(2)]");
3787+
update("//foo:all", /*loadingPhaseThreads=*/ 1, /*doAnalysis=*/ false);
3788+
assertContainsEventWithFrequency("this is a print statement", 2);
3789+
}
3790+
3791+
@Test
3792+
public void identicalPrintStatementsOnSameLineNotDeduplicated_macroCalledFromMultipleBuildFiles()
3793+
throws Exception {
3794+
scratch.file("defs/BUILD");
3795+
scratch.file("defs/macro.bzl", "def macro():", " print('this is a print statement')");
3796+
scratch.file("foo/BUILD", "load('//defs:macro.bzl', 'macro')", "macro()");
3797+
scratch.file("bar/BUILD", "load('//defs:macro.bzl', 'macro')", "macro()");
3798+
update("//...", /*loadingPhaseThreads=*/ 1, /*doAnalysis=*/ false);
3799+
assertContainsEventWithFrequency("this is a print statement", 2);
3800+
}
3801+
3802+
@Test
3803+
public void identicalPrintStatementsOnSameLineNotDeduplicated_ruleImplementationFunction()
3804+
throws Exception {
3805+
scratch.file(
3806+
"foo/defs.bzl",
3807+
"def _my_rule_impl(ctx):",
3808+
" print('this is a print statement')",
3809+
"my_rule = rule(implementation = _my_rule_impl)");
3810+
scratch.file(
3811+
"foo/BUILD", "load(':defs.bzl', 'my_rule')", "my_rule(name = 'a')", "my_rule(name = 'b')");
3812+
update("//foo:all", /*loadingPhaseThreads=*/ 1, /*doAnalysis=*/ true);
3813+
assertContainsEventWithFrequency("this is a print statement", 2);
3814+
}
37833815
}

src/test/shell/integration/starlark_configurations_external_workspaces_test.sh

+4
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,15 @@ EOF
228228
expect_log "value before transition: prickly-pear"
229229
expect_log "value after transition: prickly-pear"
230230

231+
bazel clean 2>"$TEST_log" || fail "Clean failed"
232+
231233
bazel build :my_target --@sub//:my_flag=prickly-pear \
232234
> output 2>"$TEST_log" || fail "Expected success"
233235
expect_log "value before transition: prickly-pear"
234236
expect_log "value after transition: prickly-pear"
235237

238+
bazel clean 2>"$TEST_log" || fail "Clean failed"
239+
236240
bazel build :my_target --@//:my_flag=prickly-pear \
237241
> output 2>"$TEST_log" || fail "Expected success"
238242
expect_log "value before transition: prickly-pear"

src/test/shell/integration/starlark_configurations_test.sh

+2
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ function test_multiple_starlark_flags() {
173173
expect_log "type=coffee"
174174
expect_log "temp=iced"
175175

176+
bazel clean 2>"$TEST_log" || fail "Clean failed"
177+
176178
# Ensure that order doesn't matter.
177179
bazel build //$pkg:my_drink --//$pkg:temp="iced" --//$pkg:type="coffee" \
178180
> output 2>"$TEST_log" || fail "Expected success"

0 commit comments

Comments
 (0)