[Skymeld] Propagate events from deps to BuildDriverKey nodes.

Otherwise, these events from the direct deps of a BuildDriverKey (e.g. ConfiguredTargetKey) could be lost in the incremental builds. PiperOrigin-RevId: 523710165 Change-Id: I41b0c8a9b3d3b29aaccd4eebfb309a4188bfda6f
bazelbuild · Apr 12, 2023 · b2d1ca9 · b2d1ca9
1 parent 2d2ff7e
commit b2d1ca9
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 2 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -877,6 +877,9 @@ protected abstract MemoizingEvaluator createEvaluator(
    * more generally action lookup nodes) to action execution nodes. We take advantage of the fact
    * that if a node depends on an action lookup node and is not itself an action lookup node, then
    * it is an execution-phase node: the action lookup nodes are terminal in the analysis phase.
+   *
+   * <p>Skymeld: propagate events to BuildDriverKey nodes, since they cover both analysis &
+   * execution.
    */
   protected static final EventFilter DEFAULT_EVENT_FILTER_WITH_ACTIONS =
       new EventFilter() {
@@ -888,7 +891,10 @@ public boolean storeEvents() {
         @Override
         public boolean shouldPropagate(SkyKey depKey, SkyKey primaryKey) {
           // Do not propagate events from analysis phase nodes to execution phase nodes.
-          return isAnalysisPhaseKey(primaryKey) || !isAnalysisPhaseKey(depKey);
+          return isAnalysisPhaseKey(primaryKey)
+              || !isAnalysisPhaseKey(depKey)
+              // Skymeld only.
+              || primaryKey instanceof BuildDriverKey;
         }
       };
 

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java
@@ -391,7 +391,6 @@ public void symlinksReplantedEachBuild() throws Exception {
   public void targetAnalysisFailure_skymeld_correctAnalysisEvents(@TestParameter boolean keepGoing)
       throws Exception {
     addOptions("--keep_going=" + keepGoing);
-    addOptions("--experimental_merged_skyframe_analysis_execution");
     writeMyRuleBzl();
     write(
         "foo/BUILD",
@@ -584,6 +583,41 @@ public void analysisOverlapPercentageSanityCheck_success() throws Exception {
     assertSingleAnalysisPhaseCompleteEventWithLabels("//foo:foo", "//foo:bar");
   }
 
+  // Regression test for b/277783687.
+  @Test
+  public void targetAnalysisFailureNullBuild_correctErrorsPropagated(
+      @TestParameter boolean keepGoing) throws Exception {
+    addOptions("--keep_going=" + keepGoing);
+    writeMyRuleBzl();
+    write(
+        "foo/BUILD",
+        "load('//foo:my_rule.bzl', 'my_rule')",
+        "my_rule(name = 'analysis_failure', srcs = ['foo.in'], deps = [':missing'])");
+    write("foo/foo.in");
+
+    if (keepGoing) {
+      assertThrows(BuildFailedException.class, () -> buildTarget("//foo:analysis_failure"));
+
+    } else {
+      assertThrows(ViewCreationFailedException.class, () -> buildTarget("//foo:analysis_failure"));
+    }
+    events.assertContainsError(
+        "in deps attribute of my_rule rule //foo:analysis_failure: rule '//foo:missing' does not"
+            + " exist");
+    events.clear();
+
+    // Null build
+    if (keepGoing) {
+      assertThrows(BuildFailedException.class, () -> buildTarget("//foo:analysis_failure"));
+
+    } else {
+      assertThrows(ViewCreationFailedException.class, () -> buildTarget("//foo:analysis_failure"));
+    }
+    events.assertContainsError(
+        "in deps attribute of my_rule rule //foo:analysis_failure: rule '//foo:missing' does not"
+            + " exist");
+  }
+
   private void assertSingleAnalysisPhaseCompleteEventWithLabels(String... labels) {
     assertThat(eventsSubscriber.getAnalysisPhaseCompleteEvents()).hasSize(1);
     AnalysisPhaseCompleteEvent analysisPhaseCompleteEvent =