Automated rollback of commit c9f9b0b.

*** Reason for rollback ***

Some tools rely on the old message and I will first try to update them.

*** Original change description ***

Fix spitting out multiple "Build completed" messages.

Also make these a proper event, so that `--ui_event_filters` is applied
properly.

Fixes bazelbuild#16085

RELNOTES[INC]:
This has the side effect of changing the message on unsuccessful builds from
```
FAILED: Build did NOT complete successfully (0 packages loaded)
```
to
```
ERROR: Build did NOT complete successfully
```
PiperOrigin-RevId: 484274963
Change-Id: I2a3a954510ccf5d0c973d63d3a570b7356c7a7d6

src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java

@@ -18,7 +18,6 @@
 import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent;
 import com.google.devtools.build.lib.clock.Clock;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
 import com.google.devtools.build.lib.skyframe.LoadingPhaseStartedEvent;
@@ -27,6 +26,7 @@
 import com.google.devtools.build.lib.util.io.PositionAwareAnsiTerminalWriter;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.io.IOException;
+import java.time.Instant;
 
 /** Tracks the state of Skymeld builds and determines what to display at each state in the UI. */
 final class SkymeldUiStateTracker extends UiStateTracker {
@@ -229,9 +229,30 @@ synchronized void progressReceiverAvailable(ExecutionProgressReceiverAvailableEv
   }
 
   @Override
-  Event buildComplete(BuildCompleteEvent event) {
+  void buildComplete(BuildCompleteEvent event) {
     buildStatus = BuildStatus.BUILD_COMPLETED;
-    return super.buildComplete(event);
+    buildCompleteAt = Instant.ofEpochMilli(clock.currentTimeMillis());
+
+    if (event.getResult().getSuccess()) {
+      int actionsCompleted = this.actionsCompleted.get();
+      if (failedTests == 0) {
+        additionalMessage =
+            "Build completed successfully, "
+                + actionsCompleted
+                + pluralize(" total action", actionsCompleted);
+      } else {
+        additionalMessage =
+            "Build completed, "
+                + failedTests
+                + pluralize(" test", failedTests)
+                + " FAILED, "
+                + actionsCompleted
+                + pluralize(" total action", actionsCompleted);
+      }
+    } else {
+      ok = false;
+      additionalMessage = "Build did NOT complete successfully";
+    }
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java

@@ -585,17 +585,15 @@ public void buildComplete(BuildCompleteEvent event) {
     // it as an event and add a timestamp, if events are supposed to have a timestamp.
     boolean done = false;
     synchronized (this) {
-      handleInternal(stateTracker.buildComplete(event));
+      stateTracker.buildComplete(event);
       ignoreRefreshLimitOnce();
+      refresh();
 
       // After a build has completed, only stop updating the UI if there is no more activities.
       if (!stateTracker.hasActivities()) {
         buildRunning = false;
         done = true;
       }
-
-      // Only refresh after we have determined whether we need to keep the progress bar up.
-      refresh();
     }
     if (done) {
       stopUpdateThread();
@@ -998,10 +996,8 @@ private synchronized void addProgressBar() throws IOException {
           TIMESTAMP_FORMAT.format(
               Instant.ofEpochMilli(clock.currentTimeMillis()).atZone(ZoneId.systemDefault()));
     }
-    if (stateTracker.hasActivities()) {
-      stateTracker.writeProgressBar(terminalWriter, /*shortVersion=*/ !cursorControl, timestamp);
-      terminalWriter.newline();
-    }
+    stateTracker.writeProgressBar(terminalWriter, /*shortVersion=*/ !cursorControl, timestamp);
+    terminalWriter.newline();
     numLinesProgressBar = countingTerminalWriter.getWrittenLines();
     if (progressInTermTitle) {
       LoggingTerminalWriter stringWriter = new LoggingTerminalWriter(true);

src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java

@@ -42,7 +42,6 @@
 import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
@@ -88,7 +87,7 @@ class UiStateTracker {
 
   private int sampleSize = 3;
 
-  protected String status;
+  private String status;
   protected String additionalMessage;
   // Not null after the loading phase has completed.
   protected RepositoryMapping mainRepositoryMapping;
@@ -463,31 +462,31 @@ synchronized void progressReceiverAvailable(ExecutionProgressReceiverAvailableEv
     executionProgressReceiver = event.getExecutionProgressReceiver();
   }
 
-  Event buildComplete(BuildCompleteEvent event) {
+  void buildComplete(BuildCompleteEvent event) {
     buildComplete = true;
     buildCompleteAt = Instant.ofEpochMilli(clock.currentTimeMillis());
 
-    status = null;
-    additionalMessage = "";
     if (event.getResult().getSuccess()) {
+      status = "INFO";
       int actionsCompleted = this.actionsCompleted.get();
       if (failedTests == 0) {
-        return Event.info(
+        additionalMessage =
             "Build completed successfully, "
                 + actionsCompleted
-                + pluralize(" total action", actionsCompleted));
+                + pluralize(" total action", actionsCompleted);
       } else {
-        return Event.info(
+        additionalMessage =
             "Build completed, "
                 + failedTests
                 + pluralize(" test", failedTests)
                 + " FAILED, "
                 + actionsCompleted
-                + pluralize(" total action", actionsCompleted));
+                + pluralize(" total action", actionsCompleted);
       }
     } else {
       ok = false;
-      return Event.error("Build did NOT complete successfully");
+      status = "FAILED";
+      additionalMessage = "Build did NOT complete successfully";
     }
   }
 
@@ -1338,9 +1337,7 @@ synchronized void writeProgressBar(
       return;
     }
 
-    if (!buildComplete) {
-      writeExecutionProgress(terminalWriter, shortVersion);
-    }
+    writeExecutionProgress(terminalWriter, shortVersion);
 
     if (!shortVersion) {
       reportOnDownloads(terminalWriter);

src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java

@@ -151,7 +151,7 @@ public void buildCompleted_stateChanges() {
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     clock.advanceMillis(SECONDS.toMillis(1));
     buildResult.setStopTime(clock.currentTimeMillis());
-    var unused = uiStateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    uiStateTracker.buildComplete(new BuildCompleteEvent(buildResult));
 
     assertThat(uiStateTracker.buildStatus).isEqualTo(BuildStatus.BUILD_COMPLETED);
   }

src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java

@@ -27,7 +27,6 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.testutil.ManualClock;
-import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.testing.junit.testparameterinjector.TestParameter;
 import com.google.testing.junit.testparameterinjector.TestParameterInjector;
@@ -47,8 +46,6 @@ public final class UiEventHandlerStdOutAndStdErrTest {
 
   private static final BuildCompleteEvent BUILD_COMPLETE_EVENT =
       new BuildCompleteEvent(new BuildResult(/*startTimeMillis=*/ 0));
-  private static final String BUILD_DID_NOT_COMPLETE_MESSAGE =
-      "\033[31m\033[1mERROR: \033[0mBuild did NOT complete successfully" + System.lineSeparator();
 
   @TestParameter private TestedOutput testedOutput;
 
@@ -96,54 +93,25 @@ private void createUiEventHandler(UiOptions uiOptions) {
   }
 
   @Test
-  public void buildComplete_outputsBuildFailedOnStderr() {
+  public void buildComplete_outputsNothing() {
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
-
-    if (testedOutput == TestedOutput.STDOUT) {
-      output.assertFlushed();
-    } else {
-      output.assertFlushed(BUILD_DID_NOT_COMPLETE_MESSAGE);
-    }
+    output.assertFlushed();
   }
 
   @Test
   public void buildComplete_flushesBufferedMessage() {
     uiEventHandler.handle(output("hello"));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    if (testedOutput == TestedOutput.STDOUT) {
-      output.assertFlushed("hello");
-    } else {
-      output.assertFlushed("hello", System.lineSeparator() + BUILD_DID_NOT_COMPLETE_MESSAGE);
-    }
-  }
-
-  @Test
-  public void buildComplete_successfulBuild() {
-    uiEventHandler.handle(output(""));
-    var buildSuccessResult = new BuildResult(/*startTimeMillis=*/ 0);
-    buildSuccessResult.setDetailedExitCode(DetailedExitCode.success());
-    uiEventHandler.buildComplete(new BuildCompleteEvent(buildSuccessResult));
-
-    if (testedOutput == TestedOutput.STDOUT) {
-      output.assertFlushed();
-    } else {
-      output.assertFlushed(
-          "\033[32mINFO: \033[0mBuild completed successfully, 0 total actions"
-              + System.lineSeparator());
-    }
+    output.assertFlushed("hello");
   }
 
   @Test
-  public void buildComplete_emptyBuffer_outputsBuildFailedOnStderr() {
+  public void buildComplete_emptyBuffer_outputsNothing() {
     uiEventHandler.handle(output(""));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    if (testedOutput == TestedOutput.STDOUT) {
-      output.assertFlushed();
-    } else {
-      output.assertFlushed(BUILD_DID_NOT_COMPLETE_MESSAGE);
-    }
+    output.assertFlushed();
   }
 
   @Test
@@ -158,11 +126,7 @@ public void handleOutputEvent_concatenatesInBuffer() {
     uiEventHandler.handle(output("there"));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    if (testedOutput == TestedOutput.STDOUT) {
-      output.assertFlushed("hello there");
-    } else {
-      output.assertFlushed("hello there", System.lineSeparator() + BUILD_DID_NOT_COMPLETE_MESSAGE);
-    }
+    output.assertFlushed("hello there");
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java

@@ -1433,7 +1433,7 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport1, transport2)));
     stateTracker.buildEventTransportsAnnounced(
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport3)));
-    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
 
     LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(true);
 
@@ -1444,6 +1444,8 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, containsString("BuildEventTransport1"));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, containsString("BuildEventTransport3"));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport1));
@@ -1454,6 +1456,8 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, containsString("BuildEventTransport3"));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport3));
@@ -1464,6 +1468,8 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, not(containsString("BuildEventTransport3")));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport2));
@@ -1474,6 +1480,8 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, not(containsString("BuildEventTransport2")));
     assertThat(output, not(containsString("BuildEventTransport3")));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
     assertThat(output.split("\\n")).hasLength(1);
   }
 
