Make injectRemoteFile consistent with injectRemoteDirectory. Now both take a RemoteFileArtifactValue instead of the individual parameters. This allows us to remove OutputStore#injectRemoteFile in favor of calling injectOutputData directly. RELNOTES: None. PiperOrigin-RevId: 311347677
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java index afbba6a..e83f34a 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
@@ -27,19 +27,15 @@ /** * Injects metadata of a file that is stored remotely. * - * @param output a regular output file. - * @param digest the digest of the file. - * @param size the size of the file in bytes. - * @param locationIndex is only used in Blaze. - * @param actionId the id of the action that produced this file. + * @param output a regular output file + * @param metadata the remote file metadata */ - void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId); + void injectRemoteFile(Artifact output, RemoteFileArtifactValue metadata); /** - * Inject the metadata of a tree artifact whose contents are stored remotely. + * Injects the metadata of a tree artifact whose contents are stored remotely. * - * @param output an output directory + * @param output an output directory {@linkplain Artifact#isTreeArtifact tree artifact} * @param children the metadata of the files stored in the directory */ void injectRemoteDirectory(
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 d03ccbe..1cb1ef0 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
@@ -638,10 +638,11 @@ } metadataInjector.injectRemoteFile( output, - DigestUtil.toBinaryDigest(outputMetadata.digest()), - outputMetadata.digest().getSizeBytes(), - /* locationIndex= */ 1, - actionId); + new RemoteFileArtifactValue( + DigestUtil.toBinaryDigest(outputMetadata.digest()), + outputMetadata.digest().getSizeBytes(), + /*locationIndex=*/ 1, + actionId)); } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index bc5a13d..1efd143 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -498,8 +498,7 @@ } @Override - public void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId) { + public void injectRemoteFile(Artifact output, RemoteFileArtifactValue metadata) { Preconditions.checkArgument( isKnownOutput(output), "%s is not a declared output of this action", output); Preconditions.checkArgument( @@ -508,7 +507,7 @@ output); Preconditions.checkState( executionMode.get(), "Tried to inject %s outside of execution", output); - store.injectRemoteFile(output, digest, size, locationIndex, actionId); + store.injectOutputData(output, metadata); } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java index d69b309..b230929 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java
@@ -73,13 +73,6 @@ return ImmutableMap.copyOf(treeArtifactData); } - void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId) { - injectOutputData( - output, - new FileArtifactValue.RemoteFileArtifactValue(digest, size, locationIndex, actionId)); - } - final void injectOutputData(Artifact output, FileArtifactValue artifactValue) { injectedFiles.add(output); artifactData.put(output, artifactValue);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 4506c2e..16d8aa6 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -962,8 +962,7 @@ } @Override - public void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId) { + public void injectRemoteFile(Artifact output, RemoteFileArtifactValue metadata) { throw new UnsupportedOperationException(); }
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java index 8f0c7af..13790d5 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
@@ -17,8 +17,7 @@ import static com.google.devtools.build.lib.remote.util.DigestUtil.toBinaryDigest; 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.anyInt; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -78,6 +77,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -928,10 +928,8 @@ // assert assertThat(inMemoryOutput).isNull(); - verify(injector) - .injectRemoteFile(eq(a1), eq(toBinaryDigest(d1)), eq(d1.getSizeBytes()), anyInt(), any()); - verify(injector) - .injectRemoteFile(eq(a2), eq(toBinaryDigest(d2)), eq(d2.getSizeBytes()), anyInt(), any()); + verify(injector).injectRemoteFile(eq(a1), remoteFileMatchingDigest(d1)); + verify(injector).injectRemoteFile(eq(a2), remoteFileMatchingDigest(d2)); Path outputBase = artifactRoot.getRoot().asPath(); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); @@ -1145,10 +1143,8 @@ assertThat(inMemoryOutput.getContents()).isEqualTo(expectedContents); assertThat(inMemoryOutput.getOutput()).isEqualTo(a1); // The in memory file also needs to be injected as an output - verify(injector) - .injectRemoteFile(eq(a1), eq(toBinaryDigest(d1)), eq(d1.getSizeBytes()), anyInt(), any()); - verify(injector) - .injectRemoteFile(eq(a2), eq(toBinaryDigest(d2)), eq(d2.getSizeBytes()), anyInt(), any()); + verify(injector).injectRemoteFile(eq(a1), remoteFileMatchingDigest(d1)); + verify(injector).injectRemoteFile(eq(a2), remoteFileMatchingDigest(d2)); Path outputBase = artifactRoot.getRoot().asPath(); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); @@ -1184,8 +1180,7 @@ // assert assertThat(inMemoryOutput).isNull(); // The in memory file metadata also should not have been injected. - verify(injector, never()) - .injectRemoteFile(eq(a1), eq(toBinaryDigest(d1)), eq(d1.getSizeBytes()), anyInt(), any()); + verify(injector, never()).injectRemoteFile(eq(a1), remoteFileMatchingDigest(d1)); } @Test @@ -1506,6 +1501,13 @@ assertThat(remoteCache.findMissingDigests(toQuery)).isEmpty(); } + private static RemoteFileArtifactValue remoteFileMatchingDigest(Digest expectedDigest) { + return argThat( + metadata -> + Arrays.equals(metadata.getDigest(), toBinaryDigest(expectedDigest)) + && metadata.getSize() == expectedDigest.getSizeBytes()); + } + private InMemoryRemoteCache newRemoteCache(Map<Digest, byte[]> casEntries) { RemoteOptions options = Options.getDefaults(RemoteOptions.class); return new InMemoryRemoteCache(casEntries, options, digestUtil);
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 8e44093..9ff5ef9 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
@@ -179,8 +179,7 @@ public MetadataInjector getMetadataInjector() { return new MetadataInjector() { @Override - public void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId) { + public void injectRemoteFile(Artifact output, RemoteFileArtifactValue metadata) { throw new UnsupportedOperationException(); }
diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index 7196fee..0c505bc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java
@@ -132,8 +132,7 @@ public MetadataInjector getMetadataInjector() { return new MetadataInjector() { @Override - public void injectRemoteFile( - Artifact output, byte[] digest, long size, int locationIndex, String actionId) { + public void injectRemoteFile(Artifact output, RemoteFileArtifactValue metadata) { throw new UnsupportedOperationException(); }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java index 9ee4dc3..4534431 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -332,7 +332,8 @@ assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(10); // Inject a remote file of size 42. - handler.injectRemoteFile(artifact, new byte[] {1, 2, 3}, 42, 0, "ultimate-answer"); + handler.injectRemoteFile( + artifact, new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 42, 0, "ultimate-answer")); assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(42); // Reset this output, which will make the handler stat the file again. @@ -358,7 +359,8 @@ byte[] digest = new byte[] {1, 2, 3}; int size = 10; - handler.injectRemoteFile(artifact, digest, size, /* locationIndex= */ 1, "action-id"); + handler.injectRemoteFile( + artifact, new RemoteFileArtifactValue(digest, size, /*locationIndex=*/ 1, "action-id")); FileArtifactValue v = handler.getMetadata(artifact); assertThat(v).isNotNull();