From 56ad5d2551b37bf7ed2bd56c98ec1ce751685c38 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 12 Sep 2024 11:46:37 -0700 Subject: [PATCH] Wire up `internal-inline-outputs` requirement with RE API In-memory outputs marked with the `internal-inline-outputs` execution requirement are now added to the `inline_output_files` hint field of the remote execution and remote cache API, potentially saving a round trip on files that are unconditionally read by Bazel client after spawn execution. Support for output file inlining has been added to the remote worker implementation. Fixes #8421. Closes #23428. PiperOrigin-RevId: 673938116 Change-Id: Ieaea53502c43f9fc90b99f72dd4cdb4711239dd3 --- .../build/lib/remote/GrpcCacheClient.java | 7 +- .../build/lib/remote/RemoteCache.java | 16 +- .../lib/remote/RemoteExecutionService.java | 67 ++++++-- .../RemoteRepositoryRemoteExecutor.java | 7 +- .../lib/remote/common/RemoteCacheClient.java | 8 +- .../lib/remote/disk/DiskCacheClient.java | 5 +- .../lib/remote/http/HttpCacheClient.java | 6 +- .../build/lib/remote/GrpcCacheClientTest.java | 17 +- .../build/lib/remote/RemoteCacheTest.java | 3 +- .../RemoteRepositoryRemoteExecutorTest.java | 34 +++- .../lib/remote/RemoteSpawnCacheTest.java | 55 +++++-- .../lib/remote/RemoteSpawnRunnerTest.java | 42 +++-- .../build/lib/remote/UploadManifestTest.java | 6 +- .../lib/remote/http/HttpCacheClientTest.java | 6 +- .../lib/remote/util/InMemoryCacheClient.java | 6 +- .../bazel/remote/remote_execution_test.sh | 149 ++++++++++++++++++ .../remote/worker/ActionCacheServer.java | 23 ++- .../build/remote/worker/ExecutionServer.java | 36 ++++- 18 files changed, 416 insertions(+), 77 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java index be25837e4ccdd6..e75313538fd61a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java @@ -71,6 +71,7 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; @@ -270,7 +271,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles) { GetActionResultRequest request = GetActionResultRequest.newBuilder() .setInstanceName(options.remoteInstanceName) @@ -278,6 +282,7 @@ public ListenableFuture downloadActionResult( .setActionDigest(actionKey.getDigest()) .setInlineStderr(inlineOutErr) .setInlineStdout(inlineOutErr) + .addAllInlineOutputFiles(inlineOutputFiles) .build(); return Utils.refreshIfUnauthenticatedAsync( () -> diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index 0cbe9561a72c78..906626ac3ed22a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -61,6 +61,7 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; @@ -146,7 +147,10 @@ public static CachedActionResult disk(ActionResult actionResult) { } public CachedActionResult downloadActionResult( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles) throws IOException, InterruptedException { var spawnExecutionContext = context.getSpawnExecutionContext(); @@ -180,7 +184,8 @@ public CachedActionResult downloadActionResult( spawnExecutionContext.report(SPAWN_CHECKING_REMOTE_CACHE_EVENT); } return Futures.transform( - downloadActionResultFromRemote(context, actionKey, inlineOutErr), + downloadActionResultFromRemote( + context, actionKey, inlineOutErr, inlineOutputFiles), CachedActionResult::remote, directExecutor()); } else { @@ -194,10 +199,13 @@ public CachedActionResult downloadActionResult( } private ListenableFuture downloadActionResultFromRemote( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles) { checkState(remoteCacheClient != null && context.getReadCachePolicy().allowRemoteCache()); return Futures.transformAsync( - remoteCacheClient.downloadActionResult(context, actionKey, inlineOutErr), + remoteCacheClient.downloadActionResult(context, actionKey, inlineOutErr, inlineOutputFiles), (actionResult) -> { if (actionResult == null) { return immediateFuture(null); 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 f11fe32c841cd4..cde0b74cce4e41 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 @@ -768,11 +768,19 @@ public RemoteActionResult lookupCache(RemoteAction action) action.getRemoteActionExecutionContext().getReadCachePolicy().allowAnyCache(), "spawn doesn't accept cached result"); + ImmutableSet inlineOutputFiles = ImmutableSet.of(); + PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn()); + if (inMemoryOutputPath != null) { + inlineOutputFiles = + ImmutableSet.of(action.getRemotePathResolver().localPathToOutputPath(inMemoryOutputPath)); + } + CachedActionResult cachedActionResult = remoteCache.downloadActionResult( action.getRemoteActionExecutionContext(), action.getActionKey(), - /* inlineOutErr= */ false); + /* inlineOutErr= */ false, + inlineOutputFiles); if (cachedActionResult == null) { return null; @@ -958,11 +966,13 @@ public static class FileMetadata { private final Path path; private final Digest digest; private final boolean isExecutable; + private final ByteString contents; - private FileMetadata(Path path, Digest digest, boolean isExecutable) { + private FileMetadata(Path path, Digest digest, boolean isExecutable, ByteString contents) { this.path = path; this.digest = digest; this.isExecutable = isExecutable; + this.contents = contents; } public Path path() { @@ -976,6 +986,10 @@ public Digest digest() { public boolean isExecutable() { return isExecutable; } + + public ByteString content() { + return contents; + } } static class DirectoryMetadata { @@ -1039,7 +1053,10 @@ private static DirectoryMetadata parseDirectory( for (FileNode file : dir.getFilesList()) { filesBuilder.add( new FileMetadata( - parent.getRelative(file.getName()), file.getDigest(), file.getIsExecutable())); + parent.getRelative(file.getName()), + file.getDigest(), + file.getIsExecutable(), + ByteString.EMPTY)); } ImmutableList.Builder symlinksBuilder = ImmutableList.builder(); @@ -1105,7 +1122,11 @@ static ActionResultMetadata parseActionResultMetadata( reencodeExternalToInternal(outputFile.getPath())); files.put( localPath, - new FileMetadata(localPath, outputFile.getDigest(), outputFile.getIsExecutable())); + new FileMetadata( + localPath, + outputFile.getDigest(), + outputFile.getIsExecutable(), + outputFile.getContents())); } var symlinkMap = new HashMap(); @@ -1181,7 +1202,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re var expireAtEpochMilli = Instant.now().plus(remoteOptions.remoteCacheTtl).toEpochMilli(); ActionInput inMemoryOutput = null; - AtomicReference inMemoryOutputData = new AtomicReference<>(null); + AtomicReference inMemoryOutputData = new AtomicReference<>(null); PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn()); if (inMemoryOutputPath != null) { for (ActionInput output : action.getSpawn().getOutputFiles()) { @@ -1227,15 +1248,25 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } if (isInMemoryOutputFile) { - downloadsBuilder.add( - transform( - remoteCache.downloadBlob( - context, inMemoryOutputPath.getPathString(), file.digest()), - data -> { - inMemoryOutputData.set(data); - return null; - }, - directExecutor())); + if (file.contents.isEmpty()) { + // As the contents field doesn't have presence information, we use the digest size to + // distinguish between an empty file and one that wasn't inlined. + if (file.digest.getSizeBytes() == 0) { + inMemoryOutputData.set(ByteString.EMPTY); + } else { + downloadsBuilder.add( + transform( + remoteCache.downloadBlob( + context, inMemoryOutputPath.getPathString(), file.digest()), + data -> { + inMemoryOutputData.set(ByteString.copyFrom(data)); + return null; + }, + directExecutor())); + } + } else { + inMemoryOutputData.set(file.contents); + } } } } @@ -1369,7 +1400,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } if (inMemoryOutput != null && inMemoryOutputData.get() != null) { - return new InMemoryOutput(inMemoryOutput, ByteString.copyFrom(inMemoryOutputData.get())); + return new InMemoryOutput(inMemoryOutput, inMemoryOutputData.get()); } return null; @@ -1811,6 +1842,12 @@ public RemoteActionResult executeRemotely( if (remoteOptions.remoteExecutionPriority != 0) { requestBuilder.getExecutionPolicyBuilder().setPriority(remoteOptions.remoteExecutionPriority); } + PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn()); + if (inMemoryOutputPath != null) { + requestBuilder.addInlineOutputFiles( + reencodeInternalToExternal( + action.getRemotePathResolver().localPathToOutputPath(inMemoryOutputPath))); + } ExecuteRequest request = requestBuilder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java index 48ddddf87c9ea7..3672c11b42c3b1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java @@ -25,6 +25,7 @@ import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Maps; import com.google.devtools.build.lib.analysis.platform.PlatformUtils; @@ -153,7 +154,11 @@ public ExecutionResult execute( try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { cachedActionResult = - remoteCache.downloadActionResult(context, actionKey, /* inlineOutErr= */ true); + remoteCache.downloadActionResult( + context, + actionKey, + /* inlineOutErr= */ true, + /* inlineOutputFiles= */ ImmutableSet.of()); } ActionResult actionResult = null; if (cachedActionResult != null) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java index 4a14155d2296c7..1a302ceb2f23fb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java @@ -24,6 +24,7 @@ import com.google.protobuf.ByteString; import java.io.IOException; import java.io.OutputStream; +import java.util.Set; /** * An interface for a remote caching protocol. @@ -76,11 +77,16 @@ public int hashCode() { * @param actionKey The digest of the {@link Action} that generated the action result. * @param inlineOutErr A hint to the server to inline the stdout and stderr in the {@code * ActionResult} message. + * @param inlineOutputFiles A hint to the server to inline the specified output files in the + * {@code ActionResult} message. * @return A Future representing pending download of an action result. If an action result for * {@code actionKey} cannot be found the result of the Future is {@code null}. */ ListenableFuture downloadActionResult( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr); + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles); /** * Uploads an action result for the {@code actionKey}. diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java index 192952407893e5..f1e085ab3960dd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java @@ -122,9 +122,8 @@ public DiskCacheClient( * deliberately use the mtime because the atime is more likely to be externally modified and may * be unavailable on some filesystems. * - *

