From 7760121d6a5690c4bd1aaa3cce0d383883469e67 Mon Sep 17 00:00:00 2001
From: Benjamin Peterson <benjamin@engflow.com>
Date: Wed, 27 Sep 2023 11:34:58 -0700
Subject: [PATCH] Check that digesting consumes the expected number of bytes.
In Bazel, the time a file is `stat`ed to find its size can be very far from the time the file's digest is computed, which creates a window for shenanigans. Allow passing the expected size into `Path.getDigest` and raise an error there if the number of digested bytes doesn't match the previously seen size.
---
.../build/lib/actions/FileArtifactValue.java | 2 +-
.../analysis/ConfiguredRuleClassProvider.java | 4 +-
.../build/lib/exec/RunfilesTreeUpdater.java | 4 +-
.../build/lib/exec/SpawnLogContext.java | 3 +-
.../lib/remote/RemoteActionFileSystem.java | 4 +-
.../build/lib/remote/util/DigestUtil.java | 3 +-
.../skyframe/ActionOutputMetadataStore.java | 2 +-
.../lib/testing/vfs/SpiedFileSystem.java | 4 +-
.../build/lib/unix/UnixFileSystem.java | 4 +-
.../devtools/build/lib/vfs/DigestUtils.java | 33 +++++++++++---
.../devtools/build/lib/vfs/FileSystem.java | 29 +++++++++---
.../build/lib/vfs/JavaIoFileSystem.java | 4 +-
.../google/devtools/build/lib/vfs/Path.java | 24 ++++++++--
.../PathTransformingDelegateFileSystem.java | 4 +-
.../analysis/StubbableFSBuildViewTest.java | 2 +-
.../lib/exec/SingleBuildFileCacheTest.java | 4 +-
.../lib/exec/SpawnLogContextTestBase.java | 4 +-
.../remote/RemoteActionFileSystemTest.java | 17 +++----
.../remote/RemoteExecutionServiceTest.java | 45 +++++++++++--------
.../lib/skyframe/ArtifactFunctionTest.java | 6 ++-
.../skyframe/ArtifactFunctionTestCase.java | 2 +-
.../lib/skyframe/FileArtifactValueTest.java | 2 +-
.../build/lib/skyframe/FileFunctionTest.java | 6 +--
.../unix/UnixDigestHashAttributeNameTest.java | 4 +-
.../build/lib/vfs/DigestUtilsTest.java | 35 ++++++++-------
.../build/lib/vfs/FileSystemTest.java | 5 ++-
26 files changed, 165 insertions(+), 91 deletions(-)
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 b40c6426b41e6c..f4aa37f4895263 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
@@ -273,7 +273,7 @@ private static FileArtifactValue create(
return new DirectoryArtifactValue(path.getLastModifiedTime());
}
if (digest == null) {
- digest = DigestUtils.getDigestWithManualFallback(path, xattrProvider);
+ digest = DigestUtils.getDigestWithManualFallback(path, size, xattrProvider);
}
Preconditions.checkState(digest != null, path);
return createForNormalFile(digest, proxy, size);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index e7e3ff8e3031ed..42c2a4f57b3bdd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -126,11 +126,11 @@ public BundledFileSystem() {
@Override
protected synchronized byte[] getFastDigest(PathFragment path) {
- return getDigest(path);
+ return getDigest(path, -1);
}
@Override
- protected synchronized byte[] getDigest(PathFragment path) {
+ protected synchronized byte[] getDigest(PathFragment path, long expectedSize) {
return getDigestFunction().getHashFunction().hashString(path.toString(), UTF_8).asBytes();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
index 33b73f430fec47..3694e65ec48bd1 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
@@ -138,8 +138,8 @@ private void updateRunfilesTree(
if (tree.getSymlinksMode() != SKIP
&& !outputManifest.isSymbolicLink()
&& Arrays.equals(
- DigestUtils.getDigestWithManualFallback(outputManifest, xattrProvider),
- DigestUtils.getDigestWithManualFallback(inputManifest, xattrProvider))
+ DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(outputManifest, xattrProvider),
+ DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(inputManifest, xattrProvider))
&& (OS.getCurrent() != OS.WINDOWS || isRunfilesDirectoryPopulated(runfilesDirPath))) {
return;
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java
index 30a5ba3277fde9..886c77c5626e89 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java
@@ -177,7 +177,8 @@ protected Digest computeDigest(
// Try to obtain a digest from the filesystem.
return builder
.setHash(
- HashCode.fromBytes(DigestUtils.getDigestWithManualFallback(path, xattrProvider))
+ HashCode.fromBytes(
+ DigestUtils.getDigestWithManualFallback(path, fileSize, xattrProvider))
.toString())
.setSizeBytes(fileSize)
.build();
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 4fd13a1abf9e71..f178fac495aa2d 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
@@ -434,7 +434,7 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
path = resolveSymbolicLinks(path).asFragment();
// Try to obtain a fast digest through a stat. This is only possible for in-memory files.
// The parent path has already been canonicalized by resolveSymbolicLinks, so FOLLOW_NONE is
@@ -443,7 +443,7 @@ protected byte[] getDigest(PathFragment path) throws IOException {
if (status instanceof FileStatusWithDigest) {
return ((FileStatusWithDigest) status).getDigest();
}
- return localFs.getPath(path).getDigest();
+ return localFs.getPath(path).getDigest(expectedSize);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java
index 781adf4abb0a42..271b3f534b782d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java
@@ -72,7 +72,8 @@ public Digest compute(Path file) throws IOException {
}
public Digest compute(Path file, long fileSize) throws IOException {
- return buildDigest(DigestUtils.getDigestWithManualFallback(file, xattrProvider), fileSize);
+ return buildDigest(
+ DigestUtils.getDigestWithManualFallback(file, fileSize, xattrProvider), fileSize);
}
public Digest compute(VirtualActionInput input) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
index 13697b03fcfc4d..a112964bac4d2f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
@@ -557,7 +557,7 @@ private FileArtifactValue constructFileArtifactValue(
// possible to hit the digest cache - we probably already computed the digest for the
// target during previous action execution.
Path pathToDigest = isResolvedSymlink ? statAndValue.realPath() : statAndValue.pathNoFollow();
- actualDigest = DigestUtils.manuallyComputeDigest(pathToDigest);
+ actualDigest = DigestUtils.manuallyComputeDigest(pathToDigest, value.getSize());
}
if (!isResolvedSymlink) {
diff --git a/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java b/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java
index 3487cf6f47118f..9eadd9623217f5 100644
--- a/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java
@@ -58,8 +58,8 @@ public boolean createWritableDirectory(PathFragment path) throws IOException {
}
@Override
- public byte[] getDigest(PathFragment path) throws IOException {
- return super.getDigest(path);
+ public byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
+ return super.getDigest(path, expectedSize);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
index 249fe92706bb78..75d2cd73731847 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -446,11 +446,11 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
- return super.getDigest(path);
+ return super.getDigest(path, expectedSize);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_MD5, name);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
index 9716dfe872a675..b4347ec2568a50 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
@@ -160,19 +160,40 @@ public static CacheStats getCacheStats() {
* #manuallyComputeDigest} to skip an additional attempt to obtain the fast digest.
*
* @param path Path of the file.
+ * @param fileSize Size of the file. Used to determine if digest calculation should be done
+ * serially or in parallel. Files larger than a certain threshold will be read serially, in
+ * order to avoid excessive disk seeks.
*/
- public static byte[] getDigestWithManualFallback(Path path, XattrProvider xattrProvider)
- throws IOException {
+ public static byte[] getDigestWithManualFallback(
+ Path path, long fileSize, XattrProvider xattrProvider) throws IOException {
byte[] digest = xattrProvider.getFastDigest(path);
- return digest != null ? digest : manuallyComputeDigest(path);
+ return digest != null ? digest : manuallyComputeDigest(path, fileSize);
+ }
+
+ /**
+ * Gets the digest of {@code path}, using a constant-time xattr call if the filesystem supports
+ * it, and calculating the digest manually otherwise.
+ *
+ * <p>Unlike {@link #getDigestWithManualFallback}, will not rate-limit manual digesting of files,
+ * so only use this method if the file size is truly unknown and you don't expect many concurrent
+ * manual digests of large files.
+ *
+ * @param path Path of the file.
+ */
+ public static byte[] getDigestWithManualFallbackWhenSizeUnknown(
+ Path path, XattrProvider xattrProvider) throws IOException {
+ return getDigestWithManualFallback(path, -1, xattrProvider);
}
/**
* Calculates the digest manually.
*
* @param path Path of the file.
+ * @param fileSize Size of the file. Used to determine if digest calculation should be done
+ * serially or in parallel. Files larger than a certain threshold will be read serially, in
+ * order to avoid excessive disk seeks.
*/
- public static byte[] manuallyComputeDigest(Path path) throws IOException {
+ public static byte[] manuallyComputeDigest(Path path, long fileSize) throws IOException {
byte[] digest;
// Attempt a cache lookup if the cache is enabled.
@@ -186,9 +207,9 @@ public static byte[] manuallyComputeDigest(Path path) throws IOException {
}
}
- digest = path.getDigest();
+ digest = path.getDigest(fileSize);
- Preconditions.checkNotNull(digest, "Missing digest for %s", path);
+ Preconditions.checkNotNull(digest, "Missing digest for %s (size %s)", path, fileSize);
if (cache != null) {
cache.put(key, digest);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index ce45f0020c4092..b6b7b8556aa01c 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -20,6 +20,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import com.google.common.hash.Funnels;
import com.google.common.io.ByteSource;
import com.google.common.io.CharStreams;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -33,6 +34,7 @@
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import javax.annotation.Nullable;
/** This interface models a file system. */
@@ -347,13 +349,26 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
* @return a new byte array containing the file's digest
* @throws IOException if the digest could not be computed for any reason
*/
- protected byte[] getDigest(PathFragment path) throws IOException {
- return new ByteSource() {
- @Override
- public InputStream openStream() throws IOException {
- return getInputStream(path);
- }
- }.hash(digestFunction.getHashFunction()).asBytes();
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
+ var bs =
+ new ByteSource() {
+ @Override
+ public InputStream openStream() throws IOException {
+ return getInputStream(path);
+ }
+ };
+ var hasher = digestFunction.getHashFunction().newHasher();
+ var copied = bs.copyTo(Funnels.asOutputStream(hasher));
+ if (expectedSize != -1 && copied != expectedSize) {
+ throw new IOException(
+ String.format(
+ Locale.US,
+ "digesting %s saw %s bytes rather than the expected %s",
+ path,
+ copied,
+ expectedSize));
+ }
+ return hasher.hash().asBytes();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
index 9311eeba1ed92b..387c53b29da17d 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
@@ -420,11 +420,11 @@ public void setLastModifiedTime(PathFragment path, long newTime) throws IOExcept
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
- return super.getDigest(path);
+ return super.getDigest(path, expectedSize);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_MD5, name);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index f0258243612bfa..b84c4b2f26af80 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -686,7 +686,23 @@ public byte[] getFastDigest() throws IOException {
* @throws IOException if the digest could not be computed for any reason
*/
public byte[] getDigest() throws IOException {
- return fileSystem.getDigest(asFragment());
+ return getDigest(-1);
+ }
+
+ /**
+ * Returns the digest of the file denoted by the current path, following symbolic links. Is not
+ * guaranteed to call {@link #getFastDigest} internally, even if a fast digest is likely
+ * available. Callers should prefer {@link DigestUtils#getDigestWithManualFallback} to this method
+ * unless they know that a fast digest is unavailable and do not need the other features
+ * (disk-read rate-limiting, global cache) that {@link DigestUtils} provides.
+ *
+ * @param expectedSize If not -1, the expected number of bytes to digest. If this does not match
+ * the number of bytes digested, an {@link IOException} may be raised.
+ * @return a new byte array containing the file's digest
+ * @throws IOException if the digest could not be computed for any reason
+ */
+ public byte[] getDigest(long expectedSize) throws IOException {
+ return fileSystem.getDigest(asFragment(), expectedSize);
}
/**
@@ -716,7 +732,8 @@ public String getDirectoryDigest(XattrProvider xattrProvider) throws IOException
} else {
hasher.putChar('-');
}
- hasher.putBytes(DigestUtils.getDigestWithManualFallback(path, xattrProvider));
+ hasher.putBytes(
+ DigestUtils.getDigestWithManualFallback(path, stat.getSize(), xattrProvider));
} else if (stat.isDirectory()) {
hasher.putChar('d').putUnencodedChars(path.getDirectoryDigest(xattrProvider));
} else if (stat.isSymbolicLink()) {
@@ -730,7 +747,8 @@ public String getDirectoryDigest(XattrProvider xattrProvider) throws IOException
} else {
hasher.putChar('-');
}
- hasher.putBytes(DigestUtils.getDigestWithManualFallback(resolved, xattrProvider));
+ hasher.putBytes(
+ DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(resolved, xattrProvider));
} else {
// link to a non-file: include the link itself in the hash
hasher.putChar('l').putUnencodedChars(link.toString());
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
index 3e427af8b1ca2e..9074cf39736499 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
@@ -227,8 +227,8 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
- return delegateFs.getDigest(toDelegatePath(path));
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
+ return delegateFs.getDigest(toDelegatePath(path), expectedSize);
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StubbableFSBuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StubbableFSBuildViewTest.java
index b7783a59db988d..e9d8121b41df1b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StubbableFSBuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StubbableFSBuildViewTest.java
@@ -137,7 +137,7 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
if (stubbedFastDigestErrors.containsKey(path)) {
throw stubbedFastDigestErrors.get(path);
}
- return getDigest(path);
+ return getDigest(path, -1);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
index aa698adecb8066..aaf32540e03ace 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java
@@ -61,9 +61,9 @@ protected InputStream getInputStream(PathFragment path) throws IOException {
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
byte[] override = digestOverrides.get(path.getPathString());
- return override != null ? override : super.getDigest(path);
+ return override != null ? override : super.getDigest(path, expectedSize);
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
index 81ea2772cfc2b8..59d7e3ec1df1ab 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
@@ -86,11 +86,11 @@ protected static final class FakeActionFileSystem extends DelegateFileSystem {
@Override
protected byte[] getFastDigest(PathFragment path) throws IOException {
- return super.getDigest(path);
+ return super.getDigest(path, -1);
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
throw new UnsupportedOperationException();
}
}
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 fe2af660aac9b9..0fef7f9fd37291 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
@@ -21,6 +21,7 @@
import static java.util.Arrays.stream;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
@@ -578,9 +579,9 @@ public void getDigest_fromInputArtifactData_forLocalArtifact() throws Exception
// Verify that we don't fall back to a slow digest.
reset(fs);
assertThat(actionFs.getFastDigest(path)).isEqualTo(getDigest("local contents"));
- verify(fs, never()).getDigest(any());
+ verify(fs, never()).getDigest(any(), anyLong());
- assertThat(actionFs.getDigest(path)).isEqualTo(getDigest("local contents"));
+ assertThat(actionFs.getDigest(path, -1)).isEqualTo(getDigest("local contents"));
}
@Test
@@ -593,9 +594,9 @@ public void getDigest_fromInputArtifactData_forRemoteArtifact() throws Exception
// Verify that we don't fall back to a slow digest.
reset(fs);
assertThat(actionFs.getFastDigest(path)).isEqualTo(getDigest("remote contents"));
- verify(fs, never()).getDigest(any());
+ verify(fs, never()).getDigest(any(), anyLong());
- assertThat(actionFs.getDigest(path)).isEqualTo(getDigest("remote contents"));
+ assertThat(actionFs.getDigest(path, -1)).isEqualTo(getDigest("remote contents"));
}
@Test
@@ -606,7 +607,7 @@ public void getDigest_fromRemoteOutputTree() throws Exception {
injectRemoteFile(actionFs, artifact.getPath().asFragment(), "remote contents");
assertThat(actionFs.getFastDigest(path)).isEqualTo(getDigest("remote contents"));
- assertThat(actionFs.getDigest(path)).isEqualTo(getDigest("remote contents"));
+ assertThat(actionFs.getDigest(path, -1)).isEqualTo(getDigest("remote contents"));
}
@Test
@@ -617,7 +618,7 @@ public void getDigest_fromLocalFilesystem() throws Exception {
writeLocalFile(actionFs, artifact.getPath().asFragment(), "local contents");
assertThat(actionFs.getFastDigest(path)).isNull();
- assertThat(actionFs.getDigest(path)).isEqualTo(getDigest("local contents"));
+ assertThat(actionFs.getDigest(path, -1)).isEqualTo(getDigest("local contents"));
}
@Test
@@ -627,7 +628,7 @@ public void getDigest_notFound() throws Exception {
PathFragment path = artifact.getPath().asFragment();
assertThrows(FileNotFoundException.class, () -> actionFs.getFastDigest(path));
- assertThrows(FileNotFoundException.class, () -> actionFs.getDigest(path));
+ assertThrows(FileNotFoundException.class, () -> actionFs.getDigest(path, -1));
}
@Test
@@ -650,7 +651,7 @@ public void getDigest_followSymlinks(
assertThat(actionFs.getFastDigest(linkPath)).isEqualTo(getDigest("content"));
}
- assertThat(actionFs.getDigest(linkPath)).isEqualTo(getDigest("content"));
+ assertThat(actionFs.getDigest(linkPath, -1)).isEqualTo(getDigest("content"));
}
@Test
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 8a4d43c69efc8e..3ab9f324ccc962 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
@@ -385,7 +385,7 @@ public void downloadOutputs_outputFiles() throws Exception {
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
assertThat(readContent(execRoot.getRelative("outputs/file1"), UTF_8)).isEqualTo("content1");
assertThat(context.isLockOutputFilesCalled()).isTrue();
@@ -422,9 +422,10 @@ public void downloadOutputs_outputDirectories() throws Exception {
// assert
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/a/dir/foo")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/a/dir/foo"), -1))
.isEqualTo(toBinaryDigest(fooDigest));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/a/dir/subdir/bar")))
+ assertThat(
+ actionFs.getDigest(execRoot.asFragment().getRelative("outputs/a/dir/subdir/bar"), -1))
.isEqualTo(toBinaryDigest(barDigest));
assertThat(readContent(execRoot.getRelative("outputs/a/dir/foo"), UTF_8))
.isEqualTo("foo-contents");
@@ -1150,9 +1151,9 @@ public void downloadOutputs_outputFiles_partialDownload() throws Exception {
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2"), -1))
.isEqualTo(toBinaryDigest(d2));
assertThat(execRoot.getRelative("outputs/file1").exists()).isTrue();
assertThat(execRoot.getRelative("outputs/file2").exists()).isFalse();
@@ -1184,9 +1185,9 @@ public void downloadOutputs_outputFiles_noDownload() throws Exception {
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2"), -1))
.isEqualTo(toBinaryDigest(d2));
assertThat(execRoot.getRelative("outputs/file1").exists()).isFalse();
assertThat(execRoot.getRelative("outputs/file2").exists()).isFalse();
@@ -1234,9 +1235,9 @@ public void downloadOutputs_outputDirectories_partialDownload() throws Exception
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/a/file2")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/a/file2"), -1))
.isEqualTo(toBinaryDigest(d2));
assertThat(execRoot.getRelative("outputs/dir/file1").exists()).isTrue();
assertThat(execRoot.getRelative("outputs/dir/a").exists()).isFalse();
@@ -1283,9 +1284,9 @@ public void downloadOutputs_outputDirectories_noDownload() throws Exception {
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/a/file2")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/dir/a/file2"), -1))
.isEqualTo(toBinaryDigest(d2));
assertThat(execRoot.getRelative("outputs/dir/file1").exists()).isFalse();
assertThat(execRoot.getRelative("outputs/dir/a").exists()).isFalse();
@@ -1366,8 +1367,10 @@ public void downloadOutputs_nonInlinedStdoutAndStderr_alwaysDownload() throws Ex
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(outErr.getOutputPathFragment())).isEqualTo(toBinaryDigest(dOut));
- assertThat(actionFs.getDigest(outErr.getErrorPathFragment())).isEqualTo(toBinaryDigest(dErr));
+ assertThat(actionFs.getDigest(outErr.getOutputPathFragment(), -1))
+ .isEqualTo(toBinaryDigest(dOut));
+ assertThat(actionFs.getDigest(outErr.getErrorPathFragment(), -1))
+ .isEqualTo(toBinaryDigest(dErr));
assertThat(outErr.outAsLatin1()).isEqualTo("stdout");
assertThat(outErr.errAsLatin1()).isEqualTo("stderr");
Path outputBase = checkNotNull(artifactRoot.getRoot().asPath());
@@ -1400,8 +1403,10 @@ public void downloadOutputs_inlinedStdoutAndStderr_alwaysDownload() throws Excep
// assert
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(outErr.getOutputPathFragment())).isEqualTo(toBinaryDigest(dOut));
- assertThat(actionFs.getDigest(outErr.getErrorPathFragment())).isEqualTo(toBinaryDigest(dErr));
+ assertThat(actionFs.getDigest(outErr.getOutputPathFragment(), -1))
+ .isEqualTo(toBinaryDigest(dOut));
+ assertThat(actionFs.getDigest(outErr.getErrorPathFragment(), -1))
+ .isEqualTo(toBinaryDigest(dErr));
assertThat(inMemoryOutput).isNull();
assertThat(outErr.outAsLatin1()).isEqualTo("stdout");
assertThat(outErr.errAsLatin1()).isEqualTo("stderr");
@@ -1443,9 +1448,9 @@ public void downloadOutputs_inMemoryOutput_doNotDownload() throws Exception {
assertThat(inMemoryOutput.getOutput())
.isEqualTo(ActionsTestUtil.createArtifact(artifactRoot, "file1"));
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file1"), -1))
.isEqualTo(toBinaryDigest(d1));
- assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2")))
+ assertThat(actionFs.getDigest(execRoot.asFragment().getRelative("outputs/file2"), -1))
.isEqualTo(toBinaryDigest(d2));
assertThat(execRoot.getRelative("outputs/file1").exists()).isFalse();
assertThat(execRoot.getRelative("outputs/file2").exists()).isFalse();
@@ -1554,9 +1559,11 @@ public void downloadOutputs_pathUnmapped() throws Exception {
assertThat(inMemoryOutput).isNull();
RemoteActionFileSystem actionFs = context.getActionFileSystem();
- assertThat(actionFs.getDigest(output1.getPath().asFragment())).isEqualTo(toBinaryDigest(d1));
+ assertThat(actionFs.getDigest(output1.getPath().asFragment(), -1))
+ .isEqualTo(toBinaryDigest(d1));
assertThat(readContent(output1.getPath(), UTF_8)).isEqualTo("content1");
- assertThat(actionFs.getDigest(output2.getPath().asFragment())).isEqualTo(toBinaryDigest(d2));
+ assertThat(actionFs.getDigest(output2.getPath().asFragment(), -1))
+ .isEqualTo(toBinaryDigest(d2));
assertThat(readContent(output2.getPath(), UTF_8)).isEqualTo("content2");
assertThat(context.isLockOutputFilesCalled()).isTrue();
}
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 77e43a5d1c5eae..2d9f06572147c5 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
@@ -119,8 +119,10 @@ public void testUnreadableInputWithFsWithAvailableDigest() throws Throwable {
setupRoot(
new CustomInMemoryFs() {
@Override
- public byte[] getDigest(PathFragment path) throws IOException {
- return path.getBaseName().equals("unreadable") ? expectedDigest : super.getDigest(path);
+ public byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
+ return path.getBaseName().equals("unreadable")
+ ? expectedDigest
+ : super.getDigest(path, expectedSize);
}
});
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
index f67a9a06326e30..920666934db15e 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
@@ -180,7 +180,7 @@ protected class CustomInMemoryFs extends InMemoryFileSystem {
@Override
@SuppressWarnings("UnsynchronizedOverridesSynchronized")
protected byte[] getFastDigest(PathFragment path) throws IOException {
- return fastDigest ? getDigest(path) : null;
+ return fastDigest ? getDigest(path, -1) : null;
}
}
}
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 c0a393dcd18679..a3432cc0cbcd68 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
@@ -186,7 +186,7 @@ public void testIOException() throws Exception {
FileSystem fs =
new InMemoryFileSystem(DigestHashFunction.SHA256) {
@Override
- public byte[] getDigest(PathFragment path) throws IOException {
+ public byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
throw exception;
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 7294283a2168bb..e9b2689f8aea9d 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -895,9 +895,9 @@ public void testDigest() throws Exception {
fs =
new CustomInMemoryFs(manualClock) {
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
digestCalls.incrementAndGet();
- return super.getDigest(path);
+ return super.getDigest(path, expectedSize);
}
};
pkgRoot = Root.fromPath(fs.getPath("/root"));
@@ -1816,7 +1816,7 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
if (stubbedFastDigestErrors.containsKey(path)) {
throw stubbedFastDigestErrors.get(path);
}
- return fastDigest ? getDigest(path) : null;
+ return fastDigest ? getDigest(path, -1) : null;
}
void stubStat(Path path, @Nullable FileStatus stubbedResult) {
diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java
index 357a16ae9a680b..97001c1ed4bb36 100644
--- a/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java
+++ b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java
@@ -42,7 +42,9 @@ public void testFoo() throws Exception {
// Instead of actually trying to access this file, a call to getxattr() should be made. We
// intercept this call and return a fake extended attribute value, thereby causing the checksum
// computation to be skipped entirely.
- assertThat(DigestUtils.getDigestWithManualFallback(absolutize("myfile"), SyscallCache.NO_CACHE))
+ assertThat(
+ DigestUtils.getDigestWithManualFallback(
+ absolutize("myfile"), 123, SyscallCache.NO_CACHE))
.isEqualTo(FAKE_DIGEST);
}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java
index a3ef020445fc86..def866af785f11 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java
@@ -49,7 +49,7 @@ private static void assertDigestCalculationConcurrency(
FileSystem myfs =
new InMemoryFileSystem(hf) {
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
try {
barrierLatch.countDown();
readyLatch.countDown();
@@ -59,12 +59,12 @@ protected byte[] getDigest(PathFragment path) throws IOException {
} catch (Exception e) {
throw new IOException(e);
}
- return super.getDigest(path);
+ return super.getDigest(path, expectedSize);
}
@Override
protected byte[] getFastDigest(PathFragment path) throws IOException {
- return fastDigest ? super.getDigest(path) : null;
+ return fastDigest ? super.getDigest(path, -1) : null;
}
};
@@ -75,14 +75,12 @@ protected byte[] getFastDigest(PathFragment path) throws IOException {
TestThread thread1 =
new TestThread(
- () -> {
- var unused = DigestUtils.getDigestWithManualFallback(myFile1, SyscallCache.NO_CACHE);
- });
+ () ->
+ DigestUtils.getDigestWithManualFallback(myFile1, fileSize1, SyscallCache.NO_CACHE));
TestThread thread2 =
new TestThread(
- () -> {
- var unused = DigestUtils.getDigestWithManualFallback(myFile2, SyscallCache.NO_CACHE);
- });
+ () ->
+ DigestUtils.getDigestWithManualFallback(myFile2, fileSize2, SyscallCache.NO_CACHE));
thread1.start();
thread2.start();
if (!expectConcurrent) { // Synchronized case.
@@ -111,9 +109,9 @@ protected byte[] getFastDigest(PathFragment path) {
}
@Override
- protected byte[] getDigest(PathFragment path) throws IOException {
+ protected byte[] getDigest(PathFragment path, long expectedSize) throws IOException {
getDigestCounter.incrementAndGet();
- return super.getDigest(path);
+ return super.getDigest(path, expectedSize);
}
};
@@ -122,18 +120,23 @@ protected byte[] getDigest(PathFragment path) throws IOException {
Path file = tracingFileSystem.getPath("/file.txt");
FileSystemUtils.writeContentAsLatin1(file, "some contents");
- byte[] digest = DigestUtils.getDigestWithManualFallback(file, SyscallCache.NO_CACHE);
+ byte[] digest =
+ DigestUtils.getDigestWithManualFallback(file, file.getFileSize(), SyscallCache.NO_CACHE);
assertThat(getFastDigestCounter.get()).isEqualTo(1);
assertThat(getDigestCounter.get()).isEqualTo(1);
- assertThat(DigestUtils.getDigestWithManualFallback(file, SyscallCache.NO_CACHE))
+ assertThat(
+ DigestUtils.getDigestWithManualFallback(
+ file, file.getFileSize(), SyscallCache.NO_CACHE))
.isEqualTo(digest);
assertThat(getFastDigestCounter.get()).isEqualTo(2);
assertThat(getDigestCounter.get()).isEqualTo(1); // Cached.
DigestUtils.clearCache();
- assertThat(DigestUtils.getDigestWithManualFallback(file, SyscallCache.NO_CACHE))
+ assertThat(
+ DigestUtils.getDigestWithManualFallback(
+ file, file.getFileSize(), SyscallCache.NO_CACHE))
.isEqualTo(digest);
assertThat(getFastDigestCounter.get()).isEqualTo(3);
assertThat(getDigestCounter.get()).isEqualTo(2); // Not cached.
@@ -150,13 +153,13 @@ protected byte[] getFastDigest(PathFragment path) {
}
@Override
- protected byte[] getDigest(PathFragment path) {
+ protected byte[] getDigest(PathFragment path, long expectedSize) {
return digest;
}
};
Path file = noDigestFileSystem.getPath("/f.txt");
FileSystemUtils.writeContentAsLatin1(file, "contents");
- assertThat(DigestUtils.manuallyComputeDigest(file)).isEqualTo(digest);
+ assertThat(DigestUtils.manuallyComputeDigest(file, /* fileSize= */ 8)).isEqualTo(digest);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
index 379185572c0314..5364ed36bc40f7 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
@@ -1794,11 +1794,14 @@ public void testGetDigestForEmptyFile() throws Exception {
@Test
public void testGetDigest() throws Exception {
- byte[] buffer = new byte[500000];
+ var size = 500000;
+ byte[] buffer = new byte[size];
for (int i = 0; i < buffer.length; ++i) {
buffer[i] = 1;
}
FileSystemUtils.writeContent(xFile, buffer);
+ assertThrows(IOException.class, () -> xFile.getDigest(size + 1));
+ assertThrows(IOException.class, () -> xFile.getDigest(size - 1));
Fingerprint fp = new Fingerprint(digestHashFunction);
fp.addBytes(buffer);
assertThat(fp.hexDigestAndReset())