diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index d783b419a8c414..ba09826d4652ed 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -102,6 +102,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:output_service", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 02e097a393c105..1bcf2844c05dcd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -20,7 +20,6 @@ import static com.google.common.collect.Streams.stream; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; @@ -28,19 +27,24 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.cache.MetadataInjector; +import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.DelegateFileSystem; +import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.inmemoryfs.FileInfo; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryContentInfo; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.nio.channels.ReadableByteChannel; import java.util.Collection; -import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; @@ -61,8 +65,7 @@ public class RemoteActionFileSystem extends DelegateFileSystem { private final ActionInputMap inputArtifactData; private final ImmutableMap outputMapping; private final RemoteActionInputFetcher inputFetcher; - private final Map injectedMetadata = new HashMap<>(); - private final Map injectedTreeMetadata = new HashMap<>(); + private final RemoteInMemoryFileSystem remoteOutputTree; @Nullable private MetadataInjector metadataInjector = null; @@ -80,9 +83,12 @@ public class RemoteActionFileSystem extends DelegateFileSystem { this.outputMapping = stream(outputArtifacts).collect(toImmutableMap(Artifact::getExecPath, a -> a)); this.inputFetcher = checkNotNull(inputFetcher, "inputFetcher"); + this.remoteOutputTree = new RemoteInMemoryFileSystem(getDigestFunction()); } - /** Returns true if {@code path} is a file that's stored remotely. */ + /** + * Returns true if {@code path} is a file that's stored remotely. + */ boolean isRemote(Path path) { return getRemoteInputMetadata(path.asFragment()) != null; } @@ -91,43 +97,58 @@ public void updateContext(MetadataInjector metadataInjector) { this.metadataInjector = metadataInjector; } - void injectTree(SpecialArtifact tree, TreeArtifactValue metadata) { - for (Map.Entry entry : - metadata.getChildValues().entrySet()) { - FileArtifactValue childMetadata = entry.getValue(); - if (childMetadata instanceof RemoteFileArtifactValue) { - TreeFileArtifact child = entry.getKey(); - injectedMetadata.put(child.getExecPath(), (RemoteFileArtifactValue) childMetadata); - } + void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) + throws IOException { + if (!path.startsWith(outputBase)) { + return; } - - injectedTreeMetadata.put(tree.getExecPath(), metadata); - } - - void injectFile(ActionInput file, RemoteFileArtifactValue metadata) { - injectedMetadata.put(file.getExecPath(), metadata); + remoteOutputTree.injectRemoteFile(path, digest, size, actionId); } - void flush() { + void flush() throws IOException { checkNotNull(metadataInjector, "metadataInjector is null"); for (Map.Entry entry : outputMapping.entrySet()) { PathFragment execPath = entry.getKey(); + PathFragment path = execRoot.getRelative(execPath); Artifact output = entry.getValue(); if (output.isTreeArtifact()) { - TreeArtifactValue metadata = injectedTreeMetadata.get(execPath); - if (metadata != null) { - metadataInjector.injectTree((SpecialArtifact) output, metadata); + SpecialArtifact parent = (SpecialArtifact) output; + TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent); + if (remoteOutputTree.exists(path)) { + TreeArtifactValue.visitTree(remoteOutputTree.getPath(path), + ((parentRelativePath, type) -> { + if (type == Dirent.Type.DIRECTORY) { + return; + } + RemoteFileInfo remoteFile = remoteOutputTree.getRemoteFileInfo( + path.getRelative(parentRelativePath), /* followSymlinks= */ true); + if (remoteFile != null) { + TreeFileArtifact child = TreeFileArtifact.createTreeOutput(parent, + parentRelativePath); + tree.putChild(child, createRemoteMetadata(remoteFile)); + } + })); } + + // TODO: Check directory content on the local fs to support mixed tree. + + metadataInjector.injectTree(parent, tree.build()); } else { - RemoteFileArtifactValue metadata = injectedMetadata.get(execPath); - if (metadata != null) { - metadataInjector.injectFile(output, metadata); + RemoteFileInfo remoteFile = remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ + true); + if (remoteFile != null) { + metadataInjector.injectFile(output, createRemoteMetadata(remoteFile)); } } } } + private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) { + return RemoteFileArtifactValue.create(remoteFile.getFastDigest(), + remoteFile.getSize(), /* locationIndex= */ 1, remoteFile.getActionId()); + } + @Override public String getFileSystemType(PathFragment path) { return "remoteActionFS"; @@ -139,7 +160,7 @@ protected boolean delete(PathFragment path) throws IOException { if (m == null) { return super.delete(path); } - return true; + return remoteOutputTree.getPath(path).delete(); } @Override @@ -160,6 +181,7 @@ public void setLastModifiedTime(PathFragment path, long newTime) throws IOExcept if (m == null) { super.setLastModifiedTime(path, newTime); } + remoteOutputTree.setLastModifiedTime(path, newTime); } @Override @@ -391,7 +413,13 @@ private RemoteFileArtifactValue getRemoteInputMetadata(PathFragment path) { return (RemoteFileArtifactValue) m; } - return injectedMetadata.get(execPath); + RemoteFileInfo remoteFile = remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ + true); + if (remoteFile != null) { + return createRemoteMetadata(remoteFile); + } + + return null; } private void downloadFileIfRemote(PathFragment path) throws IOException { @@ -438,4 +466,93 @@ protected void createHardLink(PathFragment linkPath, PathFragment originalPath) throws IOException { super.createHardLink(linkPath, originalPath); } + + static class RemoteInMemoryFileSystem extends InMemoryFileSystem { + + public RemoteInMemoryFileSystem(DigestHashFunction hashFunction) { + super(hashFunction); + } + + + @Override + protected synchronized OutputStream getOutputStream(PathFragment path, boolean append) + throws IOException { + // To get an output stream from remote file, we need to first stage it. + throw new IllegalStateException("Shouldn't be called directly"); + } + + @Override + protected FileInfo newFile(Clock clock, PathFragment path) { + return new RemoteFileInfo(clock); + } + + void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) + throws IOException { + createDirectoryAndParents(path.getParentDirectory()); + InMemoryContentInfo node = getOrCreateWritableInode(path); + // If a node was already existed and is not a remote file node (i.e. directory or symlink node + // ), throw an error. + if (!(node instanceof RemoteFileInfo)) { + throw new IOException("Could not inject into " + node); + } + + RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node; + remoteFileInfo.set(digest, size, actionId); + } + + @Nullable + RemoteFileInfo getRemoteFileInfo(PathFragment path, boolean followSymlinks) { + InMemoryContentInfo node = inodeStatErrno(path, followSymlinks).inode(); + if (!(node instanceof RemoteFileInfo)) { + return null; + } + return (RemoteFileInfo) node; + } + } + + static class RemoteFileInfo extends FileInfo { + + private byte[] digest; + private long size; + private String actionId; + + RemoteFileInfo(Clock clock) { + super(clock); + } + + private void set(byte[] digest, long size, String actionId) { + this.digest = digest; + this.size = size; + this.actionId = actionId; + } + + @Override + public OutputStream getOutputStream(boolean append) throws IOException { + throw new IllegalStateException("Shouldn't be called directly"); + } + + @Override + public InputStream getInputStream() throws IOException { + throw new IllegalStateException("Shouldn't be called directly"); + } + + @Override + public byte[] getxattr(String name) throws IOException { + throw new IllegalStateException("Shouldn't be called directly"); + } + + @Override + public byte[] getFastDigest() { + return digest; + } + + @Override + public long getSize() { + return size; + } + + public String getActionId() { + return actionId; + } + } } 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 cfd811d22cd2f6..679fc9b5013830 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 @@ -66,12 +66,9 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; -import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; -import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.Spawn; @@ -109,7 +106,6 @@ import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; -import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -751,55 +747,33 @@ private void createSymlinks(Iterable symlinks) throws IOExcepti } } - private void injectRemoteArtifact( - RemoteAction action, ActionInput output, ActionResultMetadata metadata) throws IOException { + private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata) + throws IOException { FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem(); checkState(actionFileSystem instanceof RemoteActionFileSystem); RemoteActionExecutionContext context = action.getRemoteActionExecutionContext(); RemoteActionFileSystem remoteActionFileSystem = (RemoteActionFileSystem) actionFileSystem; - Path path = remotePathResolver.outputPathToLocalPath(output); - if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) { - DirectoryMetadata directory = metadata.directory(path); - if (directory == null) { - // A declared output wasn't created. It might have been an optional output and if not - // SkyFrame will make sure to fail. - return; - } + for (Map.Entry entry : metadata.directories()) { + DirectoryMetadata directory = entry.getValue(); if (!directory.symlinks().isEmpty()) { throw new IOException( "Symlinks in action outputs are not yet supported by " + "--experimental_remote_download_outputs=minimal"); } - SpecialArtifact parent = (SpecialArtifact) output; - TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent); + for (FileMetadata file : directory.files()) { - TreeFileArtifact child = - TreeFileArtifact.createTreeOutput(parent, file.path().relativeTo(parent.getPath())); - RemoteFileArtifactValue value = - RemoteFileArtifactValue.create( - DigestUtil.toBinaryDigest(file.digest()), - file.digest().getSizeBytes(), - /*locationIndex=*/ 1, - context.getRequestMetadata().getActionId()); - tree.putChild(child, value); + remoteActionFileSystem.injectRemoteFile(file.path().asFragment(), + DigestUtil.toBinaryDigest(file.digest()), file.digest().getSizeBytes(), + context.getRequestMetadata().getActionId()); } - remoteActionFileSystem.injectTree(parent, tree.build()); - } else { - FileMetadata outputMetadata = metadata.file(path); - if (outputMetadata == null) { - // A declared output wasn't created. It might have been an optional output and if not - // SkyFrame will make sure to fail. - return; - } - remoteActionFileSystem.injectFile( - output, - RemoteFileArtifactValue.create( - DigestUtil.toBinaryDigest(outputMetadata.digest()), - outputMetadata.digest().getSizeBytes(), - /*locationIndex=*/ 1, - context.getRequestMetadata().getActionId())); + } + + for (FileMetadata file : metadata.files()) { + remoteActionFileSystem.injectRemoteFile(file.path().asFragment(), + DigestUtil.toBinaryDigest(file.digest()), file.digest().getSizeBytes(), + context.getRequestMetadata().getActionId()); } } @@ -1154,9 +1128,10 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re inMemoryOutputDigest = m.digest(); inMemoryOutput = output; } - injectRemoteArtifact(action, output, metadata); } + injectRemoteArtifacts(action, metadata); + try (SilentCloseable c = Profiler.instance().profile("Remote.downloadInMemoryOutput")) { if (inMemoryOutput != null) { ListenableFuture inMemoryOutputDownload = diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index f07e53a94d686a..a1329d4c513e40 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.SkyFunction.Environment; +import java.io.IOException; import java.util.Map; import java.util.UUID; import javax.annotation.Nullable; @@ -100,7 +101,7 @@ public void finalizeBuild(boolean buildSuccessful) { } @Override - public void flushActionFileSystem(FileSystem actionFileSystem) { + public void flushActionFileSystem(FileSystem actionFileSystem) throws IOException { ((RemoteActionFileSystem) actionFileSystem).flush(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index eebdfbc5d2f059..a97944091724f7 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -1200,7 +1200,17 @@ private ActionExecutionValue actuallyCompleteAction( Preconditions.checkState(action.inputsDiscovered(), "Action %s successfully executed, but inputs still not known", action); - flushActionFileSystem(actionExecutionContext.getActionFileSystem(), outputService); + try { + flushActionFileSystem(actionExecutionContext.getActionFileSystem(), outputService); + } catch (IOException e) { + logger.atWarning().withCause(e).log("unable to flush action filesystem: '%s'", action); + throw toActionExecutionException( + "unable to flush action filesystem", + e, + action, + fileOutErr, + Code.ACTION_FINALIZATION_FAILURE); + } if (!checkOutputs( action, @@ -1514,7 +1524,8 @@ private static LostInputsCheck lostInputsCheck( } private static void flushActionFileSystem( - @Nullable FileSystem actionFileSystem, @Nullable OutputService outputService) { + @Nullable FileSystem actionFileSystem, @Nullable OutputService outputService) + throws IOException { if (outputService != null && actionFileSystem != null) { outputService.flushActionFileSystem(actionFileSystem); } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 749c8c8a55ca3c..e981614f7b1e06 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -205,7 +205,7 @@ default void checkActionFileSystemForLostInputs(FileSystem actionFileSystem, Act * Flush the internal state of filesystem returned by {@link #createActionFileSystem} after action * execution, before skyframe checking the action outputs. */ - default void flushActionFileSystem(FileSystem actionFileSystem) {} + default void flushActionFileSystem(FileSystem actionFileSystem) throws IOException {} default boolean supportsPathResolverForArtifactValues() { return false; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index fca7a5aa8eafdd..abc1858b0043a5 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -127,15 +127,20 @@ public void testCreateSymbolicLink() throws InterruptedException, IOException { @Test public void testDeleteRemoteFile() throws Exception { // arrange - ActionInputMap inputs = new ActionInputMap(1); - Artifact remoteArtifact = createRemoteArtifact("remote-file", "remote contents", inputs); + ActionInputMap inputs = new ActionInputMap(0); RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs); + Path path = outputRoot.getRoot().asPath().getRelative("file"); + byte[] contents = "remote contents".getBytes(StandardCharsets.UTF_8); + HashCode hashCode = HASH_FUNCTION.getHashFunction().hashBytes(contents); + actionFs.injectRemoteFile(path.asFragment(), hashCode.asBytes(), contents.length, "action-id"); + assertThat(actionFs.exists(path.asFragment())).isTrue(); // act - boolean success = actionFs.delete(remoteArtifact.getPath().asFragment()); + boolean success = actionFs.delete(path.asFragment()); // assert assertThat(success).isTrue(); + assertThat(actionFs.exists(path.asFragment())).isFalse(); } @Test @@ -143,14 +148,15 @@ public void testDeleteLocalFile() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(0); RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs); - Path filePath = actionFs.getPath(execRoot.getPathString()).getChild("local-file"); - FileSystemUtils.writeContent(filePath, StandardCharsets.UTF_8, "local contents"); + Path path = outputRoot.getRoot().asPath().getRelative("file"); + FileSystemUtils.writeContent(path, StandardCharsets.UTF_8, "local contents"); // act - boolean success = actionFs.delete(filePath.asFragment()); + boolean success = actionFs.delete(path.asFragment()); // assert assertThat(success).isTrue(); + assertThat(actionFs.exists(path.asFragment())).isFalse(); } private RemoteActionFileSystem newRemoteActionFileSystem(ActionInputMap inputs) { 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 1c38900e1a9937..cfdd1ab23fedc7 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 @@ -22,7 +22,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; @@ -60,12 +60,9 @@ import com.google.devtools.build.lib.actions.ActionUploadFinishedEvent; import com.google.devtools.build.lib.actions.ActionUploadStartedEvent; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; -import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.ExecutionRequirements; -import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; @@ -99,7 +96,6 @@ import com.google.devtools.build.lib.remote.util.TempPathGenerator; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; -import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; @@ -112,7 +108,6 @@ import com.google.protobuf.ByteString; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Collection; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -870,7 +865,7 @@ public void downloadOutputs_outputFilesWithTopLevel_download() throws Exception service.downloadOutputs(action, result); // assert - verify(actionFileSystem, never()).injectFile(any(), any()); + verify(actionFileSystem, never()).injectRemoteFile(any(), any(), anyLong(), any()); assertThat(digestUtil.compute(execRoot.getRelative("outputs/file1"))).isEqualTo(d1); assertThat(context.isLockOutputFilesCalled()).isTrue(); } @@ -900,7 +895,9 @@ public void downloadOutputs_outputFilesWithoutTopLevel_inject() throws Exception // assert assertThat(inMemoryOutput).isNull(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); - verify(actionFileSystem).injectFile(eq(a1), remoteFileMatchingDigest(d1)); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes()), any()); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -936,8 +933,12 @@ public void downloadOutputs_outputFilesWithMinimal_injectingMetadata() throws Ex assertThat(inMemoryOutput).isNull(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); Artifact a2 = ActionsTestUtil.createArtifact(artifactRoot, "file2"); - verify(actionFileSystem).injectFile(eq(a1), remoteFileMatchingDigest(d1)); - verify(actionFileSystem).injectFile(eq(a2), remoteFileMatchingDigest(d2)); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes()), any()); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a2.getExecPath())), eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes()), any()); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -986,21 +987,12 @@ public void downloadOutputs_outputDirectoriesWithMinimal_injectingMetadata() thr // assert assertThat(inMemoryOutput).isNull(); - SpecialArtifact dir = - ActionsTestUtil.createTreeArtifactWithGeneratingAction( - artifactRoot, PathFragment.create("outputs/dir")); - TreeArtifactValue tree = - TreeArtifactValue.newBuilder(dir) - .putChild( - TreeFileArtifact.createTreeOutput(dir, "file1"), - RemoteFileArtifactValue.create( - toBinaryDigest(d1), d1.getSizeBytes(), 1, action.getActionId())) - .putChild( - TreeFileArtifact.createTreeOutput(dir, "a/file2"), - RemoteFileArtifactValue.create( - toBinaryDigest(d2), d2.getSizeBytes(), 1, action.getActionId())) - .build(); - verify(actionFileSystem).injectTree(dir, tree); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative("outputs/dir/file1")), eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes()), eq(action.getActionId())); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative("outputs/dir/a/file2")), eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes()), eq(action.getActionId())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1129,8 +1121,12 @@ public void downloadOutputs_inMemoryOutputWithMinimal_downloadIt() throws Except Artifact a2 = ActionsTestUtil.createArtifact(artifactRoot, "file2"); assertThat(inMemoryOutput.getOutput()).isEqualTo(a1); // The in memory file also needs to be injected as an output - verify(actionFileSystem).injectFile(eq(a1), remoteFileMatchingDigest(d1)); - verify(actionFileSystem).injectFile(eq(a2), remoteFileMatchingDigest(d2)); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes()), eq(action.getActionId())); + verify(actionFileSystem).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a2.getExecPath())), eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes()), eq(action.getActionId())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1174,7 +1170,9 @@ public void downloadOutputs_missingInMemoryOutputWithMinimal_returnsNull() throw // assert assertThat(inMemoryOutput).isNull(); // The in memory file metadata also should not have been injected. - verify(actionFileSystem, never()).injectFile(eq(a1), remoteFileMatchingDigest(d1)); + verify(actionFileSystem, never()).injectRemoteFile( + eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes()), eq(action.getActionId())); } @Test @@ -1839,11 +1837,4 @@ private RemoteExecutionService newRemoteExecutionService( tempPathGenerator, null); } - - private static RemoteFileArtifactValue remoteFileMatchingDigest(Digest expectedDigest) { - return argThat( - metadata -> - Arrays.equals(metadata.getDigest(), toBinaryDigest(expectedDigest)) - && metadata.getSize() == expectedDigest.getSizeBytes()); - } } 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 6d5ac8f7b32fa0..00f2deec2c0d74 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 @@ -1112,6 +1112,7 @@ public void testDownloadMinimalOnCacheHit() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); RemoteExecutionService service = runner.getRemoteExecutionService(); doReturn(actionResult).when(service).lookupCache(any()); + doReturn(null).when(service).downloadOutputs(any(), any()); Spawn spawn = newSimpleSpawn(); SpawnExecutionContext policy = getSpawnContext(spawn); @@ -1140,6 +1141,7 @@ public void testDownloadMinimalOnCacheMiss() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); RemoteExecutionService service = runner.getRemoteExecutionService(); + doReturn(null).when(service).downloadOutputs(any(), any()); Spawn spawn = newSimpleSpawn(); FakeSpawnExecutionContext policy = getSpawnContext(spawn);