Inject remote metadata for action stdout/stderr.

This will be necessary once RemoteActionFileSystem becomes a MetadataProvider in a followup change. Otherwise, a spawn receiving the stdout/stderr of a previous spawn inside the same action (as is the case with test rules) will fail to find the metadata and crash.

PiperOrigin-RevId: 515593787
Change-Id: I97e129e7f7537560a2fb6f8cbce8773e0d56ad0d

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index b5f598a..49d11f1 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -108,6 +108,7 @@
 import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.DigestUtils;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -808,7 +809,11 @@
   }
 
   private void injectRemoteArtifacts(
-      RemoteAction action, ActionResultMetadata metadata, long expireAtEpochMilli)
+      RemoteAction action,
+      ActionResult actionResult,
+      ActionResultMetadata metadata,
+      FileOutErr outErr,
+      long expireAtEpochMilli)
       throws IOException {
     FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem();
     checkState(actionFileSystem instanceof RemoteActionFileSystem);
@@ -839,6 +844,47 @@
           file.digest().getSizeBytes(),
           expireAtEpochMilli);
     }
+
+    // Inject the metadata for the stdout/stderr, which could be inputs to a followup spawn in the
+    // same action (as is the case e.g. with test rules).
+
+    maybeInjectRemoteArtifactForStdoutOrStderr(
+        remoteActionFileSystem,
+        outErr.getOutputPath(),
+        actionResult.hasStdoutDigest() ? actionResult.getStdoutDigest() : null,
+        actionResult.getStdoutRaw().size(),
+        expireAtEpochMilli);
+
+    maybeInjectRemoteArtifactForStdoutOrStderr(
+        remoteActionFileSystem,
+        outErr.getErrorPath(),
+        actionResult.hasStderrDigest() ? actionResult.getStderrDigest() : null,
+        actionResult.getStderrRaw().size(),
+        expireAtEpochMilli);
+  }
+
+  private void maybeInjectRemoteArtifactForStdoutOrStderr(
+      RemoteActionFileSystem remoteActionFileSystem,
+      Path path,
+      @Nullable Digest digest,
+      long inlinedSize,
+      long expireAtEpochMilli)
+      throws IOException {
+    if (digest != null) {
+      remoteActionFileSystem.injectRemoteFile(
+          path.asFragment(),
+          DigestUtil.toBinaryDigest(digest),
+          digest.getSizeBytes(),
+          expireAtEpochMilli);
+    } else if (inlinedSize > 0) {
+      // If we didn't get a digest back from the service, but we did get the inlined contents,
+      // compute the digest from the output file previously written by downloadOutputs.
+      remoteActionFileSystem.injectRemoteFile(
+          path.asFragment(),
+          DigestUtils.manuallyComputeDigest(path, inlinedSize),
+          inlinedSize,
+          expireAtEpochMilli);
+    }
   }
 
   /** In-memory representation of action result metadata. */
@@ -1167,7 +1213,7 @@
       }
 
       var expireAtEpochMilli = Instant.now().plus(remoteOptions.remoteCacheTtl).toEpochMilli();
-      injectRemoteArtifacts(action, metadata, expireAtEpochMilli);
+      injectRemoteArtifacts(action, result.actionResult, metadata, outErr, expireAtEpochMilli);
 
       try (SilentCloseable c = Profiler.instance().profile("Remote.downloadInMemoryOutput")) {
         if (inMemoryOutput != null) {

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
index 2eeee1a..ec6e25f 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
@@ -1133,9 +1133,7 @@
   }
 
   @Test
-  public void downloadOutputs_stdoutAndStdErrWithMinimal_works() throws Exception {
-    // Test that downloading of non-embedded stdout and stderr works
-
+  public void downloadOutputs_nonInlinedStdoutAndStderrWithMinimal_works() throws Exception {
     // arrange
     Digest dOut = cache.addContents(remoteActionExecutionContext, "stdout");
     Digest dErr = cache.addContents(remoteActionExecutionContext, "stderr");
@@ -1159,6 +1157,51 @@
     InMemoryOutput inMemoryOutput = service.downloadOutputs(action, result);
 
     // assert
+    verify(actionFileSystem)
+        .injectRemoteFile(
+            eq(outErr.getOutputPathFragment()), eq(toBinaryDigest(dOut)), eq(6L), anyLong());
+    verify(actionFileSystem)
+        .injectRemoteFile(
+            eq(outErr.getErrorPathFragment()), eq(toBinaryDigest(dErr)), eq(6L), anyLong());
+    assertThat(inMemoryOutput).isNull();
+    assertThat(outErr.outAsLatin1()).isEqualTo("stdout");
+    assertThat(outErr.errAsLatin1()).isEqualTo("stderr");
+    Path outputBase = checkNotNull(artifactRoot.getRoot().asPath());
+    assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty();
+    assertThat(context.isLockOutputFilesCalled()).isTrue();
+  }
+
+  @Test
+  public void downloadOutputs_inlinedStdoutAndStderrWithMinimal_works() throws Exception {
+    // arrange
+    Digest dOut = digestUtil.compute("stdout".getBytes(UTF_8));
+    Digest dErr = digestUtil.compute("stderr".getBytes(UTF_8));
+    ActionResult r =
+        ActionResult.newBuilder()
+            .setExitCode(0)
+            .setStdoutRaw(ByteString.copyFromUtf8("stdout"))
+            .setStderrRaw(ByteString.copyFromUtf8("stderr"))
+            .build();
+
+    RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
+    Spawn spawn = newSpawnFromResult(result);
+    RemoteActionFileSystem actionFileSystem = mock(RemoteActionFileSystem.class);
+    FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, actionFileSystem);
+    RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+    remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL;
+    RemoteExecutionService service = newRemoteExecutionService(remoteOptions);
+    RemoteAction action = service.buildRemoteAction(spawn, context);
+
+    // act
+    InMemoryOutput inMemoryOutput = service.downloadOutputs(action, result);
+
+    // assert
+    verify(actionFileSystem)
+        .injectRemoteFile(
+            eq(outErr.getOutputPathFragment()), eq(toBinaryDigest(dOut)), eq(6L), anyLong());
+    verify(actionFileSystem)
+        .injectRemoteFile(
+            eq(outErr.getErrorPathFragment()), eq(toBinaryDigest(dErr)), eq(6L), anyLong());
     assertThat(inMemoryOutput).isNull();
     assertThat(outErr.outAsLatin1()).isEqualTo("stdout");
     assertThat(outErr.errAsLatin1()).isEqualTo("stderr");