We use digests only if a fast digest lookup is available from the filesystem. If not, we + * fall back to mtime-based digests. This avoids the case where Blaze must read all files involved + * in the build in order to check for modifications in the case where fast digest lookups are not + * available. + * + * @param stat must be of type "File". (Not a symlink). + */ + private static FileStateValue createRegularFileStateValueFromPath( + Path path, + FileStatusWithDigest stat, + boolean digestWillBeInjected, + XattrProvider xattrProvider, + @Nullable TimestampGranularityMonitor tsgm) + throws InconsistentFilesystemException { + checkState(stat.isFile(), path); + + try { + // If the digest will be injected, we can skip calling getFastDigest, but we need to store a + // contents proxy because if the digest is injected but is not available from the filesystem, + // we will need the proxy to determine whether the file was modified. + byte[] digest = digestWillBeInjected ? null : tryGetDigest(path, stat, xattrProvider); + if (digest == null) { + // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe method. + if (tsgm != null) { + tsgm.notifyDependenceOnFileTime(path.asFragment(), stat.getLastChangeTime()); + } + return new RegularFileStateValueWithContentsProxy( + stat.getSize(), FileContentsProxy.create(stat)); + } else { + // We are careful here to avoid putting the value ID into FileMetadata if we already have a + // digest. Arbitrary filesystems may do weird things with the value ID; a digest is more + // robust. + return new RegularFileStateValueWithDigest(stat.getSize(), digest); + } + } catch (IOException e) { + String errorMessage = e.getMessage() != null ? "error '" + e.getMessage() + "'" : "an error"; + throw new InconsistentFilesystemException( + "'stat' said " + + path + + " is a file but then we " + + "later encountered " + + errorMessage + + " which indicates that " + + path + + " is no " + + "longer a file. Did you delete it during the build?"); + } + } + + @Nullable + private static byte[] tryGetDigest( + Path path, FileStatusWithDigest stat, XattrProvider xattrProvider) throws IOException { + try { + byte[] digest = stat.getDigest(); + return digest != null ? digest : xattrProvider.getFastDigest(path); + } catch (IOException ioe) { + if (!path.isReadable()) { + return null; + } + throw ioe; + } + } + @ThreadSafe public static RootedPath key(RootedPath rootedPath) { // RootedPath is already the SkyKey we want; see FileStateKey. This method and that interface @@ -168,80 +235,89 @@ public String toString() { abstract String prettyPrint(); /** - * Implementation of {@link FileStateValue} for regular files that exist. - * - *
A union of (digest, mtime). We use digests only if a fast digest lookup is available from - * the filesystem. If not, we fall back to mtime-based digests. This avoids the case where Blaze - * must read all files involved in the build in order to check for modifications in the case where - * fast digest lookups are not available. + * Implementation of {@link FileStateValue} for regular files when a {@link #digest} is provided. */ - @ThreadSafe - public static final class RegularFileStateValue extends FileStateValue { + public static final class RegularFileStateValueWithDigest extends FileStateValue { private final long size; - @Nullable private final byte[] digest; - @Nullable private final FileContentsProxy contentsProxy; + private final byte[] digest; @VisibleForTesting - public RegularFileStateValue(long size, byte[] digest, FileContentsProxy contentsProxy) { - Preconditions.checkState((digest == null) != (contentsProxy == null)); + public RegularFileStateValueWithDigest(long size, byte[] digest) { this.size = size; - this.digest = digest; - this.contentsProxy = contentsProxy; - } - - /** - * Creates a FileFileStateValue instance corresponding to the given existing file. - * - * @param stat must be of type "File". (Not a symlink). - */ - private static RegularFileStateValue fromPath( - Path path, - FileStatusWithDigest stat, - boolean digestWillBeInjected, - XattrProvider xattrProvider, - @Nullable TimestampGranularityMonitor tsgm) - throws InconsistentFilesystemException { - Preconditions.checkState(stat.isFile(), path); - - try { - // If the digest will be injected, we can skip calling getFastDigest, but we need to store a - // contents proxy because if the digest is injected but is not available from the - // filesystem, we will need the proxy to determine whether the file was modified. - byte[] digest = digestWillBeInjected ? null : tryGetDigest(path, stat, xattrProvider); - if (digest == null) { - // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe - // method. - if (tsgm != null) { - tsgm.notifyDependenceOnFileTime(path.asFragment(), stat.getLastChangeTime()); - } - return new RegularFileStateValue(stat.getSize(), null, FileContentsProxy.create(stat)); - } else { - // We are careful here to avoid putting the value ID into FileMetadata if we already have - // a digest. Arbitrary filesystems may do weird things with the value ID; a digest is more - // robust. - return new RegularFileStateValue(stat.getSize(), digest, null); - } - } catch (IOException e) { - String errorMessage = e.getMessage() != null - ? "error '" + e.getMessage() + "'" : "an error"; - throw new InconsistentFilesystemException("'stat' said " + path + " is a file but then we " - + "later encountered " + errorMessage + " which indicates that " + path + " is no " - + "longer a file. Did you delete it during the build?"); - } + this.digest = checkNotNull(digest); } + @Override + public FileStateType getType() { + return FileStateType.REGULAR_FILE; + } + + @Override + public long getSize() { + return size; + } + + @Override + public byte[] getDigest() { + return digest; + } + + @Override @Nullable - private static byte[] tryGetDigest( - Path path, FileStatusWithDigest stat, XattrProvider xattrProvider) throws IOException { - try { - byte[] digest = stat.getDigest(); - return digest != null ? digest : xattrProvider.getFastDigest(path); - } catch (IOException ioe) { - if (!path.isReadable()) { - return null; - } - throw ioe; + public FileContentsProxy getContentsProxy() { + return null; + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof RegularFileStateValueWithDigest)) { + return false; } + RegularFileStateValueWithDigest other = (RegularFileStateValueWithDigest) obj; + return size == other.size && Arrays.equals(digest, other.digest); + } + + @Override + public int hashCode() { + return Objects.hash(size, Arrays.hashCode(digest)); + } + + @Override + public byte[] getValueFingerprint() { + Fingerprint fp = new Fingerprint().addLong(size); + fp.addBytes(digest); + return fp.digestAndReset(); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).add("digest", digest).add("size", size).toString(); + } + + @Override + public String prettyPrint() { + String contents = String.format("digest of %s", Arrays.toString(digest)); + return String.format("regular file with size of %d and %s", size, contents); + } + } + + /** + * Implementation of {@link FileStateValue} for regular files when {@link FileContentsProxy} is + * provided. + * + *
{@link #contentsProxy} is used to determine whether the file was modified. + */ + public static final class RegularFileStateValueWithContentsProxy extends FileStateValue { + private final long size; + private final FileContentsProxy contentsProxy; + + @VisibleForTesting + public RegularFileStateValueWithContentsProxy(long size, FileContentsProxy contentsProxy) { + this.size = size; + this.contentsProxy = checkNotNull(contentsProxy); } @Override @@ -257,7 +333,7 @@ public long getSize() { @Override @Nullable public byte[] getDigest() { - return digest; + return null; } @Override @@ -270,46 +346,37 @@ public boolean equals(Object obj) { if (obj == this) { return true; } - if (!(obj instanceof RegularFileStateValue)) { + if (!(obj instanceof RegularFileStateValueWithContentsProxy)) { return false; } - RegularFileStateValue other = (RegularFileStateValue) obj; - return size == other.size - && Arrays.equals(digest, other.digest) - && Objects.equals(contentsProxy, other.contentsProxy); + RegularFileStateValueWithContentsProxy other = (RegularFileStateValueWithContentsProxy) obj; + return size == other.size && Objects.equals(contentsProxy, other.contentsProxy); } @Override public int hashCode() { - return Objects.hash(size, Arrays.hashCode(digest), contentsProxy); + return Objects.hash(size, contentsProxy); } @Override public byte[] getValueFingerprint() { Fingerprint fp = new Fingerprint().addLong(size); - if (digest != null) { - fp.addBytes(digest); - } - if (contentsProxy != null) { - contentsProxy.addToFingerprint(fp); - } + contentsProxy.addToFingerprint(fp); return fp.digestAndReset(); } @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("digest", digest) .add("size", size) - .add("contentsProxy", contentsProxy).toString(); + .add("contentsProxy", contentsProxy) + .toString(); } @Override public String prettyPrint() { - String contents = digest != null - ? String.format("digest of %s", Arrays.toString(digest)) - : contentsProxy.prettyPrint(); - return String.format("regular file with size of %d and %s", size, contents); + return String.format( + "regular file with size of %d and %s", size, contentsProxy.prettyPrint()); } } @@ -320,7 +387,7 @@ public static final class SpecialFileStateValue extends FileStateValue { @VisibleForTesting public SpecialFileStateValue(FileContentsProxy contentsProxy) { - this.contentsProxy = Preconditions.checkNotNull(contentsProxy); + this.contentsProxy = checkNotNull(contentsProxy); } private static SpecialFileStateValue fromStat( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index ad69283bb6e1d3..3dc808cf3b19cf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java @@ -13,10 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; @@ -24,7 +26,8 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateType; import com.google.devtools.build.lib.actions.FileStateValue; -import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValue; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithContentsProxy; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithDigest; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.actions.HasDigest; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -105,14 +108,15 @@ public enum Type { SYMLINK_CYCLE_OR_INFINITE_EXPANSION, } - private final Type type; + private final RecursiveFilesystemTraversalException.Type type; - RecursiveFilesystemTraversalException(String message, Type type) { + RecursiveFilesystemTraversalException( + String message, RecursiveFilesystemTraversalException.Type type) { super(message); this.type = type; } - public Type getType() { + public RecursiveFilesystemTraversalException.Type getType() { return type; } } @@ -129,9 +133,9 @@ static final class DanglingSymlinkException extends RecursiveFilesystemTraversal super( String.format( "Found dangling symlink: %s, unresolved path: \"%s\"", path, unresolvedLink), - Type.DANGLING_SYMLINK); - Preconditions.checkArgument(path != null && !path.isEmpty()); - Preconditions.checkArgument(unresolvedLink != null && !unresolvedLink.isEmpty()); + RecursiveFilesystemTraversalException.Type.DANGLING_SYMLINK); + checkArgument(path != null && !path.isEmpty()); + checkArgument(unresolvedLink != null && !unresolvedLink.isEmpty()); } } @@ -288,8 +292,8 @@ private static final class FileInfo { HasDigest metadata, @Nullable RootedPath realPath, @Nullable PathFragment unresolvedSymlinkTarget) { - Preconditions.checkNotNull(metadata.getDigest(), metadata); - this.type = Preconditions.checkNotNull(type); + checkNotNull(metadata.getDigest(), metadata); + this.type = checkNotNull(type); this.metadata = metadata; this.realPath = realPath; this.unresolvedSymlinkTarget = unresolvedSymlinkTarget; @@ -330,7 +334,7 @@ private static FileInfo lookUpFileInfo( } RootedPath realPath = traversal.root().asRootedPath(); if (traversal.strictOutputFiles()) { - Preconditions.checkNotNull(fsVal, "Strict Fileset output tree has null FileArtifactValue"); + checkNotNull(fsVal, "Strict Fileset output tree has null FileArtifactValue"); return new FileInfo( fsVal instanceof TreeArtifactValue ? FileType.DIRECTORY : FileType.FILE, fsVal, @@ -451,15 +455,14 @@ static HasDigest withDigest(HasDigest fsVal, Path path, XattrProvider syscallCac throws IOException { if (fsVal instanceof FileStateValue) { FileStateValue fsv = (FileStateValue) fsVal; - if (fsv instanceof RegularFileStateValue) { - RegularFileStateValue rfsv = (RegularFileStateValue) fsv; - return rfsv.getDigest() != null - // If we have the digest, then simply convert it with the digest value. - ? FileArtifactValue.createForVirtualActionInput(rfsv.getDigest(), rfsv.getSize()) - // Otherwise, create a file FileArtifactValue (RegularFileArtifactValue) based on the - // path and size. - : FileArtifactValue.createForNormalFileUsingPath(path, rfsv.getSize(), syscallCache); + if (fsv instanceof RegularFileStateValueWithDigest) { + RegularFileStateValueWithDigest rfsv = (RegularFileStateValueWithDigest) fsv; + return FileArtifactValue.createForVirtualActionInput(rfsv.getDigest(), rfsv.getSize()); + } else if (fsv instanceof RegularFileStateValueWithContentsProxy) { + RegularFileStateValueWithContentsProxy rfsv = (RegularFileStateValueWithContentsProxy) fsv; + return FileArtifactValue.createForNormalFileUsingPath(path, rfsv.getSize(), syscallCache); } + return new HasDigest.ByteStringDigest(fsv.getValueFingerprint()); } else if (fsVal instanceof FileArtifactValue) { FileArtifactValue fav = ((FileArtifactValue) fsVal); @@ -481,37 +484,38 @@ private enum Type { CONFLICT, DIRECTORY, PKG } - private final Type type; + private final PkgLookupResult.Type type; final TraversalRequest traversal; final FileInfo rootInfo; /** Result for a generated directory that conflicts with a source package. */ static PkgLookupResult conflict(TraversalRequest traversal, FileInfo rootInfo) { - return new PkgLookupResult(Type.CONFLICT, traversal, rootInfo); + return new PkgLookupResult(PkgLookupResult.Type.CONFLICT, traversal, rootInfo); } /** Result for a source or generated directory (not a package). */ static PkgLookupResult directory(TraversalRequest traversal, FileInfo rootInfo) { - return new PkgLookupResult(Type.DIRECTORY, traversal, rootInfo); + return new PkgLookupResult(PkgLookupResult.Type.DIRECTORY, traversal, rootInfo); } /** Result for a package, i.e. a directory with a BUILD file. */ static PkgLookupResult pkg(TraversalRequest traversal, FileInfo rootInfo) { - return new PkgLookupResult(Type.PKG, traversal, rootInfo); + return new PkgLookupResult(PkgLookupResult.Type.PKG, traversal, rootInfo); } - private PkgLookupResult(Type type, TraversalRequest traversal, FileInfo rootInfo) { - this.type = Preconditions.checkNotNull(type); - this.traversal = Preconditions.checkNotNull(traversal); - this.rootInfo = Preconditions.checkNotNull(rootInfo); + private PkgLookupResult( + PkgLookupResult.Type type, TraversalRequest traversal, FileInfo rootInfo) { + this.type = checkNotNull(type); + this.traversal = checkNotNull(traversal); + this.rootInfo = checkNotNull(rootInfo); } boolean isPackage() { - return type == Type.PKG; + return type == PkgLookupResult.Type.PKG; } boolean isConflicting() { - return type == Type.CONFLICT; + return type == PkgLookupResult.Type.CONFLICT; } @Override @@ -531,8 +535,8 @@ public String toString() { private static PkgLookupResult checkIfPackage( Environment env, TraversalRequest traversal, FileInfo rootInfo, SyscallCache syscallCache) throws IOException, InterruptedException, BuildFileNotFoundException { - Preconditions.checkArgument(rootInfo.type.exists() && !rootInfo.type.isFile(), - "{%s} {%s}", traversal, rootInfo); + checkArgument( + rootInfo.type.exists() && !rootInfo.type.isFile(), "{%s} {%s}", traversal, rootInfo); // PackageLookupFunction/dependencies can only throw IOException, BuildFileNotFoundException, // and RepositoryFetchException, and RepositoryFetchException is not in play here. Note that // run-of-the-mill circular symlinks will *not* throw here, and will trigger later errors during @@ -579,8 +583,7 @@ private static PkgLookupResult checkIfPackage( */ private static RecursiveFilesystemTraversalValue resultForDanglingSymlink( RootedPath linkName, FileInfo info) { - Preconditions.checkState(info.type.isSymlink() && !info.type.exists(), "{%s} {%s}", linkName, - info.type); + checkState(info.type.isSymlink() && !info.type.exists(), "{%s} {%s}", linkName, info.type); return RecursiveFilesystemTraversalValue.of( ResolvedFileFactory.danglingSymlink(linkName, info.unresolvedSymlinkTarget, info.metadata)); } @@ -593,8 +596,7 @@ private static RecursiveFilesystemTraversalValue resultForDanglingSymlink( */ private static RecursiveFilesystemTraversalValue resultForFileRoot( RootedPath path, FileInfo info) { - Preconditions.checkState( - info.type.isFile() && info.type.exists(), "{%s} {%s}", path, info.type); + checkState(info.type.isFile() && info.type.exists(), "{%s} {%s}", path, info.type); if (info.type.isSymlink()) { return RecursiveFilesystemTraversalValue.of( ResolvedFileFactory.symlinkToFile( diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java index 2aaa77b6c29aa3..6198b13d32f0bb 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java @@ -21,7 +21,8 @@ import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.actions.FileContentsProxy; import com.google.devtools.build.lib.actions.FileStateValue; -import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValue; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithContentsProxy; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithDigest; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.actions.FileValue.RegularFileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -43,9 +44,7 @@ import org.junit.runners.JUnit4; import org.mockito.Mockito; -/** - * Tests for @{link RepositoryFunction} - */ +/** Tests for {@link RepositoryFunction} */ @RunWith(JUnit4.class) public class RepositoryFunctionTest extends BuildViewTestCase { @@ -139,7 +138,7 @@ public void testFileValueToMarkerValue() throws Exception { RootedPath.toRootedPath(Root.fromPath(rootDirectory), scratch.file("foo", "bar")); // Digest should be returned if the FileStateValue has it. - FileStateValue fsv = new RegularFileStateValue(3, new byte[] {1, 2, 3, 4}, null); + FileStateValue fsv = new RegularFileStateValueWithDigest(3, new byte[] {1, 2, 3, 4}); FileValue fv = new RegularFileValue(path, fsv); assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo("01020304"); @@ -147,7 +146,7 @@ public void testFileValueToMarkerValue() throws Exception { FileStatus status = Mockito.mock(FileStatus.class); when(status.getLastChangeTime()).thenReturn(100L); when(status.getNodeId()).thenReturn(200L); - fsv = new RegularFileStateValue(3, null, FileContentsProxy.create(status)); + fsv = new RegularFileStateValueWithContentsProxy(3, FileContentsProxy.create(status)); fv = new RegularFileValue(path, fsv); String expectedDigest = BaseEncoding.base16().lowerCase().encode(path.asPath().getDigest()); assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileStateValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileStateValueTest.java index 4fec7f815efd14..43c5c118363fcd 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileStateValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileStateValueTest.java @@ -33,14 +33,12 @@ public class FileStateValueTest { @Test public void testCodec() throws Exception { new SerializationTester( - new FileStateValue.RegularFileStateValue( - /*size=*/ 1, /*digest=*/ new byte[] {1, 2, 3}, /*contentsProxy=*/ null), - new FileStateValue.RegularFileStateValue( - /*size=*/ 1, /*digest=*/ new byte[0], /*contentsProxy=*/ null), - new FileStateValue.RegularFileStateValue( - /*size=*/ 1, - /*digest=*/ null, - makeFileContentsProxy(/* ctime= */ 2, /* nodeId= */ 42)), + new FileStateValue.RegularFileStateValueWithDigest( + /* size= */ 1, /* digest= */ new byte[] {1, 2, 3}), + new FileStateValue.RegularFileStateValueWithDigest( + /* size= */ 1, /* digest= */ new byte[0]), + new FileStateValue.RegularFileStateValueWithContentsProxy( + /* size= */ 1, makeFileContentsProxy(/* ctime= */ 2, /* nodeId= */ 42)), new FileStateValue.SpecialFileStateValue( makeFileContentsProxy(/* ctime= */ 4, /* nodeId= */ 84)), FileStateValue.DIRECTORY_FILE_STATE_NODE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java index 56e4acc3aafc5c..85ce4a5e24170e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileValueTest.java @@ -42,14 +42,14 @@ public void testCodec() throws Exception { FsUtils.TEST_ROOTED_PATH, FileStateValue.DIRECTORY_FILE_STATE_NODE), new FileValue.SymlinkFileValueWithStoredChain( FsUtils.TEST_ROOTED_PATH, - new FileStateValue.RegularFileStateValue( - /*size=*/ 100, /*digest=*/ new byte[] {1, 2, 3, 4, 5}, /*contentsProxy=*/ null), + new FileStateValue.RegularFileStateValueWithDigest( + /* size= */ 100, /* digest= */ new byte[] {1, 2, 3, 4, 5}), ImmutableList.of(FsUtils.TEST_ROOTED_PATH), PathFragment.create("somewhere/else")), new FileValue.SymlinkFileValueWithoutStoredChain( FsUtils.TEST_ROOTED_PATH, - new FileStateValue.RegularFileStateValue( - /*size=*/ 100, /*digest=*/ new byte[] {1, 2, 3, 4, 5}, /*contentsProxy=*/ null), + new FileStateValue.RegularFileStateValueWithDigest( + /* size= */ 100, /* digest= */ new byte[] {1, 2, 3, 4, 5}), PathFragment.create("somewhere/else"))); FsUtils.addDependencies(serializationTester); serializationTester.runTests(); 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 ee27ea04d031bb..da98f28d0cec1f 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 @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode.CROSS; @@ -26,7 +27,6 @@ import static org.junit.Assert.assertThrows; import com.google.auto.value.AutoValue; -import com.google.common.base.Preconditions; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -40,7 +40,8 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileContentsProxy; import com.google.devtools.build.lib.actions.FileStateValue; -import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValue; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithContentsProxy; +import com.google.devtools.build.lib.actions.FileStateValue.RegularFileStateValueWithDigest; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot; import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode; @@ -273,18 +274,16 @@ private static RootedPath childOf(RootedPath path, String relative) { } private static RootedPath parentOf(RootedPath path) { - return Preconditions.checkNotNull(path.getParentDirectory()); + return checkNotNull(path.getParentDirectory()); } private static RootedPath siblingOf(RootedPath path, String relative) { - PathFragment parent = - Preconditions.checkNotNull(path.getRootRelativePath().getParentDirectory()); + PathFragment parent = checkNotNull(path.getRootRelativePath().getParentDirectory()); return RootedPath.toRootedPath(path.getRoot(), parent.getRelative(relative)); } private static RootedPath siblingOf(Artifact artifact, String relative) { - PathFragment parent = - Preconditions.checkNotNull(artifact.getRootRelativePath().getParentDirectory()); + PathFragment parent = checkNotNull(artifact.getRootRelativePath().getParentDirectory()); return RootedPath.toRootedPath(artifact.getRoot().getRoot(), parent.getRelative(relative)); } @@ -1152,8 +1151,7 @@ private final class ActionFakeFunction implements SkyFunction { public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { return env.getValue( new NonHermeticArtifactSkyKey( - Preconditions.checkNotNull( - artifacts.get(((ActionLookupData) skyKey).getActionIndex()), skyKey))); + checkNotNull(artifacts.get(((ActionLookupData) skyKey).getActionIndex()), skyKey))); } } @@ -1203,8 +1201,8 @@ public void testWithDigestFileArtifactValue() throws Exception { public void testWithDigestFileStateValue() throws Exception { // RegularFileStateValue with actual digest will be transformed with the same digest byte[] expectedBytes = new byte[] {1, 2, 3}; - RegularFileStateValue withDigest = - new RegularFileStateValue(10L, expectedBytes, /* contentsProxy */ null); + RegularFileStateValueWithDigest withDigest = + new RegularFileStateValueWithDigest(/* size= */ 10L, /* digest= */ expectedBytes); HasDigest result = RecursiveFilesystemTraversalFunction.withDigest(withDigest, null, SyscallCache.NO_CACHE); assertThat(result).isInstanceOf(FileArtifactValue.class); @@ -1226,11 +1224,9 @@ public void testRegularFileStateValueWithoutDigest() throws Exception { createFile(rootedPath, "fooy-content"); FileStatus status = rootedPath.asPath().stat(); - RegularFileStateValue withoutDigest = - new RegularFileStateValue( - status.getSize(), /* digest */ - null, /* contentsProxy */ - FileContentsProxy.create(status)); + RegularFileStateValueWithContentsProxy withoutDigest = + new RegularFileStateValueWithContentsProxy( + status.getSize(), /* contentsProxy= */ FileContentsProxy.create(status)); HasDigest withoutDigestResult = RecursiveFilesystemTraversalFunction.withDigest( withoutDigest, rootedPath.asPath(), SyscallCache.NO_CACHE);