@@ -1491,14 +1499,16 @@ public void testBuildEventTransportsOnNarrowTerminal() throws IOException {
     stateTracker.buildStarted();
     stateTracker.buildEventTransportsAnnounced(
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport1, transport2)));
-    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.writeProgressBar(terminalWriter);
     String output = terminalWriter.getTranscript();
     assertThat(longestLine(output)).isAtMost(60);
     assertThat(output, containsString("1s"));
     assertThat(output, containsString("A".repeat(30) + "..."));
     assertThat(output, containsString("BuildEventTransport"));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport2));
@@ -1509,6 +1519,8 @@ public void testBuildEventTransportsOnNarrowTerminal() throws IOException {
     assertThat(output, containsString("2s"));
     assertThat(output, containsString("A".repeat(30) + "..."));
     assertThat(output, not(containsString("BuildEventTransport")));
+    assertThat(output, containsString("success"));
+    assertThat(output, containsString("complete"));
     assertThat(output.split("\\n")).hasLength(2);
   }
 
@@ -1587,7 +1599,7 @@ public void waitingRemoteCacheMessage_afterBuildComplete_visible() throws IOExce
     BuildResult buildResult = new BuildResult(clock.currentTimeMillis());
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     buildResult.setStopTime(clock.currentTimeMillis());
-    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     clock.advanceMillis(Duration.ofSeconds(2).toMillis());
     LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true);
 
@@ -1606,7 +1618,7 @@ public void waitingRemoteCacheMessage_multipleUploadEvents_countCorrectly() thro
     BuildResult buildResult = new BuildResult(clock.currentTimeMillis());
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     buildResult.setStopTime(clock.currentTimeMillis());
-    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     stateTracker.actionUploadStarted(ActionUploadStartedEvent.create(action, "b"));
     stateTracker.actionUploadStarted(ActionUploadStartedEvent.create(action, "c"));
     stateTracker.actionUploadFinished(ActionUploadFinishedEvent.create(action, "a"));