Do not filter debug messages

Debug messages (generated with Skylark's `print` function) used to be filtered out by the output filter: if such messages are generated during the analysis phase in a package different to the target package (e.g. if a user builds //foo:foo and the message is generated in a dependency //bar:bar), the message are not shown by default, which makes an erroneous impression that the code for //bar:bar hasn't been executed at all and interferes with debugging. Now the output filter doesn't affect debug messages at all.

RELNOTES: Debug messages generated by `print()` are not being filtered out by --output_filter anymore, it's recommended not to use them in production code.
PiperOrigin-RevId: 177431255

7 files changed, 108 insertions, 26 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/events/Reporter.java b/src/main/java/com/google/devtools/build/lib/events/Reporter.java
index 6c164a1ebd..e654748422 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Reporter.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Reporter.java
@@ -70,7 +70,7 @@ public final class Reporter implements ExtendedEventHandler, ExceptionListener {
   /** Constructor which configures a reporter with the specified handlers. */
   public Reporter(EventBus eventBus, EventHandler... handlers) {
     this.eventBus = eventBus;
-    for (EventHandler handler: handlers) {
+    for (EventHandler handler : handlers) {
       addHandler(handler);
     }
   }
@@ -103,7 +103,10 @@ public final class Reporter implements ExtendedEventHandler, ExceptionListener {
    */
   @Override
   public synchronized void handle(Event e) {
-    if (e.getKind() != EventKind.ERROR && e.getTag() != null && !showOutput(e.getTag())) {
+    if (e.getKind() != EventKind.ERROR
+        && e.getKind() != EventKind.DEBUG
+        && e.getTag() != null
+        && !showOutput(e.getTag())) {
       return;
     }
     for (EventHandler handler : handlers) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
index 1ffb7e2cee..1edc800c2b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
@@ -52,6 +52,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace());
     codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
+    codedOut.writeBoolNoTag(semantics.incompatibleShowAllPrintMessages());
     codedOut.writeBoolNoTag(semantics.incompatibleStringIsNotIterable());
     codedOut.writeBoolNoTag(semantics.internalDoNotExportBuiltins());
     codedOut.writeBoolNoTag(semantics.internalSkylarkFlagTestCanary());
@@ -75,6 +76,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     builder.incompatibleListPlusEqualsInplace(codedIn.readBool());
     builder.incompatibleLoadArgumentIsLabel(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());
+    builder.incompatibleShowAllPrintMessages(codedIn.readBool());
     builder.incompatibleStringIsNotIterable(codedIn.readBool());
     builder.internalDoNotExportBuiltins(codedIn.readBool());
     builder.internalSkylarkFlagTestCanary(codedIn.readBool());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 6545d064b5..9f3c31891a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -208,6 +208,20 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   public boolean incompatibleNewActionsApi;
 
   @Option(
+    name = "incompatible_show_all_print_messages",
+    defaultValue = "true",
+    category = "incompatible changes",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+    help =
+        "If set to true, the print function will generate DEBUG messages that aren't affected by "
+            + "the --output_filter option. Otherwise it will generate filterable WARNING "
+            + "messages."
+  )
+  public boolean incompatibleShowAllPrintMessages;
+
+  @Option(
     name = "incompatible_string_is_not_iterable",
     defaultValue = "false",
     category = "incompatible changes",
@@ -259,6 +273,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
         .incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace)
         .incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)
+        .incompatibleShowAllPrintMessages(incompatibleShowAllPrintMessages)
         .incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
         .internalDoNotExportBuiltins(internalDoNotExportBuiltins)
         .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index b0dc7e4abd..0c798a91c1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -2139,18 +2139,17 @@ public class MethodLibrary {
         public Runtime.NoneType invoke(
             String sep, SkylarkList<?> starargs, Location loc, Environment env)
             throws EvalException {
-          String msg =
-              starargs
-                  .stream()
-                  .map(Printer::str)
-                  .collect(joining(sep));
+          String msg = starargs.stream().map(Printer::str).collect(joining(sep));
           // As part of the integration test "skylark_flag_test.sh", if the
           // "--internal_skylark_flag_test_canary" flag is enabled, append an extra marker string to
           // the output.
           if (env.getSemantics().internalSkylarkFlagTestCanary()) {
             msg += "<== skylark flag test ==>";
           }
-          env.handleEvent(Event.debug(loc, msg));
+          env.handleEvent(
+              env.getSemantics().incompatibleShowAllPrintMessages()
+                  ? Event.debug(loc, msg)
+                  : Event.warn(loc, msg));
           return Runtime.NONE;
         }
       };
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 39a7673998..685570bc48 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -51,6 +51,7 @@ public abstract class SkylarkSemantics {
   public abstract boolean incompatibleListPlusEqualsInplace();
   public abstract boolean incompatibleLoadArgumentIsLabel();
   public abstract boolean incompatibleNewActionsApi();
+  public abstract boolean incompatibleShowAllPrintMessages();
   public abstract boolean incompatibleStringIsNotIterable();
   public abstract boolean internalDoNotExportBuiltins();
   public abstract boolean internalSkylarkFlagTestCanary();
@@ -59,24 +60,26 @@ public abstract class SkylarkSemantics {
     return new AutoValue_SkylarkSemantics.Builder();
   }
 
-  public static final SkylarkSemantics DEFAULT_SEMANTICS = builder()
-      // <== Add new options here in alphabetic order ==>
-      .incompatibleBzlDisallowLoadAfterStatement(false)
-      .incompatibleCheckedArithmetic(true)
-      .incompatibleComprehensionVariablesDoNotLeak(true)
-      .incompatibleDepsetIsNotIterable(false)
-      .incompatibleDictLiteralHasNoDuplicates(true)
-      .incompatibleDisallowDictPlus(false)
-      .incompatibleDisallowKeywordOnlyArgs(true)
-      .incompatibleDisallowToplevelIfStatement(true)
-      .incompatibleDisallowUncalledSetConstructor(false)
-      .incompatibleListPlusEqualsInplace(true)
-      .incompatibleLoadArgumentIsLabel(false)
-      .incompatibleNewActionsApi(false)
-      .incompatibleStringIsNotIterable(false)
-      .internalDoNotExportBuiltins(false)
-      .internalSkylarkFlagTestCanary(false)
-      .build();
+  public static final SkylarkSemantics DEFAULT_SEMANTICS =
+      builder()
+          // <== Add new options here in alphabetic order ==>
+          .incompatibleBzlDisallowLoadAfterStatement(false)
+          .incompatibleCheckedArithmetic(true)
+          .incompatibleComprehensionVariablesDoNotLeak(true)
+          .incompatibleDepsetIsNotIterable(false)
+          .incompatibleDictLiteralHasNoDuplicates(true)
+          .incompatibleDisallowDictPlus(false)
+          .incompatibleDisallowKeywordOnlyArgs(true)
+          .incompatibleDisallowToplevelIfStatement(true)
+          .incompatibleDisallowUncalledSetConstructor(false)
+          .incompatibleListPlusEqualsInplace(true)
+          .incompatibleLoadArgumentIsLabel(false)
+          .incompatibleNewActionsApi(false)
+          .incompatibleShowAllPrintMessages(true)
+          .incompatibleStringIsNotIterable(false)
+          .internalDoNotExportBuiltins(false)
+          .internalSkylarkFlagTestCanary(false)
+          .build();
 
   /** Builder for {@link SkylarkSemantics}. All fields are mandatory. */
   @AutoValue.Builder
@@ -95,6 +98,7 @@ public abstract class SkylarkSemantics {
     public abstract Builder incompatibleListPlusEqualsInplace(boolean value);
     public abstract Builder incompatibleLoadArgumentIsLabel(boolean value);
     public abstract Builder incompatibleNewActionsApi(boolean value);
+    public abstract Builder incompatibleShowAllPrintMessages(boolean value);
     public abstract Builder incompatibleStringIsNotIterable(boolean value);
     public abstract Builder internalDoNotExportBuiltins(boolean value);
     public abstract Builder internalSkylarkFlagTestCanary(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 50307b0f28..2cb039c7b3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestBase;
 import com.google.devtools.build.lib.analysis.util.ExpectedTrimmedConfigurationErrors;
 import com.google.devtools.build.lib.analysis.util.MockRule;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.OutputFilter.RegexOutputFilter;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
@@ -453,6 +454,62 @@ public class BuildViewTest extends BuildViewTestBase {
   }
 
   @Test
+  public void testOutputFilterWithDebug() throws Exception {
+    scratch.file(
+        "java/a/BUILD",
+        "java_library(name = 'a',",
+        "  srcs = ['A.java'],",
+        "  deps = ['//java/b'])");
+    scratch.file(
+        "java/b/rules.bzl",
+        "def _impl(ctx):",
+        "  print('debug in b')",
+        "  ctx.file_action(",
+        "    output = ctx.outputs.my_output,",
+        "    content = 'foo',",
+        "  )",
+        "gen = rule(implementation = _impl, outputs = {'my_output': 'B.java'})");
+    scratch.file(
+        "java/b/BUILD",
+        "load(':rules.bzl', 'gen')",
+        "gen(name='src')",
+        "java_library(name = 'b', srcs = [':src'])");
+    reporter.setOutputFilter(RegexOutputFilter.forPattern(Pattern.compile("^//java/a")));
+
+    useConfiguration("--incompatible_show_all_print_messages=true");
+    update("//java/a:a");
+    assertContainsEvent("DEBUG /workspace/java/b/rules.bzl:2:3: debug in b");
+  }
+
+  @Test
+  public void testOutputFilterWithWarning() throws Exception {
+    scratch.file(
+        "java/a/BUILD",
+        "java_library(name = 'a',",
+        "  srcs = ['A.java'],",
+        "  deps = ['//java/b'])");
+    scratch.file(
+        "java/b/rules.bzl",
+        "def _impl(ctx):",
+        "  print('debug in b')",
+        "  ctx.file_action(",
+        "    output = ctx.outputs.my_output,",
+        "    content = 'foo',",
+        "  )",
+        "gen = rule(implementation = _impl, outputs = {'my_output': 'B.java'})");
+    scratch.file(
+        "java/b/BUILD",
+        "load(':rules.bzl', 'gen')",
+        "gen(name='src')",
+        "java_library(name = 'b', srcs = [':src'])");
+    reporter.setOutputFilter(RegexOutputFilter.forPattern(Pattern.compile("^//java/a")));
+
+    useConfiguration("--incompatible_show_all_print_messages=false");
+    update("//java/a:a");
+    assertDoesNotContainEvent("rules.bzl:2:3: debug in b");
+  }
+
+  @Test
   public void testAnalysisErrorMessageWithKeepGoing() throws Exception {
     scratch.file("a/BUILD", "sh_binary(name='a', srcs=['a1.sh', 'a2.sh'])");
     reporter.removeHandler(failFastHandler);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 2d388ca348..d2b7ef365f 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -117,6 +117,7 @@ public class SkylarkSemanticsConsistencyTest {
         "--incompatible_list_plus_equals_inplace=" + rand.nextBoolean(),
         "--incompatible_load_argument_is_label=" + rand.nextBoolean(),
         "--incompatible_new_actions_api=" + rand.nextBoolean(),
+        "--incompatible_show_all_print_messages=" + rand.nextBoolean(),
         "--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
         "--internal_do_not_export_builtins=" + rand.nextBoolean(),
         "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
@@ -141,6 +142,7 @@ public class SkylarkSemanticsConsistencyTest {
         .incompatibleListPlusEqualsInplace(rand.nextBoolean())
         .incompatibleLoadArgumentIsLabel(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())
+        .incompatibleShowAllPrintMessages(rand.nextBoolean())
         .incompatibleStringIsNotIterable(rand.nextBoolean())
         .internalDoNotExportBuiltins(rand.nextBoolean())
         .internalSkylarkFlagTestCanary(rand.nextBoolean())