Properly flush remaining buffered output at the end of the build.
Previously, this didn't happen in all cases, in particular if `aquery` output
included a newline character, e.g. if the length of a string is 10 bytes it
would be cut off.
A better, but much more involved fix would be to circumvent `UiEventHandler` for `{,a,c}query` output to stdout.
PiperOrigin-RevId: 660332325
Change-Id: I1a816028660da6598a001740bd7e3c95f9a1a098
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
index 7cf6b2e..54309b0 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
@@ -675,6 +675,7 @@
}
completeBuild();
try {
+ flushStdOutStdErrBuffers();
terminal.resetTerminal();
terminal.flush();
} catch (IOException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/AqueryBuildToolTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/AqueryBuildToolTest.java
index 621758b..33af63a 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/AqueryBuildToolTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/AqueryBuildToolTest.java
@@ -17,9 +17,11 @@
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.AnalysisProtosV2.ActionGraphContainer;
import com.google.devtools.build.lib.buildtool.AqueryProcessor.AqueryActionFilterException;
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
import com.google.devtools.build.lib.cmdline.TargetPattern;
+import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.query2.aquery.ActionGraphQueryEnvironment;
import com.google.devtools.build.lib.query2.aquery.AqueryOptions;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
@@ -29,6 +31,9 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.commands.AqueryCommand;
import com.google.devtools.build.lib.server.FailureDetails.ActionQuery.Code;
+import com.google.protobuf.ExtensionRegistry;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -86,4 +91,49 @@
assertThat(result.getDetailedExitCode().getFailureDetail().getActionQuery().getCode())
.isEqualTo(Code.SKYFRAME_STATE_PREREQ_UNMET);
}
+
+ @Test
+ public void testAquerySkyframeStateProtoNotCutoff() throws Exception {
+ // First, prepare and run the build.
+ write(
+ "x/BUILD",
+ """
+ genrule(
+ name = "x",
+ srcs = ["in"],
+ # This has the length 10, so it will include a 0x0a / newline character
+ # that triggers the cutoff.
+ outs = ["1234567890"],
+ cmd = "touch $(OUTS)",
+ )
+ """);
+ write("x/in", "");
+ buildTarget("//x");
+
+ // Then, run aquery and dump the action graph as of the previous skyframe state.
+ addOptions("--output=proto", "--skyframe_state");
+ CommandEnvironment env = runtimeWrapper.newCommand(AqueryCommand.class);
+ ByteArrayOutputStream stdout = new ByteArrayOutputStream();
+ env.getReporter()
+ .addHandler(
+ event -> {
+ if (event.getKind().equals(EventKind.STDOUT)) {
+ try {
+ stdout.write(event.getMessageBytes());
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+ });
+
+ AqueryProcessor aqueryProcessor = new AqueryProcessor(null, TargetPattern.defaultParser());
+ BlazeCommandResult result = aqueryProcessor.dumpActionGraphFromSkyframe(env);
+ assertThat(result.isSuccess()).isTrue();
+
+ // Test whether stdout is a valid proto.
+ assertThat(stdout.size()).isGreaterThan(0);
+ ActionGraphContainer actionGraphContainer =
+ ActionGraphContainer.parseFrom(stdout.toByteArray(), ExtensionRegistry.getEmptyRegistry());
+ assertThat(actionGraphContainer.getActionsList()).isNotEmpty();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
index 01247ce..d17b529 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
@@ -586,14 +586,17 @@
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:runtime/blaze_command_result",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/query2",
"//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/runtime/commands",
+ "//src/main/protobuf:analysis_v2_java_proto",
"//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
+ "//third_party/protobuf:protobuf_java",
],
)
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
index 1272825..62c5860 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
@@ -241,6 +241,15 @@
assertThat(output.flushed.get(4)).doesNotContain("\033[1A\033[K");
}
+ @Test
+ public void handleOutputEvent_flushesRemainingLines() {
+ Assume.assumeTrue(testedOutput == TestedOutput.STDOUT);
+ uiEventHandler.handle(output("hello\nto\neveryone"));
+ output.assertFlushed("hello\nto\n");
+ uiEventHandler.afterCommand(new AfterCommandEvent());
+ output.assertFlushed("hello\nto\n", "everyone");
+ }
+
private Event output(String message) {
return Event.of(eventKind, message);
}