Prefer calling {@link #downloadBlob} or {@link #downloadActionResult} instead, which will - * automatically update the mtime. This method should only be called by the remote worker - * implementation. + *

Prefer calling {@link #downloadBlob} instead, which will automatically update the mtime. + * This method should only be called by the remote worker implementation. * * @throws IOException if an I/O error other than a missing file occurs. */ diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java index 849ed57a07d89f..907b2babbded09 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java @@ -87,6 +87,7 @@ import java.util.Map.Entry; import java.util.NoSuchElementException; import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -617,7 +618,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles) { return retrier.executeAsync( () -> Utils.downloadAsActionResult( diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 24f21ff24ef9d6..b94b727c8d770b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -52,6 +52,7 @@ import com.google.bytestream.ByteStreamProto.WriteResponse; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Maps; import com.google.common.eventbus.EventBus; @@ -779,10 +780,12 @@ public void getActionResult( GrpcCacheClient client = newClient(remoteOptions); RemoteCache remoteCache = new RemoteCache(client, /* diskCacheClient= */ null, remoteOptions, DIGEST_UTIL); - remoteCache.downloadActionResult( - context, - DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), - /* inlineOutErr= */ false); + var unused = + remoteCache.downloadActionResult( + context, + DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), + /* inlineOutErr= */ false, + /* inlineOutputFiles= */ ImmutableSet.of()); } @Test @@ -1099,7 +1102,11 @@ public void getActionResult( }); assertThat( getFromFuture( - client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false))) + client.downloadActionResult( + context, + actionKey, + /* inlineOutErr= */ false, + /* inlineOutputFiles= */ ImmutableSet.of()))) .isNull(); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java index 8cf343e104295e..f0856cbf6487e8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java @@ -174,7 +174,8 @@ public void downloadActionResult_reportsSpawnCheckingCacheEvent() throws Excepti remoteCache.downloadActionResult( remoteActionExecutionContext, digestUtil.asActionKey(digestUtil.computeAsUtf8("key")), - /* inlineOutErr= */ false); + /* inlineOutErr= */ false, + /* inlineOutputFiles= */ ImmutableSet.of()); verify(remoteActionExecutionContext.getSpawnExecutionContext()) .report(SpawnCheckingCacheEvent.create("remote-cache")); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java index 90fead98361d3f..f5743175993d22 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java @@ -25,6 +25,7 @@ import build.bazel.remote.execution.v2.ExecuteResponse; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.events.Reporter; @@ -77,7 +78,11 @@ public void testZeroExitCodeFromCache() throws IOException, InterruptedException // Test that an ActionResult with exit code zero is accepted as cached. ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult( + any(), + any(), + /* inlineOutErr= */ eq(true), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(cachedResult)); ExecutionResult executionResult = @@ -89,7 +94,9 @@ public void testZeroExitCodeFromCache() throws IOException, InterruptedException /* workingDirectory= */ null, /* timeout= */ Duration.ZERO); - verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); + verify(remoteCache) + .downloadActionResult( + any(), any(), anyBoolean(), /* inlineOutputFiles= */ eq(ImmutableSet.of())); // Don't fallback to execution verify(remoteExecutor, never()).executeRemotely(any(), any(), any()); @@ -101,7 +108,11 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep // Test that an ActionResult with a none-zero exit code is not accepted as cached. ActionResult cachedResult = ActionResult.newBuilder().setExitCode(1).build(); - when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult( + any(), + any(), + /* inlineOutErr= */ eq(true), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(cachedResult)); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); @@ -116,7 +127,9 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep /* workingDirectory= */ null, /* timeout= */ Duration.ZERO); - verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); + verify(remoteCache) + .downloadActionResult( + any(), any(), anyBoolean(), /* inlineOutputFiles= */ eq(ImmutableSet.of())); // Fallback to execution verify(remoteExecutor).executeRemotely(any(), any(), any()); @@ -135,7 +148,11 @@ public void testInlineStdoutStderr() throws IOException, InterruptedException { .setStdoutRaw(ByteString.copyFrom(stdout)) .setStderrRaw(ByteString.copyFrom(stderr)) .build(); - when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult( + any(), + any(), + /* inlineOutErr= */ eq(true), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(cachedResult)); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); @@ -150,7 +167,12 @@ public void testInlineStdoutStderr() throws IOException, InterruptedException { /* workingDirectory= */ null, /* timeout= */ Duration.ZERO); - verify(remoteCache).downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true)); + verify(remoteCache) + .downloadActionResult( + any(), + any(), + /* inlineOutErr= */ eq(true), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); assertThat(executionResult.exitCode()).isEqualTo(0); assertThat(executionResult.stdout()).isEqualTo(stdout); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 52e1d666da5c31..76c804a8b7d869 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -110,6 +110,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -345,7 +346,8 @@ public void cacheHit() throws Exception { when(remoteCache.downloadActionResult( any(RemoteActionExecutionContext.class), actionKeyCaptor.capture(), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenAnswer( new Answer() { @Override @@ -400,7 +402,10 @@ public void cacheMiss() throws Exception { RemoteExecutionService service = cache.getRemoteExecutionService(); ArgumentCaptor actionKeyCaptor = ArgumentCaptor.forClass(ActionKey.class); when(remoteCache.downloadActionResult( - any(RemoteActionExecutionContext.class), actionKeyCaptor.capture(), anyBoolean())) + any(RemoteActionExecutionContext.class), + actionKeyCaptor.capture(), + anyBoolean(), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); @@ -487,7 +492,10 @@ public void noRemoteCacheSpawns_remoteCache() throws Exception { CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); verify(remoteCache, never()) .downloadActionResult( - any(RemoteActionExecutionContext.class), any(ActionKey.class), anyBoolean()); + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + anyBoolean(), + ArgumentMatchers.>any()); assertThat(simplePolicy.getDigest()).isNull(); assertThat(entry.hasResult()).isFalse(); SpawnResult result = @@ -524,7 +532,8 @@ public void noRemoteCacheSpawns_combinedCache() throws Exception { .downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false)); + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); assertThat(simplePolicy.getDigest()).isNull(); assertThat(entry.hasResult()).isFalse(); SpawnResult result = @@ -547,7 +556,10 @@ public void noRemoteCacheStillUsesLocalCache() throws Exception { RemoteSpawnCache cache = remoteSpawnCacheWithOptions(remoteOptions); ArgumentCaptor actionKeyCaptor = ArgumentCaptor.forClass(ActionKey.class); when(remoteCache.downloadActionResult( - any(RemoteActionExecutionContext.class), actionKeyCaptor.capture(), anyBoolean())) + any(RemoteActionExecutionContext.class), + actionKeyCaptor.capture(), + anyBoolean(), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); SimpleSpawn cacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_CACHE, "")); @@ -560,7 +572,8 @@ public void noRemoteCacheStillUsesLocalCache() throws Exception { .downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false)); + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); } @Test @@ -570,7 +583,10 @@ public void noRemoteExecStillUsesCache() throws Exception { simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, "")); ArgumentCaptor actionKeyCaptor = ArgumentCaptor.forClass(ActionKey.class); when(remoteCache.downloadActionResult( - any(RemoteActionExecutionContext.class), actionKeyCaptor.capture(), anyBoolean())) + any(RemoteActionExecutionContext.class), + actionKeyCaptor.capture(), + anyBoolean(), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); cache.lookup(cacheableSpawn, simplePolicy); @@ -581,7 +597,8 @@ public void noRemoteExecStillUsesCache() throws Exception { .downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false)); + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); } @Test @@ -594,7 +611,8 @@ public void failedActionsAreNotUploaded() throws Exception { .downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false)); + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); assertThat(entry.hasResult()).isFalse(); SpawnResult result = new SpawnResult.Builder() @@ -619,7 +637,8 @@ public void printWarningIfDownloadFails() throws Exception { .downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false)); + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of())); CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); assertThat(entry.hasResult()).isFalse(); @@ -652,7 +671,8 @@ public void orphanedCachedResultIgnored() throws Exception { when(remoteCache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenAnswer( new Answer() { @Override @@ -691,7 +711,8 @@ public void failedCacheActionAsCacheMiss() throws Exception { when(remoteCache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(actionResult)); CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); @@ -708,7 +729,10 @@ public void testDownloadMinimal() throws Exception { ActionResult success = ActionResult.newBuilder().setExitCode(0).build(); when(remoteCache.downloadActionResult( - any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false))) + any(RemoteActionExecutionContext.class), + any(), + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(success)); doReturn(null).when(cache.getRemoteExecutionService()).downloadOutputs(any(), any()); @@ -734,7 +758,10 @@ public void testDownloadMinimalIoError() throws Exception { ActionResult success = ActionResult.newBuilder().setExitCode(0).build(); when(remoteCache.downloadActionResult( - any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false))) + any(RemoteActionExecutionContext.class), + any(), + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(CachedActionResult.remote(success)); doThrow(downloadFailure) .when(cache.getRemoteExecutionService()) diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 4a4b56de493672..ed5b899268823e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -399,7 +399,8 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(failedAction); RemoteSpawnRunner runner = spy(newSpawnRunner()); @@ -442,7 +443,8 @@ public void treatFailedCachedActionAsCacheMiss_remote() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(failedAction); RemoteSpawnRunner runner = newSpawnRunner(); @@ -486,7 +488,8 @@ public void noRemoteExecutorFallbackFails() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); IOException err = new IOException("local execution error"); @@ -519,7 +522,8 @@ public void remoteCacheErrorFallbackFails() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenThrow(new IOException()); IOException err = new IOException("local execution error"); @@ -540,7 +544,8 @@ public void testLocalFallbackFailureRemoteExecutorFailure() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); when(executor.executeRemotely( any(RemoteActionExecutionContext.class), @@ -764,7 +769,8 @@ public void cacheDownloadFailureTriggersRemoteExecution() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(cachedResult); Exception downloadFailure = new BulkTransferException(new CacheNotFoundException(Digest.getDefaultInstance())); @@ -812,7 +818,8 @@ public void resultsDownloadFailureTriggersRemoteExecutionWithSkipCacheLookup() t when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); ActionResult execResult = ActionResult.newBuilder().setExitCode(31).build(); @@ -871,7 +878,8 @@ public void testRemoteExecutionTimeout() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); ExecuteResponse resp = ExecuteResponse.newBuilder() @@ -926,7 +934,8 @@ public void testRemoteExecutionTimeoutTimings() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); ExecuteResponse resp = ExecuteResponse.newBuilder() @@ -974,7 +983,8 @@ public void testRemoteExecutionTimeoutDoesNotTriggerFallback() throws Exception when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); ExecuteResponse resp = ExecuteResponse.newBuilder() @@ -1016,7 +1026,8 @@ public void testRemoteExecutionCommandFailureDoesNotTriggerFallback() throws Exc when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); ExecuteResponse failed = ExecuteResponse.newBuilder() @@ -1057,7 +1068,8 @@ public void testExitCode_executorfailure() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); when(executor.executeRemotely( any(RemoteActionExecutionContext.class), @@ -1085,7 +1097,8 @@ public void testExitCode_executionfailure() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenThrow(new IOException("reasons")); Spawn spawn = newSimpleSpawn(); @@ -1110,7 +1123,8 @@ public void testExitCode_remoteMessage() throws Exception { when(cache.downloadActionResult( any(RemoteActionExecutionContext.class), any(ActionKey.class), - /* inlineOutErr= */ eq(false))) + /* inlineOutErr= */ eq(false), + /* inlineOutputFiles= */ eq(ImmutableSet.of()))) .thenReturn(null); when(executor.executeRemotely( any(RemoteActionExecutionContext.class), diff --git a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java index 8ad57364f343fe..431185ae3da7f0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java @@ -558,9 +558,9 @@ public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDangli digestUtil, remotePathResolver, result, - /*followSymlinks=*/ false, - /*allowDanglingSymlinks=*/ true, - /*allowAbsoluteSymlinks=*/ true); + /* followSymlinks= */ false, + /* allowDanglingSymlinks= */ true, + /* allowAbsoluteSymlinks= */ true); um.addFiles(ImmutableList.of(link)); assertThat(um.getDigestToFile()).isEmpty(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java index d57efd5ac8a238..0190c738355718 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java @@ -32,6 +32,7 @@ import com.google.auth.Credentials; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.Spawn; @@ -705,7 +706,10 @@ public void actionResultRetryReadsFromStart() throws Exception { var actionResult = getFromFuture( blobStore.downloadActionResult( - remoteActionExecutionContext, new RemoteCacheClient.ActionKey(DIGEST), false)); + remoteActionExecutionContext, + new RemoteCacheClient.ActionKey(DIGEST), + /* inlineOutErr= */ false, + /* inlineOutputFiles= */ ImmutableSet.of())); assertThat(actionResult).isEqualTo(action2); } finally { testServer.stop(server); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java index 3262d711645d60..d2e0730939c005 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java @@ -34,6 +34,7 @@ import java.io.OutputStream; import java.util.AbstractMap.SimpleEntry; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executors; @@ -124,7 +125,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( - RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, + ActionKey actionKey, + boolean inlineOutErr, + Set inlineOutputFiles) { ActionResult actionResult = ac.get(actionKey); return Futures.immediateFuture(actionResult); } diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index c1ee0d73c2283f..ebc7e7a231dbde 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -3553,4 +3553,153 @@ EOF expect_log "2 remote cache hit" } +function setup_inlined_outputs() { + mkdir -p a + cat > a/input.txt <<'EOF' +input +EOF + cat > a/script.sh <<'EOF' +#!/bin/sh +cat $1 $1 > $2 +echo "$1" > $3 +EOF + chmod +x a/script.sh + cat > a/defs.bzl < a/BUILD <<'EOF' +load("//a:defs.bzl", "my_rule") + +my_rule( + name = "my_rule", + input = "input.txt", +) + +sh_binary( + name = "script", + srcs = ["script.sh"], +) +EOF +} + +function test_remote_cache_inlined_output() { + setup_inlined_outputs + + # Populate the cache. + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + expect_not_log "WARNING: Remote Cache:" + bazel clean --expunge + bazel shutdown + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --remote_grpc_log=grpc.log \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + expect_log "1 remote cache hit" + expect_not_log "WARNING: Remote Cache:" + assert_contains "input +input" bazel-bin/a/my_rule + + cat grpc.log > $TEST_log + # Assert that only the output is download as the unused_inputs_list is inlined. + # sha256 of "input\ninput\n" + expect_log "blobs/c88bd120ac840aa8d8a8fcedb6d620cd49c013730d387eb52be0c113bbcab640/12" + expect_log_n "google.bytestream.ByteStream/Read" 1 + + # Verify that the unused_inputs_list content is correct. + cat > a/input.txt <<'EOF' +modified +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + assert_contains "input" bazel-bin/a/my_rule +} + +function test_remote_execution_inlined_output() { + setup_inlined_outputs + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_grpc_log=grpc.log \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + expect_log "1 remote" + assert_contains "input +input" bazel-bin/a/my_rule + + cat grpc.log > $TEST_log + # Assert that only the output is download as the unused_inputs_list is inlined. + # sha256 of "input\ninput\n" + expect_log "blobs/c88bd120ac840aa8d8a8fcedb6d620cd49c013730d387eb52be0c113bbcab640/12" + expect_log_n "google.bytestream.ByteStream/Read" 1 + + # Verify that the unused_inputs_list content is correct. + cat > a/input.txt <<'EOF' +modified +EOF + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + assert_contains "input" bazel-bin/a/my_rule +} + +function test_remote_execution_inlined_output_with_path_mapping() { + setup_inlined_outputs + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_grpc_log=grpc.log \ + --experimental_output_paths=strip \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + expect_log "1 remote" + assert_contains "input +input" bazel-bin/a/my_rule + + cat grpc.log > $TEST_log + # Assert that only the output is download as the unused_inputs_list is inlined. + # sha256 of "input\ninput\n" + expect_log "blobs/c88bd120ac840aa8d8a8fcedb6d620cd49c013730d387eb52be0c113bbcab640/12" + expect_log_n "google.bytestream.ByteStream/Read" 1 + + # Verify that the unused_inputs_list content is correct. + cat > a/input.txt <<'EOF' +modified +EOF + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_output_paths=strip \ + //a:my_rule >& $TEST_log || fail "Failed to build //a:my_rule" + assert_contains "input" bazel-bin/a/my_rule +} + run_suite "Remote execution and remote cache tests" diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java index a11c5a757d1c2a..3f7995e2030bb6 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java @@ -24,12 +24,14 @@ import build.bazel.remote.execution.v2.GetActionResultRequest; import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.UpdateActionResultRequest; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; +import com.google.protobuf.ByteString; import com.google.protobuf.ExtensionRegistry; import io.grpc.stub.StreamObserver; @@ -53,14 +55,31 @@ public void getActionResult( RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); - var result = cache.downloadActionResult(context, actionKey, /* inlineOutErr= */ false); + var inlineOutputFiles = ImmutableSet.copyOf(request.getInlineOutputFilesList()); + var result = + cache.downloadActionResult( + context, actionKey, /* inlineOutErr= */ false, inlineOutputFiles); if (result == null) { responseObserver.onError(StatusUtils.notFoundError(request.getActionDigest())); return; } - responseObserver.onNext(result.actionResult()); + ActionResult actionResult = result.actionResult(); + for (int i = 0; i < actionResult.getOutputFilesCount(); i++) { + var outputFile = actionResult.getOutputFiles(i); + if (inlineOutputFiles.contains(outputFile.getPath())) { + var content = + ByteString.copyFrom(cache.downloadBlob(context, outputFile.getDigest()).get()); + actionResult = + actionResult.toBuilder() + .setOutputFiles(i, outputFile.toBuilder().setContents(content)) + .build(); + break; + } + } + + responseObserver.onNext(actionResult); responseObserver.onCompleted(); } catch (CacheNotFoundException e) { responseObserver.onError(StatusUtils.notFoundError(request.getActionDigest())); diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index 7e677f135d2301..20d1fd12a72221 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java @@ -30,6 +30,7 @@ import build.bazel.remote.execution.v2.WaitExecutionRequest; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.GoogleLogger; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; @@ -56,6 +57,7 @@ import com.google.devtools.build.runfiles.Runfiles; import com.google.longrunning.Operation; import com.google.protobuf.Any; +import com.google.protobuf.ByteString; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.util.Durations; import com.google.rpc.Code; @@ -75,6 +77,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; @@ -263,7 +266,12 @@ private ActionResult execute( "build-request-id: %s command-id: %s action-id: %s", meta.getCorrelatedInvocationsId(), meta.getToolInvocationId(), meta.getActionId()); logger.atFine().log("Received work for: %s", workDetails); - ActionResult result = execute(context, request.getActionDigest(), tempRoot); + ActionResult result = + execute( + context, + request.getActionDigest(), + ImmutableSet.copyOf(request.getInlineOutputFilesList()), + tempRoot); logger.atFine().log("Completed %s", workDetails); return result; } catch (Exception e) { @@ -283,7 +291,10 @@ private ActionResult execute( } private ActionResult execute( - RemoteActionExecutionContext context, Digest actionDigest, Path execRoot) + RemoteActionExecutionContext context, + Digest actionDigest, + Set inlineOutputFiles, + Path execRoot) throws IOException, InterruptedException, StatusException { Command command; Action action; @@ -415,6 +426,23 @@ private ActionResult execute( result = ActionResult.newBuilder().setExitCode(exitCode).build(); } + for (int i = 0; i < result.getOutputFilesCount(); i++) { + var outputFile = result.getOutputFiles(i); + if (inlineOutputFiles.contains(outputFile.getPath())) { + try { + ByteString content = + ByteString.copyFrom(cache.downloadBlob(context, outputFile.getDigest()).get()); + result = + result.toBuilder() + .setOutputFiles(i, outputFile.toBuilder().setContents(content)) + .build(); + } catch (ExecutionException e) { + // Inlining is best-effort. If it fails, we just don't inline the file. + } + break; + } + } + resp.setResult(result); if (errStatus != null) { @@ -454,8 +482,8 @@ private static long getUid() throws InterruptedException { com.google.devtools.build.lib.shell.Command cmd = new com.google.devtools.build.lib.shell.Command( new String[] {"id", "-u"}, - /*environmentVariables=*/ null, - /*workingDirectory=*/ null, + /* environmentVariables= */ null, + /* workingDirectory= */ null, uidTimeout); try { ByteArrayOutputStream stdout = new ByteArrayOutputStream();