Clean up FileArtifactValue a little:
- Rename the overloads of create() to something more indicative of their purpose
- Remove the use of ArtifactPathResolver by calling it from the single call site
- Assert that createForSourceArtifact() is only called for proper source artifacts
RELNOTES: None.
PiperOrigin-RevId: 259945500
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 1362c71..b9db313 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
@@ -180,8 +180,10 @@
}
}
- public static FileArtifactValue create(Artifact artifact, FileValue fileValue)
+ public static FileArtifactValue createForSourceArtifact(Artifact artifact, FileValue fileValue)
throws IOException {
+ Preconditions.checkState(artifact.isSourceArtifact());
+ Preconditions.checkState(!artifact.isConstantMetadata());
boolean isFile = fileValue.isFile();
FileContentsProxy proxy = getProxyFromFileStateValue(fileValue.realFileStateValue());
return create(
@@ -190,10 +192,11 @@
isFile ? fileValue.getSize() : 0,
proxy,
isFile ? fileValue.getDigest() : null,
- !artifact.isConstantMetadata());
+ /* isShareable=*/ true);
}
- public static FileArtifactValue create(Artifact artifact, ArtifactFileMetadata metadata)
+ @VisibleForTesting
+ public static FileArtifactValue createForTesting(Artifact artifact, ArtifactFileMetadata metadata)
throws IOException {
boolean isFile = metadata.isFile();
FileContentsProxy proxy = getProxyFromFileStateValue(metadata.realFileStateValue());
@@ -206,40 +209,30 @@
!artifact.isConstantMetadata());
}
- public static FileArtifactValue create(
- Artifact artifact,
- ArtifactPathResolver resolver,
- ArtifactFileMetadata fileValue,
- @Nullable byte[] injectedDigest)
- throws IOException {
- boolean isFile = fileValue.isFile();
- FileContentsProxy proxy = getProxyFromFileStateValue(fileValue.realFileStateValue());
- return create(
- resolver.toPath(artifact),
- isFile,
- isFile ? fileValue.getSize() : 0,
- proxy,
- injectedDigest,
- !artifact.isConstantMetadata());
+ public static FileArtifactValue createFromInjectedDigest(
+ ArtifactFileMetadata metadata, @Nullable byte[] digest, boolean isShareable) {
+ FileContentsProxy proxy = getProxyFromFileStateValue(metadata.realFileStateValue());
+ return createForNormalFile(digest, proxy, metadata.getSize(), isShareable);
}
@VisibleForTesting
- public static FileArtifactValue create(Artifact artifact) throws IOException {
- return create(artifact.getPath(), !artifact.isConstantMetadata());
+ public static FileArtifactValue createForTesting(Artifact artifact) throws IOException {
+ return createFromFileSystem(artifact.getPath(), !artifact.isConstantMetadata());
}
- public static FileArtifactValue createShareable(Path path) throws IOException {
- return create(path, /*isShareable=*/ true);
+ public static FileArtifactValue createFromFileSystem(Path path) throws IOException {
+ return createFromFileSystem(path, /*isShareable=*/ true);
}
- public static FileArtifactValue create(Path path, boolean isShareable) throws IOException {
+ public static FileArtifactValue createFromFileSystem(Path path, boolean isShareable)
+ throws IOException {
// Caution: there's a race condition between stating the file and computing the
// digest. We need to stat first, since we're using the stat to detect changes.
// We follow symlinks here to be consistent with getDigest.
- return create(path, path.stat(Symlinks.FOLLOW), isShareable);
+ return createFromStat(path, path.stat(Symlinks.FOLLOW), isShareable);
}
- public static FileArtifactValue create(Path path, FileStatus stat, boolean isShareable)
+ public static FileArtifactValue createFromStat(Path path, FileStatus stat, boolean isShareable)
throws IOException {
return create(
path, stat.isFile(), stat.getSize(), FileContentsProxy.create(stat), null, isShareable);
@@ -263,7 +256,7 @@
digest = DigestUtils.getDigestOrFail(path, size);
}
Preconditions.checkState(digest != null, path);
- return createNormalFile(digest, proxy, size, isShareable);
+ return createForNormalFile(digest, proxy, size, isShareable);
}
public static FileArtifactValue createForVirtualActionInput(byte[] digest, long size) {
@@ -271,25 +264,25 @@
}
@VisibleForTesting
- public static FileArtifactValue createNormalFile(
+ public static FileArtifactValue createForNormalFile(
byte[] digest, @Nullable FileContentsProxy proxy, long size, boolean isShareable) {
return isShareable
? new RegularFileArtifactValue(digest, proxy, size)
: new UnshareableRegularFileArtifactValue(digest, proxy, size);
}
- public static FileArtifactValue createNormalFile(
+ public static FileArtifactValue createFromMetadata(
ArtifactFileMetadata artifactMetadata, boolean isShareable) {
FileContentsProxy proxy = getProxyFromFileStateValue(artifactMetadata.realFileStateValue());
- return createNormalFile(
+ return createForNormalFile(
artifactMetadata.getDigest(), proxy, artifactMetadata.getSize(), isShareable);
}
- public static FileArtifactValue createDirectoryWithHash(byte[] digest) {
+ public static FileArtifactValue createForDirectoryWithHash(byte[] digest) {
return new HashedDirectoryArtifactValue(digest);
}
- public static FileArtifactValue createDirectory(long mtime) {
+ public static FileArtifactValue createForDirectoryWithMtime(long mtime) {
return new DirectoryArtifactValue(mtime);
}
@@ -299,7 +292,7 @@
*/
public static FileArtifactValue createProxy(byte[] digest) {
Preconditions.checkNotNull(digest);
- return createNormalFile(digest, /*proxy=*/ null, /*size=*/ 0, /*isShareable=*/ true);
+ return createForNormalFile(digest, /*proxy=*/ null, /*size=*/ 0, /*isShareable=*/ true);
}
private static final class DirectoryArtifactValue extends FileArtifactValue {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
index 4f60575..900e9a3 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
@@ -62,7 +62,7 @@
() -> {
Path path = ActionInputHelper.toInputPath(input, execRoot);
try {
- FileArtifactValue metadata = FileArtifactValue.createShareable(path);
+ FileArtifactValue metadata = FileArtifactValue.createFromFileSystem(path);
if (metadata.getType().isDirectory()) {
throw new DigestOfDirectoryException(
"Input is a directory: " + input.getExecPathString());
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 e4fd0fb..1ec6de7 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
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateValue;
+import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.Md5Digest;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -231,7 +232,7 @@
if (!fileMetadata.exists()) {
throw new FileNotFoundException(artifact.prettyPrint() + " does not exist");
}
- return FileArtifactValue.createNormalFile(fileMetadata, !artifact.isConstantMetadata());
+ return FileArtifactValue.createFromMetadata(fileMetadata, !artifact.isConstantMetadata());
}
// No existing metadata; this can happen if the output metadata is not injected after a spawn
@@ -259,7 +260,6 @@
* See {@link OutputStore#getAllAdditionalOutputData} for why we sometimes need to store
* additional data, even for normal (non-middleman) artifacts.
*/
- @Nullable
private FileArtifactValue maybeStoreAdditionalData(
Artifact artifact, ArtifactFileMetadata data, @Nullable byte[] injectedDigest)
throws IOException {
@@ -271,17 +271,32 @@
if (isFile && !artifact.hasParent() && data.getDigest() != null) {
// We do not need to store the FileArtifactValue separately -- the digest is in the file value
// and that is all that is needed for this file's metadata.
- return FileArtifactValue.createNormalFile(data, !artifact.isConstantMetadata());
+ return FileArtifactValue.createFromMetadata(data, !artifact.isConstantMetadata());
}
- // Unfortunately, the ArtifactFileMetadata does not contain enough information for us to
- // calculate the corresponding FileArtifactValue -- either the metadata must use the modified
- // time, which we do not expose in the ArtifactFileMetadata, or the ArtifactFileMetadata didn't
- // store the digest So we store the metadata separately.
- // Use the ArtifactFileMetadata's digest if no digest was injected, or if the file can't be
- // digested.
- injectedDigest = injectedDigest != null || !isFile ? injectedDigest : data.getDigest();
- FileArtifactValue value = FileArtifactValue.create(artifact, artifactPathResolver, data,
- injectedDigest);
+
+ final FileArtifactValue value;
+
+ if (data.isDirectory()) {
+ value =
+ FileArtifactValue.createForDirectoryWithMtime(
+ artifactPathResolver.toPath(artifact).getLastModifiedTime());
+ } else {
+ // Unfortunately, the ArtifactFileMetadata does not contain enough information for us to
+ // calculate the corresponding FileArtifactValue -- either the metadata must use the modified
+ // time, which we do not expose in the ArtifactFileMetadata, or the ArtifactFileMetadata
+ // didn't store the digest So we store the metadata separately.
+ // Use the ArtifactFileMetadata's digest if no digest was injected, or if the file can't be
+ // digested.
+ if (injectedDigest == null) {
+ injectedDigest =
+ DigestUtils.getDigestOrFail(artifactPathResolver.toPath(artifact), data.getSize());
+ }
+
+ value =
+ FileArtifactValue.createFromInjectedDigest(
+ data, injectedDigest, !artifact.isConstantMetadata());
+ }
+
store.putAdditionalOutputData(artifact, value);
return metadataFromValue(value);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index e7a69f9..d66c150 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -266,10 +266,10 @@
fp.addString(file.getNameInSymlinkTree().getPathString());
fp.addBytes(file.getMetadata().getDigest());
}
- return FileArtifactValue.createDirectoryWithHash(fp.digestAndReset());
+ return FileArtifactValue.createForDirectoryWithHash(fp.digestAndReset());
}
try {
- return FileArtifactValue.create(artifact, fileValue);
+ return FileArtifactValue.createForSourceArtifact(artifact, fileValue);
} catch (IOException e) {
return makeMissingInputFileValue(artifact, e);
}
@@ -301,7 +301,7 @@
// Directories are special-cased because their mtimes are used, so should have been constructed
// during execution of the action (in ActionMetadataHandler#maybeStoreAdditionalData).
Preconditions.checkState(data.isFile(), "Unexpected not file %s (%s)", artifact, data);
- return FileArtifactValue.createNormalFile(data, !artifact.isConstantMetadata());
+ return FileArtifactValue.createFromMetadata(data, !artifact.isConstantMetadata());
}
@Nullable
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 39b49a2..42ac908 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
@@ -338,7 +338,7 @@
if (!(input instanceof Artifact)) {
return null;
}
- return FileArtifactValue.create((Artifact) input);
+ return FileArtifactValue.createForTesting((Artifact) input);
}
@Override
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 54b4d9d..76e05e8 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
@@ -144,7 +144,8 @@
ActionCache.Entry entry =
new ActionCache.Entry("actionKey", ImmutableMap.<String, String>of(), false);
entry.toString();
- entry.addFile(PathFragment.create("foo/bar"), FileArtifactValue.createDirectory(1234));
+ entry.addFile(
+ PathFragment.create("foo/bar"), FileArtifactValue.createForDirectoryWithMtime(1234));
entry.toString();
entry.getFileDigest();
entry.toString();
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index d2bfeea..90514ac 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -92,7 +92,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true));
expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER,
@@ -110,7 +110,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true));
ArtifactExpander filesetExpander =
@@ -148,7 +148,7 @@
Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles);
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
- mockCache.put(artifact, FileArtifactValue.createDirectory(-1));
+ mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1));
IOException expected =
assertThrows(
@@ -173,7 +173,7 @@
Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles);
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
- mockCache.put(artifact, FileArtifactValue.createDirectory(-1));
+ mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1));
expander = new SpawnInputExpander(execRoot, /*strict=*/ false);
expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER,
@@ -199,11 +199,11 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact1,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
mockCache.put(
artifact2,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 12L, /*isShareable=*/ true));
expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER,
@@ -229,7 +229,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER,
@@ -253,7 +253,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER,
@@ -287,8 +287,8 @@
};
RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles);
FakeActionInputFileCache fakeCache = new FakeActionInputFileCache();
- fakeCache.put(file1, FileArtifactValue.create(file1));
- fakeCache.put(file2, FileArtifactValue.create(file2));
+ fakeCache.put(file1, FileArtifactValue.createForTesting(file1));
+ fakeCache.put(file2, FileArtifactValue.createForTesting(file2));
expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander,
ArtifactPathResolver.IDENTITY, true);
@@ -322,8 +322,8 @@
};
RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles);
FakeActionInputFileCache fakeCache = new FakeActionInputFileCache();
- fakeCache.put(file1, FileArtifactValue.create(file1));
- fakeCache.put(file2, FileArtifactValue.create(file2));
+ fakeCache.put(file1, FileArtifactValue.createForTesting(file1));
+ fakeCache.put(file2, FileArtifactValue.createForTesting(file2));
expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander,
ArtifactPathResolver.IDENTITY, true);
@@ -352,8 +352,8 @@
}
};
FakeActionInputFileCache fakeCache = new FakeActionInputFileCache();
- fakeCache.put(file1, FileArtifactValue.create(file1));
- fakeCache.put(file2, FileArtifactValue.create(file2));
+ fakeCache.put(file1, FileArtifactValue.createForTesting(file1));
+ fakeCache.put(file2, FileArtifactValue.createForTesting(file2));
Spawn spawn = new SpawnBuilder("/bin/echo", "Hello World").withInput(treeArtifact).build();
inputMappings = expander.getInputMapping(spawn, artifactExpander, ArtifactPathResolver.IDENTITY,
diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
index a746131..76dfc49 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
@@ -46,7 +46,7 @@
String hexDigest = Preconditions.checkNotNull(cas.get(input), input);
Path path = execRoot.getRelative(input.getExecPath());
FileStatus stat = path.stat(Symlinks.FOLLOW);
- return FileArtifactValue.createNormalFile(
+ return FileArtifactValue.createForNormalFile(
HashCode.fromString(hexDigest).asBytes(),
FileContentsProxy.create(stat),
stat.getSize(),
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 1953e17..fd303e6 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
@@ -153,7 +153,8 @@
Path p = outputRoot.getRoot().asPath().getRelative(pathFragment);
FileSystemUtils.writeContent(p, StandardCharsets.UTF_8, contents);
Artifact a = ActionsTestUtil.createArtifact(outputRoot, p);
- inputs.putWithNoDepOwner(a, FileArtifactValue.create(a.getPath(), /* isShareable= */ true));
+ inputs.putWithNoDepOwner(
+ a, FileArtifactValue.createFromFileSystem(a.getPath(), /* isShareable= */ true));
return a;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
index 7aee9ce..d1e2db8 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
@@ -163,7 +163,7 @@
Path p = execRoot.getRelative(artifactRoot.getExecPath()).getRelative("file1");
FileSystemUtils.writeContent(p, StandardCharsets.UTF_8, "hello world");
Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p);
- FileArtifactValue f = FileArtifactValue.create(a);
+ FileArtifactValue f = FileArtifactValue.createForTesting(a);
MetadataProvider metadataProvider = new StaticMetadataProvider(ImmutableMap.of(a, f));
AbstractRemoteActionCache remoteCache =
new StaticRemoteActionCache(options, digestUtil, new HashMap<>());
diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/InputTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/InputTreeTest.java
index a8a143a..efe8653 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/InputTreeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/InputTreeTest.java
@@ -140,17 +140,17 @@
Path barPath = dirPath.getRelative("bar.cc");
FileSystemUtils.writeContentAsLatin1(barPath, "bar");
ActionInput bar = ActionInputHelper.fromPath(barPath.relativeTo(execRoot));
- metadata.put(bar, FileArtifactValue.createShareable(barPath));
+ metadata.put(bar, FileArtifactValue.createFromFileSystem(barPath));
dirPath.getRelative("fizz").createDirectoryAndParents();
Path buzzPath = dirPath.getRelative("fizz/buzz.cc");
FileSystemUtils.writeContentAsLatin1(dirPath.getRelative("fizz/buzz.cc"), "buzz");
ActionInput buzz = ActionInputHelper.fromPath(buzzPath.relativeTo(execRoot));
- metadata.put(buzz, FileArtifactValue.createShareable(buzzPath));
+ metadata.put(buzz, FileArtifactValue.createFromFileSystem(buzzPath));
Artifact dir = ActionsTestUtil.createArtifact(artifactRoot, dirPath);
sortedInputs.put(dirPath.relativeTo(execRoot), dir);
- metadata.put(dir, FileArtifactValue.createShareable(dirPath));
+ metadata.put(dir, FileArtifactValue.createFromFileSystem(dirPath));
InputTree tree =
InputTree.build(sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil);
@@ -206,7 +206,7 @@
Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p);
sortedInputs.put(PathFragment.create(path), a);
- metadata.put(a, FileArtifactValue.create(a));
+ metadata.put(a, FileArtifactValue.createForTesting(a));
return a;
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java
index 6f790dc..94ce5b5 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java
@@ -149,7 +149,7 @@
Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p);
sortedInputs.put(PathFragment.create(path), a);
- metadata.put(a, FileArtifactValue.create(a));
+ metadata.put(a, FileArtifactValue.createForTesting(a));
return a;
}
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 6b578cd..22cdf98 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
@@ -58,7 +58,7 @@
public void withNonArtifactInput() throws Exception {
ActionInput input = ActionInputHelper.fromPath("foo/bar");
FileArtifactValue metadata =
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true);
ActionInputMap map = new ActionInputMap(1);
map.putWithNoDepOwner(input, metadata);
@@ -78,7 +78,7 @@
PathFragment path = PathFragment.create("src/a");
Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path);
FileArtifactValue metadata =
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true);
ActionInputMap map = new ActionInputMap(1);
map.putWithNoDepOwner(artifact, metadata);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index 1ca6c08..91100ab 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -237,7 +237,8 @@
treeArtifact, childRelativePath);
scratch.file(treeFileArtifact.getPath().toString(), childRelativePath);
// We do not care about the FileArtifactValues in this test.
- treeFileArtifactMap.put(treeFileArtifact, FileArtifactValue.create(treeFileArtifact));
+ treeFileArtifactMap.put(
+ treeFileArtifact, FileArtifactValue.createForTesting(treeFileArtifact));
}
artifactValueMap.put(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index c41576f..07a020b 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -14,7 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.actions.FileArtifactValue.create;
+import static com.google.devtools.build.lib.actions.FileArtifactValue.createForTesting;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import com.google.common.collect.ImmutableList;
@@ -152,8 +152,8 @@
inputs.addAll(((AggregatingArtifactValue) value).getTreeArtifacts());
assertThat(inputs)
.containsExactly(
- Pair.of(input1, create(input1)),
- Pair.of(input2, create(input2)),
+ Pair.of(input1, createForTesting(input1)),
+ Pair.of(input2, createForTesting(input2)),
Pair.of(tree, ((TreeArtifactValue) evaluateArtifactValue(tree))));
}
@@ -273,7 +273,8 @@
writeFile(path, "contents");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
- ImmutableMap.of(treeFileArtifact, FileArtifactValue.create(treeFileArtifact)));
+ ImmutableMap.of(
+ treeFileArtifact, FileArtifactValue.createForTesting(treeFileArtifact)));
Artifact.DerivedArtifact artifact3 = createDerivedArtifact("three");
FilesetOutputSymlink filesetOutputSymlink =
FilesetOutputSymlink.createForTesting(
@@ -431,15 +432,17 @@
(SpecialArtifact) output, PathFragment.create("child1"));
TreeFileArtifact treeFileArtifact2 = ActionInputHelper.treeFileArtifact(
(SpecialArtifact) output, PathFragment.create("child2"));
- TreeArtifactValue treeArtifactValue = TreeArtifactValue.create(ImmutableMap.of(
- treeFileArtifact1, FileArtifactValue.create(treeFileArtifact1),
- treeFileArtifact2, FileArtifactValue.create(treeFileArtifact2)));
+ TreeArtifactValue treeArtifactValue =
+ TreeArtifactValue.create(
+ ImmutableMap.of(
+ treeFileArtifact1, FileArtifactValue.createForTesting(treeFileArtifact1),
+ treeFileArtifact2, FileArtifactValue.createForTesting(treeFileArtifact2)));
treeArtifactData.put(output, treeArtifactValue);
} else if (action.getActionType() == MiddlemanType.NORMAL) {
ArtifactFileMetadata fileValue =
ActionMetadataHandler.fileMetadataFromArtifact(output, null, null);
artifactData.put(output, fileValue);
- additionalOutputData.put(output, FileArtifactValue.create(output, fileValue));
+ additionalOutputData.put(output, FileArtifactValue.createForTesting(output, fileValue));
} else {
additionalOutputData.put(output, FileArtifactValue.DEFAULT_MIDDLEMAN);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
index f8da9cf..b1d49f6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
@@ -14,7 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.actions.FileArtifactValue.createShareable;
+import static com.google.devtools.build.lib.actions.FileArtifactValue.createFromFileSystem;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import com.google.common.io.BaseEncoding;
@@ -60,37 +60,38 @@
// and inequality with members of other equality groups.
new EqualsTester()
.addEqualityGroup(
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
toBytes("00112233445566778899AABBCCDDEEFF"),
/*proxy=*/ null,
1L,
/*isShareable=*/ true),
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
toBytes("00112233445566778899AABBCCDDEEFF"),
/*proxy=*/ null,
1L,
/*isShareable=*/ true))
.addEqualityGroup(
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
toBytes("00112233445566778899AABBCCDDEEFF"),
/*proxy=*/ null,
2L,
/*isShareable=*/ true))
- .addEqualityGroup(FileArtifactValue.createDirectory(1))
+ .addEqualityGroup(FileArtifactValue.createForDirectoryWithMtime(1))
.addEqualityGroup(
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
toBytes("FFFFFF00000000000000000000000000"),
/*proxy=*/ null,
1L,
/*isShareable=*/ true))
.addEqualityGroup(
- FileArtifactValue.createNormalFile(
+ FileArtifactValue.createForNormalFile(
toBytes("FFFFFF00000000000000000000000000"),
/*proxy=*/ null,
1L,
/*isShareable=*/ false))
.addEqualityGroup(
- FileArtifactValue.createDirectory(2), FileArtifactValue.createDirectory(2))
+ FileArtifactValue.createForDirectoryWithMtime(2),
+ FileArtifactValue.createForDirectoryWithMtime(2))
.addEqualityGroup(FileArtifactValue.OMITTED_FILE_MARKER)
.addEqualityGroup(FileArtifactValue.MISSING_FILE_MARKER)
.addEqualityGroup(FileArtifactValue.DEFAULT_MIDDLEMAN)
@@ -115,33 +116,33 @@
new EqualsTester()
// We check for ctime and inode equality for paths.
- .addEqualityGroup(createShareable(path1))
- .addEqualityGroup(createShareable(path2))
- .addEqualityGroup(createShareable(mtimePath))
- .addEqualityGroup(createShareable(digestPath))
- .addEqualityGroup(createShareable(empty1))
- .addEqualityGroup(createShareable(empty2))
- .addEqualityGroup(createShareable(empty3))
+ .addEqualityGroup(createFromFileSystem(path1))
+ .addEqualityGroup(createFromFileSystem(path2))
+ .addEqualityGroup(createFromFileSystem(mtimePath))
+ .addEqualityGroup(createFromFileSystem(digestPath))
+ .addEqualityGroup(createFromFileSystem(empty1))
+ .addEqualityGroup(createFromFileSystem(empty2))
+ .addEqualityGroup(createFromFileSystem(empty3))
// We check for mtime equality for directories.
- .addEqualityGroup(createShareable(dir1))
- .addEqualityGroup(createShareable(dir2), createShareable(dir3))
+ .addEqualityGroup(createFromFileSystem(dir1))
+ .addEqualityGroup(createFromFileSystem(dir2), createFromFileSystem(dir3))
.testEquals();
}
@Test
public void testCtimeInEquality() throws Exception {
Path path = scratchFile("/dir/artifact1", 0L, "content");
- FileArtifactValue before = createShareable(path);
+ FileArtifactValue before = createFromFileSystem(path);
clock.advanceMillis(1);
path.chmod(0777);
- FileArtifactValue after = createShareable(path);
+ FileArtifactValue after = createFromFileSystem(path);
assertThat(before).isNotEqualTo(after);
}
@Test
public void testNoMtimeIfNonemptyFile() throws Exception {
Path path = scratchFile("/root/non-empty", 1L, "abc");
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.getDigest()).isEqualTo(path.getDigest());
assertThat(value.getSize()).isEqualTo(3L);
assertThrows(
@@ -153,7 +154,7 @@
@Test
public void testDirectory() throws Exception {
Path path = scratchDir("/dir", /*mtime=*/ 1L);
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.getDigest()).isNull();
assertThat(value.getModifiedTime()).isEqualTo(1L);
}
@@ -163,7 +164,7 @@
public void testEmptyFile() throws Exception {
Path path = scratchFile("/root/empty", 1L, "");
path.setLastModifiedTime(1L);
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.getDigest()).isEqualTo(path.getDigest());
assertThat(value.getSize()).isEqualTo(0L);
assertThrows(
@@ -190,14 +191,14 @@
Path path = fs.getPath("/some/path");
path.getParentDirectory().createDirectoryAndParents();
FileSystemUtils.writeContentAsLatin1(path, "content");
- IOException e = assertThrows(IOException.class, () -> createShareable(path));
+ IOException e = assertThrows(IOException.class, () -> createFromFileSystem(path));
assertThat(e).isSameInstanceAs(exception);
}
@Test
public void testUptodateCheck() throws Exception {
Path path = scratchFile("/dir/artifact1", 0L, "content");
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
clock.advanceMillis(1);
assertThat(value.wasModifiedSinceDigest(path)).isFalse();
clock.advanceMillis(1);
@@ -212,7 +213,7 @@
@Test
public void testUptodateCheckDeleteFile() throws Exception {
Path path = scratchFile("/dir/artifact1", 0L, "content");
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.wasModifiedSinceDigest(path)).isFalse();
path.delete();
assertThat(value.wasModifiedSinceDigest(path)).isTrue();
@@ -222,7 +223,7 @@
public void testUptodateCheckDirectory() throws Exception {
// For now, we don't attempt to detect changes to directories.
Path path = scratchDir("/dir", 0L);
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.wasModifiedSinceDigest(path)).isFalse();
path.delete();
clock.advanceMillis(1);
@@ -233,7 +234,7 @@
public void testUptodateChangeFileToDirectory() throws Exception {
// For now, we don't attempt to detect changes to directories.
Path path = scratchFile("/dir/file", 0L, "");
- FileArtifactValue value = createShareable(path);
+ FileArtifactValue value = createFromFileSystem(path);
assertThat(value.wasModifiedSinceDigest(path)).isFalse();
// If we only check ctime, then we need to change the clock here, or we get a ctime match on the
// stat.
@@ -252,7 +253,7 @@
@Test
public void testIsMarkerValue_notMarker() throws Exception {
- FileArtifactValue value = createShareable(scratchFile("/dir/artifact1", 0L, "content"));
+ FileArtifactValue value = createFromFileSystem(scratchFile("/dir/artifact1", 0L, "content"));
assertThat(value.isMarkerValue()).isFalse();
}
}
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 846d515..55e4524 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
@@ -777,7 +777,7 @@
}
ArtifactFileMetadata fileValue =
ActionMetadataHandler.fileMetadataFromArtifact(output, null, null);
- dirDatum.put(output, FileArtifactValue.create(output, fileValue));
+ dirDatum.put(output, FileArtifactValue.createForTesting(output, fileValue));
fileData.put(output, fileValue);
} catch (IOException e) {
throw new IllegalStateException(e);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index 19d6973..ae2329e 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -1018,7 +1018,7 @@
&& ((Artifact) skyKey.argument()).isTreeArtifact()) {
return TreeArtifactValue.create(allTreeFiles);
}
- return FileArtifactValue.createShareable(((Artifact) skyKey.argument()).getPath());
+ return FileArtifactValue.createFromFileSystem(((Artifact) skyKey.argument()).getPath());
} catch (IOException e) {
throw new SkyFunctionException(e, Transience.PERSISTENT){};
}
@@ -1031,7 +1031,7 @@
}
public void addNewTreeFileArtifact(TreeFileArtifact input) throws IOException {
- allTreeFiles.put(input, FileArtifactValue.createShareable(input.getPath()));
+ allTreeFiles.put(input, FileArtifactValue.createFromFileSystem(input.getPath()));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index dfa5968..adb9788 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -111,7 +111,7 @@
Map<String, FileArtifactValue> digestBuilder = new HashMap<>();
for (PathFragment child : children) {
FileArtifactValue subdigest =
- FileArtifactValue.createShareable(tree.getPath().getRelative(child));
+ FileArtifactValue.createFromFileSystem(tree.getPath().getRelative(child));
digestBuilder.put(child.getPathString(), subdigest);
}
assertThat(DigestUtils.fromMetadata(digestBuilder).getDigestBytesUnsafe())
@@ -292,7 +292,7 @@
ArtifactFileMetadata fileValue =
ActionMetadataHandler.fileMetadataFromArtifact(suboutput, null, null);
fileData.put(suboutput, fileValue);
- treeArtifactData.put(suboutput, FileArtifactValue.create(suboutput, fileValue));
+ treeArtifactData.put(suboutput, FileArtifactValue.createForTesting(suboutput, fileValue));
} catch (IOException e) {
throw new SkyFunctionException(e, Transience.TRANSIENT) {};
}
