diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index 52def7ce3daf7a..b44f9e9a3b8045 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -109,15 +109,6 @@ public int getLocationIndex() { return 0; } - /** - * Remote action source identifier for the file. - * - *

"" indicates that a remote action output was not the source of this artifact. - */ - public String getActionId() { - return ""; - } - /** Returns {@code true} if the file only exists remotely. */ public boolean isRemote() { return false; @@ -551,35 +542,27 @@ public static class RemoteFileArtifactValue extends FileArtifactValue { protected final byte[] digest; protected final long size; protected final int locationIndex; - protected final String actionId; - private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) { - this.digest = Preconditions.checkNotNull(digest, actionId); + private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex) { + this.digest = Preconditions.checkNotNull(digest); this.size = size; this.locationIndex = locationIndex; - this.actionId = actionId; - } - - public static RemoteFileArtifactValue create( - byte[] digest, long size, int locationIndex, String actionId) { - return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); } public static RemoteFileArtifactValue create(byte[] digest, long size, int locationIndex) { - return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ ""); + return new RemoteFileArtifactValue(digest, size, locationIndex); } public static RemoteFileArtifactValue create( byte[] digest, long size, int locationIndex, - String actionId, @Nullable PathFragment materializationExecPath) { if (materializationExecPath != null) { return new RemoteFileArtifactValueWithMaterializationPath( - digest, size, locationIndex, actionId, materializationExecPath); + digest, size, locationIndex, materializationExecPath); } - return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + return new RemoteFileArtifactValue(digest, size, locationIndex); } @Override @@ -591,13 +574,12 @@ public boolean equals(Object o) { RemoteFileArtifactValue that = (RemoteFileArtifactValue) o; return Arrays.equals(digest, that.digest) && size == that.size - && locationIndex == that.locationIndex - && Objects.equals(actionId, that.actionId); + && locationIndex == that.locationIndex; } @Override public int hashCode() { - return Objects.hash(Arrays.hashCode(digest), size, locationIndex, actionId); + return Objects.hash(Arrays.hashCode(digest), size, locationIndex); } @Override @@ -620,11 +602,6 @@ public long getSize() { return size; } - @Override - public String getActionId() { - return actionId; - } - @Override public long getModifiedTime() { throw new UnsupportedOperationException( @@ -652,7 +629,6 @@ public String toString() { .add("digest", bytesToString(digest)) .add("size", size) .add("locationIndex", locationIndex) - .add("actionId", actionId) .toString(); } } @@ -668,12 +644,8 @@ public static final class RemoteFileArtifactValueWithMaterializationPath private final PathFragment materializationExecPath; private RemoteFileArtifactValueWithMaterializationPath( - byte[] digest, - long size, - int locationIndex, - String actionId, - PathFragment materializationExecPath) { - super(digest, size, locationIndex, actionId); + byte[] digest, long size, int locationIndex, PathFragment materializationExecPath) { + super(digest, size, locationIndex); this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath); } @@ -693,14 +665,12 @@ public boolean equals(Object o) { return Arrays.equals(digest, that.digest) && size == that.size && locationIndex == that.locationIndex - && Objects.equals(actionId, that.actionId) && Objects.equals(materializationExecPath, that.materializationExecPath); } @Override public int hashCode() { - return Objects.hash( - Arrays.hashCode(digest), size, locationIndex, actionId, materializationExecPath); + return Objects.hash(Arrays.hashCode(digest), size, locationIndex, materializationExecPath); } @Override @@ -709,7 +679,6 @@ public String toString() { .add("digest", bytesToString(digest)) .add("size", size) .add("locationIndex", locationIndex) - .add("actionId", actionId) .add("materializationExecPath", materializationExecPath) .toString(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index 937b3538e20afc..3809a4f7db92ec 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java @@ -76,7 +76,7 @@ public class CompactPersistentActionCache implements ActionCache { private static final int NO_INPUT_DISCOVERY_COUNT = -1; - private static final int VERSION = 14; + private static final int VERSION = 15; private static final class ActionMap extends PersistentMap { private final Clock clock; @@ -466,8 +466,6 @@ private static void encodeRemoteMetadata( VarInt.putVarInt(value.getLocationIndex(), sink); - VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink); - Optional materializationExecPath = value.getMaterializationExecPath(); if (materializationExecPath.isPresent()) { VarInt.putVarInt(1, sink); @@ -491,8 +489,6 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( int locationIndex = VarInt.getVarInt(source); - String actionId = getStringForIndex(indexer, VarInt.getVarInt(source)); - PathFragment materializationExecPath = null; int numMaterializationExecPath = VarInt.getVarInt(source); if (numMaterializationExecPath > 0) { @@ -503,8 +499,7 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))); } - return RemoteFileArtifactValue.create( - digest, size, locationIndex, actionId, materializationExecPath); + return RemoteFileArtifactValue.create(digest, size, locationIndex, materializationExecPath); } /** @return action data encoded as a byte[] array. */ 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 4d59c2d73b75b0..311db88ee0f6fd 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 @@ -114,12 +114,11 @@ public void updateContext(MetadataInjector metadataInjector) { this.metadataInjector = metadataInjector; } - void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) - throws IOException { + void injectRemoteFile(PathFragment path, byte[] digest, long size) throws IOException { if (!isOutput(path)) { return; } - remoteOutputTree.injectRemoteFile(path, digest, size, actionId); + remoteOutputTree.injectRemoteFile(path, digest, size); } void flush() throws IOException { @@ -206,7 +205,6 @@ private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact outpu metadata.getDigest(), metadata.getSize(), metadata.getLocationIndex(), - metadata.getActionId(), // Avoid a double indirection when the target is already materialized as a symlink. metadata.getMaterializationExecPath().orElse(targetPath.relativeTo(execRoot))); @@ -216,10 +214,7 @@ private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact outpu private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) { return RemoteFileArtifactValue.create( - remoteFile.getFastDigest(), - remoteFile.getSize(), - /* locationIndex= */ 1, - remoteFile.getActionId()); + remoteFile.getFastDigest(), remoteFile.getSize(), /* locationIndex= */ 1); } @Override @@ -741,8 +736,7 @@ protected FileInfo newFile(Clock clock, PathFragment path) { return new RemoteFileInfo(clock); } - void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) - throws IOException { + void injectRemoteFile(PathFragment path, byte[] digest, long size) 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 @@ -752,7 +746,7 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, String action } RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node; - remoteFileInfo.set(digest, size, actionId); + remoteFileInfo.set(digest, size); } @Nullable @@ -769,16 +763,14 @@ 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) { + private void set(byte[] digest, long size) { this.digest = digest; this.size = size; - this.actionId = actionId; } @Override @@ -805,9 +797,5 @@ public byte[] getFastDigest() { public long getSize() { return size; } - - public String getActionId() { - return actionId; - } } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index ff625eb68e925e..c4300b4309cd1b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -91,7 +91,7 @@ protected ListenableFuture doDownloadFile( throws IOException { checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file."); RequestMetadata requestMetadata = - TracingMetadataUtils.buildMetadata(buildRequestId, commandId, metadata.getActionId(), null); + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", null); RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); 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 b73fbc85216eb1..44a5d25dab1825 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 @@ -793,7 +793,6 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem(); checkState(actionFileSystem instanceof RemoteActionFileSystem); - RemoteActionExecutionContext context = action.getRemoteActionExecutionContext(); RemoteActionFileSystem remoteActionFileSystem = (RemoteActionFileSystem) actionFileSystem; for (Map.Entry entry : metadata.directories()) { @@ -808,8 +807,7 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met remoteActionFileSystem.injectRemoteFile( file.path().asFragment(), DigestUtil.toBinaryDigest(file.digest()), - file.digest().getSizeBytes(), - context.getRequestMetadata().getActionId()); + file.digest().getSizeBytes()); } } @@ -817,8 +815,7 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met remoteActionFileSystem.injectRemoteFile( file.path().asFragment(), DigestUtil.toBinaryDigest(file.digest()), - file.digest().getSizeBytes(), - context.getRequestMetadata().getActionId()); + file.digest().getSizeBytes()); } } diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index ee8f3d7c4685df..7797cf67ccdd40 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java @@ -463,8 +463,7 @@ private RemoteFileArtifactValue createRemoteFileMetadata(String content) { private RemoteFileArtifactValue createRemoteFileMetadata( String content, @Nullable PathFragment materializationExecPath) { byte[] bytes = content.getBytes(UTF_8); - return RemoteFileArtifactValue.create( - digest(bytes), bytes.length, 1, "action-id", materializationExecPath); + return RemoteFileArtifactValue.create(digest(bytes), bytes.length, 1, materializationExecPath); } private static TreeArtifactValue createTreeMetadata( diff --git a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java index f8f17739d72ce0..c9f6d59a241452 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java @@ -208,8 +208,7 @@ private RemoteFileArtifactValue createRemoteMetadata( .getHashFunction() .hashBytes(bytes) .asBytes(); - return RemoteFileArtifactValue.create( - digest, bytes.length, 1, "action-id", materializationExecPath); + return RemoteFileArtifactValue.create(digest, bytes.length, 1, materializationExecPath); } private RemoteFileArtifactValue createRemoteMetadata(Artifact artifact, String content) { diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 912c269d6e9a3f..ea7b7442c4982d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -100,7 +100,6 @@ protected Artifact createRemoteArtifact( hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1, - "action-id", materializationExecPath); metadata.put(a, f); cas.put(hashCode, contentsBytes); @@ -134,7 +133,7 @@ protected Pair> createRemoteTre TreeFileArtifact.createTreeOutput(parent, PathFragment.create(entry.getKey())); RemoteFileArtifactValue childValue = RemoteFileArtifactValue.create( - hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1, "action-id"); + hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1); treeBuilder.putChild(child, childValue); metadata.put(child, childValue); cas.put(hashCode, contentsBytes); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index 15b5e2e715e922..bd9c625110864a 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -442,7 +442,7 @@ private Artifact createRemoteArtifact( byte[] b = contents.getBytes(StandardCharsets.UTF_8); HashCode h = HashCode.fromString(DIGEST_UTIL.compute(b).getHash()); FileArtifactValue f = - RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id"); + RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1); inputs.putWithNoDepOwner(a, f); return a; } @@ -513,8 +513,6 @@ public ListenableFuture> findMissingDigests( private static class AllMissingDigestsFinder implements MissingDigestsFinder { - public static final AllMissingDigestsFinder INSTANCE = new AllMissingDigestsFinder(); - @Override public ListenableFuture> findMissingDigests( RemoteActionExecutionContext context, Iterable digests) { 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 487fe3404f1545..684b3b0f070eed 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 @@ -325,7 +325,7 @@ protected void injectRemoteFile(FileSystem actionFs, PathFragment path, String c byte[] contentBytes = content.getBytes(StandardCharsets.UTF_8); HashCode hashCode = HASH_FUNCTION.getHashFunction().hashBytes(contentBytes); ((RemoteActionFileSystem) actionFs) - .injectRemoteFile(path, hashCode.asBytes(), contentBytes.length, "action-id"); + .injectRemoteFile(path, hashCode.asBytes(), contentBytes.length); } @Override @@ -341,7 +341,7 @@ private Artifact createRemoteArtifact( byte[] b = contents.getBytes(StandardCharsets.UTF_8); HashCode h = HASH_FUNCTION.getHashFunction().hashBytes(b); RemoteFileArtifactValue f = - RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id"); + RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1); inputs.putWithNoDepOwner(a, f); return a; } @@ -363,8 +363,7 @@ private TreeArtifactValue createRemoteTreeArtifactValue( byte[] b = entry.getValue().getBytes(StandardCharsets.UTF_8); HashCode h = HASH_FUNCTION.getHashFunction().hashBytes(b); RemoteFileArtifactValue childMeta = - RemoteFileArtifactValue.create( - h.asBytes(), b.length, /* locationIndex= */ 0, "action-id"); + RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 0); builder.putChild(child, childMeta); } return builder.build(); 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 b8dce08dcd183a..3a5f3fc8e512b8 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 @@ -963,8 +963,9 @@ public void downloadOutputs_outputFilesWithoutTopLevel_inject() throws Exception Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), - eq(d1.getSizeBytes()), any()); + eq(execRoot.asFragment().getRelative(a1.getExecPath())), + eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1002,12 +1003,14 @@ public void downloadOutputs_outputFilesWithMinimal_injectingMetadata() throws Ex Artifact a2 = ActionsTestUtil.createArtifact(artifactRoot, "file2"); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), - eq(d1.getSizeBytes()), any()); + eq(execRoot.asFragment().getRelative(a1.getExecPath())), + eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes())); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a2.getExecPath())), eq(toBinaryDigest(d2)), - eq(d2.getSizeBytes()), any()); + eq(execRoot.asFragment().getRelative(a2.getExecPath())), + eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1058,12 +1061,14 @@ public void downloadOutputs_outputDirectoriesWithMinimal_injectingMetadata() thr assertThat(inMemoryOutput).isNull(); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative("outputs/dir/file1")), eq(toBinaryDigest(d1)), - eq(d1.getSizeBytes()), eq(action.getActionId())); + eq(execRoot.asFragment().getRelative("outputs/dir/file1")), + eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes())); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative("outputs/dir/a/file2")), eq(toBinaryDigest(d2)), - eq(d2.getSizeBytes()), eq(action.getActionId())); + eq(execRoot.asFragment().getRelative("outputs/dir/a/file2")), + eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1194,12 +1199,14 @@ public void downloadOutputs_inMemoryOutputWithMinimal_downloadIt() throws Except // The in memory file also needs to be injected as an output verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), - eq(d1.getSizeBytes()), eq(action.getActionId())); + eq(execRoot.asFragment().getRelative(a1.getExecPath())), + eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes())); verify(actionFileSystem) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a2.getExecPath())), eq(toBinaryDigest(d2)), - eq(d2.getSizeBytes()), eq(action.getActionId())); + eq(execRoot.asFragment().getRelative(a2.getExecPath())), + eq(toBinaryDigest(d2)), + eq(d2.getSizeBytes())); Path outputBase = checkNotNull(artifactRoot.getRoot().asPath()); assertThat(outputBase.readdir(Symlinks.NOFOLLOW)).isEmpty(); assertThat(context.isLockOutputFilesCalled()).isTrue(); @@ -1219,15 +1226,15 @@ public void downloadOutputs_missingInMemoryOutputWithMinimal_returnsNull() throw Spawn spawn = new SimpleSpawn( new FakeOwner("foo", "bar", "//dummy:label"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(REMOTE_EXECUTION_INLINE_OUTPUTS, "outputs/file1"), - /*runfilesSupplier=*/ null, - /*filesetMappings=*/ ImmutableMap.of(), - /*inputs=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /*tools=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /*outputs=*/ ImmutableSet.of(a1), - /*mandatoryOutputs=*/ ImmutableSet.of(), + /* arguments= */ ImmutableList.of(), + /* environment= */ ImmutableMap.of(), + /* executionInfo= */ ImmutableMap.of(REMOTE_EXECUTION_INLINE_OUTPUTS, "outputs/file1"), + /* runfilesSupplier= */ null, + /* filesetMappings= */ ImmutableMap.of(), + /* inputs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), + /* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), + /* outputs= */ ImmutableSet.of(a1), + /* mandatoryOutputs= */ ImmutableSet.of(), ResourceSet.ZERO); RemoteActionFileSystem actionFileSystem = mock(RemoteActionFileSystem.class); @@ -1245,8 +1252,9 @@ public void downloadOutputs_missingInMemoryOutputWithMinimal_returnsNull() throw // The in memory file metadata also should not have been injected. verify(actionFileSystem, never()) .injectRemoteFile( - eq(execRoot.asFragment().getRelative(a1.getExecPath())), eq(toBinaryDigest(d1)), - eq(d1.getSizeBytes()), eq(action.getActionId())); + eq(execRoot.asFragment().getRelative(a1.getExecPath())), + eq(toBinaryDigest(d1)), + eq(d1.getSizeBytes())); } @Test @@ -1623,7 +1631,7 @@ public void uploadInputsIfNotPresent_deduplicateFindMissingBlobCalls() throws Ex executorService.execute( () -> { try { - service.uploadInputsIfNotPresent(action, /*force=*/ false); + service.uploadInputsIfNotPresent(action, /* force= */ false); } catch (Throwable e) { if (e instanceof InterruptedException) { Thread.currentThread().interrupt(); @@ -1669,7 +1677,7 @@ public void uploadInputsIfNotPresent_sameInputs_interruptOne_keepOthers() throws if (shouldInterrupt) { Thread.currentThread().interrupt(); } - service.uploadInputsIfNotPresent(action, /*force=*/ false); + service.uploadInputsIfNotPresent(action, /* force= */ false); } catch (Throwable e) { if (!(shouldInterrupt && e instanceof InterruptedException)) { error.set(e); @@ -1782,20 +1790,20 @@ public void buildMerkleTree_withMemoization_works() throws Exception { Spawn spawn1 = new SimpleSpawn( new FakeOwner("foo", "bar", "//dummy:label"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ nodeRoot1, - /*outputs=*/ ImmutableSet.of(), + /* arguments= */ ImmutableList.of(), + /* environment= */ ImmutableMap.of(), + /* executionInfo= */ ImmutableMap.of(), + /* inputs= */ nodeRoot1, + /* outputs= */ ImmutableSet.of(), ResourceSet.ZERO); Spawn spawn2 = new SimpleSpawn( new FakeOwner("foo", "bar", "//dummy:label"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ nodeRoot2, - /*outputs=*/ ImmutableSet.of(), + /* arguments= */ ImmutableList.of(), + /* environment= */ ImmutableMap.of(), + /* executionInfo= */ ImmutableMap.of(), + /* inputs= */ nodeRoot2, + /* outputs= */ ImmutableSet.of(), ResourceSet.ZERO); FakeSpawnExecutionContext context1 = newSpawnExecutionContext(spawn1); @@ -1953,11 +1961,11 @@ private Spawn newSpawn( NestedSet inputs) { return new SimpleSpawn( new FakeOwner("foo", "bar", "//dummy:label"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ executionInfo, - /*inputs=*/ inputs, - /*outputs=*/ outputs, + /* arguments= */ ImmutableList.of(), + /* environment= */ ImmutableMap.of(), + /* executionInfo= */ executionInfo, + /* inputs= */ inputs, + /* outputs= */ outputs, ResourceSet.ZERO); } @@ -1983,7 +1991,7 @@ private RemoteExecutionService newRemoteExecutionService(RemoteOptions remoteOpt return new RemoteExecutionService( directExecutor(), reporter, - /*verboseFailures=*/ true, + /* verboseFailures= */ true, execRoot, remotePathResolver, "none", 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 ebc04c3ba3149c..99e39cb1825ae0 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 @@ -112,12 +112,13 @@ private ActionMetadataHandler createHandler( public void withNonArtifactInput() throws Exception { ActionInput input = ActionInputHelper.fromPath("foo/bar"); FileArtifactValue metadata = - FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /*proxy=*/ null, /*size=*/ 10L); + FileArtifactValue.createForNormalFile( + new byte[] {1, 2, 3}, /* proxy= */ null, /* size= */ 10L); ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(input, metadata); assertThat(map.getMetadata(input)).isEqualTo(metadata); ActionMetadataHandler handler = - createHandler(map, /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + createHandler(map, /* forInputDiscovery= */ false, /* outputs= */ ImmutableSet.of()); assertThat(handler.getMetadata(input)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -127,11 +128,12 @@ public void withArtifactInput() throws Exception { PathFragment path = PathFragment.create("src/a"); Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path); FileArtifactValue metadata = - FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /*proxy=*/ null, /*size=*/ 10L); + FileArtifactValue.createForNormalFile( + new byte[] {1, 2, 3}, /* proxy= */ null, /* size= */ 10L); ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(artifact, metadata); ActionMetadataHandler handler = - createHandler(map, /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + createHandler(map, /* forInputDiscovery= */ false, /* outputs= */ ImmutableSet.of()); assertThat(handler.getMetadata(artifact)).isEqualTo(metadata); assertThat(chmodCalls).isEmpty(); } @@ -142,7 +144,9 @@ public void unknownSourceArtifactDisallowedOutsideOfInputDiscovery() { Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of()); Exception e = assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); assertThat(e).hasMessageThat().contains(artifact + " is not present in declared outputs"); assertThat(chmodCalls).isEmpty(); @@ -154,7 +158,7 @@ public void unknownSourceArtifactPermittedDuringInputDiscovery() throws Exceptio Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ true, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); assertThat(handler.getMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -165,7 +169,7 @@ public void unknownArtifactPermittedDuringInputDiscovery() throws Exception { Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(outputRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ true, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); assertThat(handler.getMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -178,8 +182,8 @@ public void withKnownOutputArtifactStatsFile() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(artifact)); assertThat(handler.getMetadata(artifact)).isNotNull(); assertThat(chmodCalls).isEmpty(); } @@ -191,8 +195,8 @@ public void withMissingOutputArtifactStatsFileFailsWithException() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(artifact)); assertThrows(FileNotFoundException.class, () -> handler.getMetadata(artifact)); assertThat(chmodCalls).isEmpty(); } @@ -203,7 +207,9 @@ public void unknownArtifactDisallowedOutsideOfInputDiscovery() { Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(outputRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of()); assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); assertThat(chmodCalls).isEmpty(); } @@ -216,7 +222,7 @@ public void unknownTreeArtifactPermittedDuringInputDiscovery() throws Exception Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz"); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ true, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); assertThat(handler.getMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -232,8 +238,8 @@ public void withUnknownOutputArtifactStatsFileTreeArtifact() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); assertThat(handler.getMetadata(artifact)).isNotNull(); assertThat(chmodCalls).isEmpty(); } @@ -246,7 +252,9 @@ public void unknownTreeArtifactDisallowedOutsideOfInputDiscovery() { Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz"); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of()); assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); assertThat(chmodCalls).isEmpty(); } @@ -266,8 +274,8 @@ public void createsTreeArtifactValueFromFilesystem() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); FileArtifactValue treeMetadata = handler.getMetadata(treeArtifact); FileArtifactValue child1Metadata = handler.getMetadata(child1); @@ -290,8 +298,8 @@ public void resettingOutputs() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ true, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ true, + /* outputs= */ ImmutableSet.of(artifact)); handler.prepareForActionExecution(); // The handler doesn't have any info. It'll stat the file and discover that it's 10 bytes long. @@ -299,8 +307,7 @@ public void resettingOutputs() throws Exception { assertThat(chmodCalls).containsExactly(outputPath, 0555); // Inject a remote file of size 42. - handler.injectFile( - artifact, RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 42, 0, "ultimate-answer")); + handler.injectFile(artifact, RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 42, 0)); assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(42); // Reset this output, which will make the handler stat the file again. @@ -317,14 +324,14 @@ public void injectRemoteArtifactMetadata() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ true, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ true, + /* outputs= */ ImmutableSet.of(artifact)); handler.prepareForActionExecution(); byte[] digest = new byte[] {1, 2, 3}; int size = 10; handler.injectFile( - artifact, RemoteFileArtifactValue.create(digest, size, /*locationIndex=*/ 1, "action-id")); + artifact, RemoteFileArtifactValue.create(digest, size, /* locationIndex= */ 1)); FileArtifactValue v = handler.getMetadata(artifact); assertThat(v).isNotNull(); @@ -343,8 +350,8 @@ public void cannotInjectTreeArtifactChildIndividually() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); RemoteFileArtifactValue childValue = RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1); @@ -367,8 +374,8 @@ public void canInjectTemplateExpansionOutput() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); RemoteFileArtifactValue value = RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1); @@ -387,18 +394,18 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); TreeArtifactValue tree = TreeArtifactValue.newBuilder(treeArtifact) .putChild( TreeFileArtifact.createTreeOutput(treeArtifact, "foo"), - RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1, "foo")) + RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1)) .putChild( TreeFileArtifact.createTreeOutput(treeArtifact, "bar"), - RemoteFileArtifactValue.create(new byte[] {4, 5, 6}, 10, 1, "bar")) + RemoteFileArtifactValue.create(new byte[] {4, 5, 6}, 10, 1)) .build(); handler.injectTree(treeArtifact, tree); @@ -415,7 +422,7 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { // child is missing, getExistingFileArtifactValue will throw. ActionExecutionValue actionExecutionValue = ActionExecutionValue.createFromOutputStore( - handler.getOutputStore(), /*outputSymlinks=*/ null, new NullAction()); + handler.getOutputStore(), /* outputSymlinks= */ null, new NullAction()); tree.getChildren().forEach(actionExecutionValue::getExistingFileArtifactValue); } @@ -465,7 +472,7 @@ private FilesetOutputSymlink createFilesetOutputSymlink(HasDigest digest, String PathFragment.create(identifier + "_symlink"), PathFragment.create(identifier), digest, - /*isGeneratedTarget=*/ true, + /* isGeneratedTarget= */ true, outputRoot.getExecPath()); } @@ -484,8 +491,8 @@ public void omitRegularArtifact() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(omitted, consumed)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(omitted, consumed)); handler.prepareForActionExecution(); handler.markOmitted(omitted); @@ -509,8 +516,8 @@ public void omitTreeArtifact() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(omittedTree, consumedTree)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(omittedTree, consumedTree)); handler.prepareForActionExecution(); handler.markOmitted(omittedTree); @@ -533,8 +540,8 @@ public void outputArtifactNotPreviouslyInjectedInExecutionMode() throws Exceptio ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(output)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(output)); handler.prepareForActionExecution(); FileArtifactValue metadata = handler.getMetadata(output); @@ -588,8 +595,8 @@ public void outputTreeArtifactNotPreviouslyInjectedInExecutionMode() throws Exce ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); FileArtifactValue treeMetadata = handler.getMetadata(treeArtifact); @@ -625,8 +632,8 @@ public void transformAfterInputDiscovery() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ true, - /*outputs=*/ ImmutableSet.of(known)); + /* forInputDiscovery= */ true, + /* outputs= */ ImmutableSet.of(known)); // Unknown artifact returns null during input discovery. assertThat(handler.getMetadata(unknown)).isNull(); @@ -654,8 +661,8 @@ public void getTreeArtifactChildren_noData_returnsEmptySet() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(treeArtifact)); assertThat(handler.getTreeArtifactChildren(treeArtifact)).isEmpty(); } @@ -673,8 +680,8 @@ public void enteringExecutionModeClearsCachedOutputs() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(artifact, treeArtifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(artifact, treeArtifact)); OutputStore store = handler.getOutputStore(); FileArtifactValue artifactMetadata1 = handler.getMetadata(artifact); @@ -704,7 +711,9 @@ public void enteringExecutionModeClearsCachedOutputs() throws Exception { public void cannotEnterExecutionModeTwice() { ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /*forInputDiscovery=*/ false, /*outputs=*/ ImmutableSet.of()); + new ActionInputMap(0), + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of()); handler.prepareForActionExecution(); assertThrows(IllegalStateException.class, handler::prepareForActionExecution); } @@ -718,8 +727,8 @@ public void fileArtifactValueFromArtifactCompatibleWithGetMetadata_changed() thr ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(artifact)); FileArtifactValue getMetadataResult = handler.getMetadata(artifact); assertThat(getMetadataResult).isNotNull(); @@ -727,7 +736,7 @@ public void fileArtifactValueFromArtifactCompatibleWithGetMetadata_changed() thr scratch.overwriteFile(artifact.getPath().getPathString(), "2"); FileArtifactValue fileArtifactValueFromArtifactResult = ActionMetadataHandler.fileArtifactValueFromArtifact( - artifact, /*statNoFollow=*/ null, SyscallCache.NO_CACHE, /*tsgm=*/ null); + artifact, /* statNoFollow= */ null, SyscallCache.NO_CACHE, /* tsgm= */ null); assertThat(fileArtifactValueFromArtifactResult).isNotNull(); assertThat(fileArtifactValueFromArtifactResult.couldBeModifiedSince(getMetadataResult)) @@ -743,15 +752,15 @@ public void fileArtifactValueFromArtifactCompatibleWithGetMetadata_notChanged() ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /*forInputDiscovery=*/ false, - /*outputs=*/ ImmutableSet.of(artifact)); + /* forInputDiscovery= */ false, + /* outputs= */ ImmutableSet.of(artifact)); FileArtifactValue getMetadataResult = handler.getMetadata(artifact); assertThat(getMetadataResult).isNotNull(); FileArtifactValue fileArtifactValueFromArtifactResult = ActionMetadataHandler.fileArtifactValueFromArtifact( - artifact, /*statNoFollow=*/ null, SyscallCache.NO_CACHE, /*tsgm=*/ null); + artifact, /* statNoFollow= */ null, SyscallCache.NO_CACHE, /* tsgm= */ null); assertThat(fileArtifactValueFromArtifactResult).isNotNull(); assertThat(fileArtifactValueFromArtifactResult.couldBeModifiedSince(getMetadataResult)) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index 598e504c309221..3c2ba720e885a2 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -1327,7 +1327,7 @@ private RemoteFileArtifactValue createRemoteFileArtifactValue(String contents) { byte[] data = contents.getBytes(); DigestHashFunction hashFn = fs.getDigestFunction(); HashCode hash = hashFn.getHashFunction().hashBytes(data); - return RemoteFileArtifactValue.create(hash.asBytes(), data.length, -1, "action-id"); + return RemoteFileArtifactValue.create(hash.asBytes(), data.length, -1); } @Test