diff options
5 files changed, 285 insertions, 71 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java index 75e0da2423..7569544b9d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import java.util.Set; +import javax.annotation.Nullable; /** * Parameters of a filesystem traversal requested by a Fileset rule. @@ -71,6 +72,13 @@ public interface FilesetTraversalParams { abstract class DirectTraversalRoot { /** + * Returns the output Artifact corresponding to this traversal, if present. Only present when + * traversing a generated output. + */ + @Nullable + public abstract Artifact getOutputArtifact(); + + /** * Returns the root part of the full path. * * <p>This is typically the workspace root or some output tree's root (e.g. genfiles, binfiles). @@ -94,15 +102,22 @@ public interface FilesetTraversalParams { @Override public abstract int hashCode(); - static DirectTraversalRoot forPackage(Artifact buildFile) { + public static DirectTraversalRoot forPackage(Artifact buildFile) { return new AutoValue_FilesetTraversalParams_DirectTraversalRoot( + null, buildFile.getRoot().getRoot(), buildFile.getRootRelativePath().getParentDirectory()); } - static DirectTraversalRoot forFileOrDirectory(Artifact fileOrDirectory) { + public static DirectTraversalRoot forFileOrDirectory(Artifact fileOrDirectory) { return new AutoValue_FilesetTraversalParams_DirectTraversalRoot( + fileOrDirectory.isSourceArtifact() ? null : fileOrDirectory, fileOrDirectory.getRoot().getRoot(), fileOrDirectory.getRootRelativePath()); } + + public static DirectTraversalRoot forRootedPath(RootedPath newPath) { + return new AutoValue_FilesetTraversalParams_DirectTraversalRoot(null, + newPath.getRoot(), newPath.getRootRelativePath()); + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java index e003bdd0a9..02f256f433 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java @@ -238,7 +238,7 @@ public final class FilesetEntryFunction implements SkyFunction { Environment env, String errorInfo, DirectTraversal traversal) throws MissingDepException, InterruptedException { SkyKey depKey = RecursiveFilesystemTraversalValue.key( - new RecursiveFilesystemTraversalValue.TraversalRequest(traversal.getRoot().asRootedPath(), + new RecursiveFilesystemTraversalValue.TraversalRequest(traversal.getRoot(), traversal.isGenerated(), traversal.getPackageBoundaryMode(), traversal.isPackage(), errorInfo)); RecursiveFilesystemTraversalValue v = (RecursiveFilesystemTraversalValue) env.getValue(depKey); 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 f6bdfc69f8..e46776d94d 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 @@ -17,6 +17,7 @@ import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.Collections2; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; @@ -24,9 +25,13 @@ import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue. import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFileFactory; import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.TraversalRequest; import com.google.devtools.build.lib.vfs.Dirent; +import com.google.devtools.build.lib.vfs.FileStatus; +import com.google.devtools.build.lib.vfs.FileStatusWithDigest; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -34,6 +39,8 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -58,7 +65,7 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { String.format( "Generated directory %s conflicts with package under the same path. " + "Additional info: %s", - traversal.path.getRootRelativePath().getPathString(), + traversal.root.asRootedPath().getRootRelativePath().getPathString(), traversal.errorInfo != null ? traversal.errorInfo : traversal.toString())); } } @@ -126,14 +133,14 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { if (!rootInfo.type.exists()) { // May be a dangling symlink or a non-existent file. Handle gracefully. if (rootInfo.type.isSymlink()) { - return resultForDanglingSymlink(traversal.path, rootInfo); + return resultForDanglingSymlink(traversal.root.asRootedPath(), rootInfo); } else { return RecursiveFilesystemTraversalValue.EMPTY; } } if (rootInfo.type.isFile()) { - return resultForFileRoot(traversal.path, rootInfo); + return resultForFileRoot(traversal.root.asRootedPath(), rootInfo); } // Otherwise the root is a directory or a symlink to one. @@ -150,7 +157,7 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { String msg = traversal.errorInfo + " crosses package boundary into package rooted at " - + traversal.path.getRootRelativePath().getPathString(); + + traversal.root.asRootedPath().getRootRelativePath().getPathString(); switch (traversal.crossPkgBoundaries) { case CROSS: // We are free to traverse the subpackage but we need to display a warning. @@ -170,7 +177,8 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { // We are free to traverse this directory. Collection<SkyKey> dependentKeys = createRecursiveTraversalKeys(env, traversal); - return resultForDirectory(traversal, rootInfo, traverseChildren(env, dependentKeys)); + return resultForDirectory(traversal, rootInfo, + traverseChildren(env, dependentKeys, /*inline=*/traversal.isGenerated)); } catch (IOException e) { throw new RecursiveFilesystemTraversalFunctionException( new FileOperationException("Error while traversing fileset: " + e.getMessage())); @@ -211,31 +219,82 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { private static FileInfo lookUpFileInfo(Environment env, TraversalRequest traversal) throws MissingDepException, IOException, InterruptedException { - // Stat the file. - FileValue fileValue = - (FileValue) env.getValueOrThrow(FileValue.key(traversal.path), IOException.class); + if (traversal.isGenerated) { + byte[] digest = null; + if (traversal.root.getOutputArtifact() != null) { + Artifact artifact = traversal.root.getOutputArtifact(); + SkyKey artifactKey = ArtifactSkyKey.key(artifact, true); + SkyValue value = env.getValue(artifactKey); + if (env.valuesMissing()) { + throw new MissingDepException(); + } - if (env.valuesMissing()) { - throw new MissingDepException(); - } - if (fileValue.exists()) { - // If it exists, it may either be a symlink or a file/directory. + if (value instanceof FileArtifactValue) { + FileArtifactValue fsVal = (FileArtifactValue) value; + digest = fsVal.getDigest(); + } else { + return new FileInfo( + FileType.NONEXISTENT, null, null, null); + } + } + + // FileArtifactValue does not currently track symlinks. If it did, we could potentially remove + // some of the filesystem operations we're doing here. + Path path = traversal.root.asRootedPath().asPath(); + FileStatus noFollowStat = path.stat(Symlinks.NOFOLLOW); + FileStatus followStat = path.statIfFound(Symlinks.FOLLOW); + FileType type; PathFragment unresolvedLinkTarget = null; - FileType type = null; - if (fileValue.isSymlink()) { - unresolvedLinkTarget = fileValue.getUnresolvedLinkTarget(); - type = fileValue.isDirectory() ? FileType.SYMLINK_TO_DIRECTORY : FileType.SYMLINK_TO_FILE; + RootedPath realPath = traversal.root.asRootedPath(); + if (followStat == null) { + type = FileType.DANGLING_SYMLINK; + if (!noFollowStat.isSymbolicLink()) { + throw new IOException("Expected symlink for " + path + ", but got: " + noFollowStat); + } + unresolvedLinkTarget = path.readSymbolicLink(); + } else if (noFollowStat.isFile()) { + type = FileType.FILE; + } else if (noFollowStat.isDirectory()) { + type = FileType.DIRECTORY; } else { - type = fileValue.isDirectory() ? FileType.DIRECTORY : FileType.FILE; + unresolvedLinkTarget = path.readSymbolicLink(); + realPath = RootedPath.toRootedPath( + Root.absoluteRoot(path.getFileSystem()), + path.resolveSymbolicLinks()); + type = followStat.isFile() ? FileType.SYMLINK_TO_FILE : FileType.SYMLINK_TO_DIRECTORY; } - return new FileInfo(type, fileValue.realFileStateValue(), - fileValue.realRootedPath(), unresolvedLinkTarget); + return new FileInfo(type, + FileStateValue.createWithStatNoFollow(traversal.root.asRootedPath(), + new StatWithDigest(noFollowStat, digest), null), + realPath, unresolvedLinkTarget); } else { - // If it doesn't exist, or it's a dangling symlink, we still want to handle that gracefully. - return new FileInfo( - fileValue.isSymlink() ? FileType.DANGLING_SYMLINK : FileType.NONEXISTENT, - fileValue.realFileStateValue(), null, - fileValue.isSymlink() ? fileValue.getUnresolvedLinkTarget() : null); + // Stat the file. + FileValue fileValue = + (FileValue) env.getValueOrThrow( + FileValue.key(traversal.root.asRootedPath()), IOException.class); + + if (env.valuesMissing()) { + throw new MissingDepException(); + } + if (fileValue.exists()) { + // If it exists, it may either be a symlink or a file/directory. + PathFragment unresolvedLinkTarget = null; + FileType type; + if (fileValue.isSymlink()) { + unresolvedLinkTarget = fileValue.getUnresolvedLinkTarget(); + type = fileValue.isDirectory() ? FileType.SYMLINK_TO_DIRECTORY : FileType.SYMLINK_TO_FILE; + } else { + type = fileValue.isDirectory() ? FileType.DIRECTORY : FileType.FILE; + } + return new FileInfo(type, fileValue.realFileStateValue(), + fileValue.realRootedPath(), unresolvedLinkTarget); + } else { + // If it doesn't exist, or it's a dangling symlink, we still want to handle that gracefully. + return new FileInfo( + fileValue.isSymlink() ? FileType.DANGLING_SYMLINK : FileType.NONEXISTENT, + fileValue.realFileStateValue(), null, + fileValue.isSymlink() ? fileValue.getUnresolvedLinkTarget() : null); + } } } @@ -297,7 +356,8 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { "{%s} {%s}", traversal, rootInfo); PackageLookupValue pkgLookup = (PackageLookupValue) - getDependentSkyValue(env, PackageLookupValue.key(traversal.path.getRootRelativePath())); + getDependentSkyValue(env, + PackageLookupValue.key(traversal.root.asRootedPath().getRootRelativePath())); if (pkgLookup.packageExists()) { if (traversal.isGenerated) { @@ -307,7 +367,7 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { } else { // The traversal's root was a source directory and it defines a package. Root pkgRoot = pkgLookup.getRoot(); - if (!pkgRoot.equals(traversal.path.getRoot())) { + if (!pkgRoot.equals(traversal.root.asRootedPath().getRoot())) { // However the root of this package is different from what we expected. stat() the real // BUILD file of that package. traversal = traversal.forChangedRootPath(pkgRoot); @@ -330,18 +390,29 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { */ private static Collection<SkyKey> createRecursiveTraversalKeys( Environment env, TraversalRequest traversal) - throws MissingDepException, InterruptedException { + throws MissingDepException, InterruptedException, IOException { // Use the traversal's path, even if it's a symlink. The contents of the directory, as listed // in the result, must be relative to it. - DirectoryListingValue dirListing = (DirectoryListingValue) getDependentSkyValue(env, - DirectoryListingValue.key(traversal.path)); + Iterable<Dirent> dirents; + if (traversal.isGenerated) { + // If we're dealing with an output file, read the directory directly instead of creating + // filesystem nodes under the output tree. + List<Dirent> direntsCollection = + new ArrayList<>( + traversal.root.asRootedPath().asPath().readdir(Symlinks.FOLLOW)); + Collections.sort(direntsCollection); + dirents = direntsCollection; + } else { + dirents = ((DirectoryListingValue) getDependentSkyValue(env, + DirectoryListingValue.key(traversal.root.asRootedPath()))).getDirents(); + } List<SkyKey> result = new ArrayList<>(); - for (Dirent dirent : dirListing.getDirents()) { + for (Dirent dirent : dirents) { RootedPath childPath = RootedPath.toRootedPath( - traversal.path.getRoot(), - traversal.path.getRootRelativePath().getRelative(dirent.getName())); + traversal.root.asRootedPath().getRoot(), + traversal.root.asRootedPath().getRootRelativePath().getRelative(dirent.getName())); TraversalRequest childTraversal = traversal.forChildEntry(childPath); result.add(RecursiveFilesystemTraversalValue.key(childTraversal)); } @@ -395,7 +466,7 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { root = ResolvedFileFactory.symlinkToDirectory( rootInfo.realPath, - traversal.path, + traversal.root.asRootedPath(), rootInfo.unresolvedSymlinkTarget, hashDirectorySymlink(children, rootInfo.metadata.hashCode())); paths = NestedSetBuilder.<ResolvedFile>stableOrder().addTransitive(children).add(root); @@ -434,12 +505,25 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { * * <p>The keys must all be {@link SkyFunctions#RECURSIVE_FILESYSTEM_TRAVERSAL} keys. */ - private static Collection<RecursiveFilesystemTraversalValue> traverseChildren( - Environment env, Iterable<SkyKey> keys) throws MissingDepException, InterruptedException { - Map<SkyKey, SkyValue> values = env.getValues(keys); + private Collection<RecursiveFilesystemTraversalValue> traverseChildren( + Environment env, Iterable<SkyKey> keys, boolean inline) + throws MissingDepException, InterruptedException, + RecursiveFilesystemTraversalFunctionException { + Map<SkyKey, SkyValue> values; + if (inline) { + // Don't create Skyframe nodes for a recursive traversal over the output tree. + // Instead, inline the recursion in the top-level request. + values = new HashMap<>(); + for (SkyKey depKey : keys) { + values.put(depKey, compute(depKey, env)); + } + } else { + values = env.getValues(keys); + } if (env.valuesMissing()) { throw new MissingDepException(); } + return Collections2.transform(values.values(), new Function<SkyValue, RecursiveFilesystemTraversalValue>() { @Override @@ -505,4 +589,60 @@ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { boolean exists() { return false; } @Override public abstract String toString(); } + + private static class StatWithDigest implements FileStatusWithDigest { + private final FileStatus noFollowStat; + private final byte[] digest; + + public StatWithDigest(FileStatus noFollowStat, byte[] digest) { + this.noFollowStat = noFollowStat; + this.digest = digest; + } + + @Nullable + @Override + public byte[] getDigest() throws IOException { + return digest; + } + + @Override + public boolean isFile() { + return noFollowStat.isFile(); + } + + @Override + public boolean isDirectory() { + return noFollowStat.isDirectory(); + } + + @Override + public boolean isSymbolicLink() { + return noFollowStat.isSymbolicLink(); + } + + @Override + public boolean isSpecialFile() { + return noFollowStat.isSpecialFile(); + } + + @Override + public long getSize() throws IOException { + return noFollowStat.getSize(); + } + + @Override + public long getLastModifiedTime() throws IOException { + return noFollowStat.getLastModifiedTime(); + } + + @Override + public long getLastChangeTime() throws IOException { + return noFollowStat.getLastChangeTime(); + } + + @Override + public long getNodeId() throws IOException { + return noFollowStat.getNodeId(); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java index 247e546335..7a1a269808 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot; import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -53,8 +54,7 @@ import javax.annotation.Nullable; */ public final class RecursiveFilesystemTraversalValue implements SkyValue { static final RecursiveFilesystemTraversalValue EMPTY = new RecursiveFilesystemTraversalValue( - Optional.<ResolvedFile>absent(), - NestedSetBuilder.<ResolvedFile>emptySet(Order.STABLE_ORDER)); + Optional.absent(), NestedSetBuilder.emptySet(Order.STABLE_ORDER)); /** The root of the traversal. May only be absent for the {@link #EMPTY} instance. */ private final Optional<ResolvedFile> resolvedRoot; @@ -110,7 +110,7 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue { public static final class TraversalRequest { /** The path to start the traversal from; may be a file, a directory or a symlink. */ - final RootedPath path; + final DirectTraversalRoot root; /** * Whether the path is in the output tree. @@ -135,24 +135,25 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue { /** Information to be attached to any error messages that may be reported. */ @Nullable final String errorInfo; - public TraversalRequest(RootedPath path, boolean isRootGenerated, + public TraversalRequest(DirectTraversalRoot root, boolean isRootGenerated, PackageBoundaryMode crossPkgBoundaries, boolean skipTestingForSubpackage, @Nullable String errorInfo) { - this.path = path; + this.root = root; this.isGenerated = isRootGenerated; this.crossPkgBoundaries = crossPkgBoundaries; this.skipTestingForSubpackage = skipTestingForSubpackage; this.errorInfo = errorInfo; } - private TraversalRequest duplicate(RootedPath newRoot, boolean newSkipTestingForSubpackage) { + private TraversalRequest duplicate(DirectTraversalRoot newRoot, + boolean newSkipTestingForSubpackage) { return new TraversalRequest(newRoot, isGenerated, crossPkgBoundaries, newSkipTestingForSubpackage, errorInfo); } /** Creates a new request to traverse a child element in the current directory (the root). */ TraversalRequest forChildEntry(RootedPath newPath) { - return duplicate(newPath, false); + return duplicate(DirectTraversalRoot.forRootedPath(newPath), false); } /** @@ -163,7 +164,9 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue { */ TraversalRequest forChangedRootPath(Root newRoot) { return duplicate( - RootedPath.toRootedPath(newRoot, path.getRootRelativePath()), skipTestingForSubpackage); + DirectTraversalRoot.forRootedPath( + RootedPath.toRootedPath(newRoot, root.asRootedPath().getRootRelativePath())), + skipTestingForSubpackage); } @Override @@ -175,21 +178,21 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue { return false; } TraversalRequest o = (TraversalRequest) obj; - return path.equals(o.path) && isGenerated == o.isGenerated + return root.equals(o.root) && isGenerated == o.isGenerated && crossPkgBoundaries == o.crossPkgBoundaries && skipTestingForSubpackage == o.skipTestingForSubpackage; } @Override public int hashCode() { - return Objects.hashCode(path, isGenerated, crossPkgBoundaries, skipTestingForSubpackage); + return Objects.hashCode(root, isGenerated, crossPkgBoundaries, skipTestingForSubpackage); } @Override public String toString() { return String.format( "TraversalParams(root=%s, is_generated=%d, skip_testing_for_subpkg=%d," - + " pkg_boundaries=%s)", path, isGenerated ? 1 : 0, + + " pkg_boundaries=%s)", root, isGenerated ? 1 : 0, skipTestingForSubpackage ? 1 : 0, crossPkgBoundaries); } } @@ -660,7 +663,7 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue { * <p>The object stores things such as the absolute path of the file or symlink, its exact type * and, if it's a symlink, the resolved and unresolved link target paths. */ - public static interface ResolvedFile { + public interface ResolvedFile { /** Type of the entity under {@link #getPath()}. */ FileType getType(); 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 70022558ca..fbfeb1e4e9 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 @@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot; import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; @@ -37,13 +38,13 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; +import com.google.devtools.build.lib.skyframe.ArtifactSkyKey.OwnedArtifact; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalFunction.FileOperationException; import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFile; import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.TraversalRequest; import com.google.devtools.build.lib.testutil.FoundationTestCase; -import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; @@ -57,9 +58,12 @@ import com.google.devtools.build.skyframe.RecordingDifferencer; import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; import com.google.devtools.build.skyframe.SequentialBuildDriver; import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.HashMap; @@ -68,6 +72,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -81,10 +86,12 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe private SequentialBuildDriver driver; private RecordingDifferencer differencer; private AtomicReference<PathPackageLocator> pkgLocator; + private ArtifactFakeFunction artifactFakeFunction; @Before public final void setUp() throws Exception { AnalysisMock analysisMock = AnalysisMock.get(); + artifactFakeFunction = new ArtifactFakeFunction(); pkgLocator = new AtomicReference<>( new PathPackageLocator( @@ -104,7 +111,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe ConfiguredRuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put(SkyFunctions.FILE_STATE, new FileStateFunction( - new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper)); + new AtomicReference<>(), externalFilesHelper)); skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( @@ -140,6 +147,9 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe new FileSymlinkInfiniteExpansionUniquenessFunction()); skyFunctions.put( SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS, new FileSymlinkCycleUniquenessFunction()); + skyFunctions.put( + SkyFunctions.ARTIFACT, + artifactFakeFunction); progressReceiver = new RecordingEvaluationProgressReceiver(); differencer = new SequencedRecordingDifferencer(); @@ -226,13 +236,14 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe } private static TraversalRequest fileLikeRoot(Artifact file, PackageBoundaryMode pkgBoundaryMode) { - return new TraversalRequest( - rootedPath(file), !file.isSourceArtifact(), pkgBoundaryMode, false, null); + return new TraversalRequest(DirectTraversalRoot.forFileOrDirectory(file), + !file.isSourceArtifact(), pkgBoundaryMode, false, null); } private static TraversalRequest pkgRoot( RootedPath pkgDirectory, PackageBoundaryMode pkgBoundaryMode) { - return new TraversalRequest(pkgDirectory, false, pkgBoundaryMode, true, null); + return new TraversalRequest(DirectTraversalRoot.forRootedPath(pkgDirectory), false, + pkgBoundaryMode, true, null); } private <T extends SkyValue> EvaluationResult<T> eval(SkyKey key) throws Exception { @@ -278,26 +289,39 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe return result; } - private void appendToFile(RootedPath rootedPath, String content) throws Exception { + private void appendToFile(RootedPath rootedPath, SkyKey toInvalidate, String content) + throws Exception { Path path = rootedPath.asPath(); if (path.exists()) { try (OutputStream os = path.getOutputStream(/*append=*/ true)) { os.write(content.getBytes(StandardCharsets.UTF_8)); } - differencer.invalidate(ImmutableList.of(FileStateValue.key(rootedPath))); + differencer.invalidate(ImmutableList.of(toInvalidate)); } else { createFile(path, content); } } + private void appendToFile(RootedPath rootedPath, String content) throws Exception { + appendToFile(rootedPath, FileStateValue.key(rootedPath), content); + } + private void appendToFile(Artifact file, String content) throws Exception { - appendToFile(rootedPath(file), content); + SkyKey key = file.isSourceArtifact() + ? FileStateValue.key(rootedPath(file)) + : ArtifactSkyKey.key(file, true); + appendToFile(rootedPath(file), key, content); } private void invalidateDirectory(RootedPath path) { differencer.invalidate(ImmutableList.of(DirectoryListingStateValue.key(path))); } + private void invalidateOutputArtifact(Artifact output) { + assertThat(output.isSourceArtifact()).isFalse(); + differencer.invalidate(ImmutableList.of(ArtifactSkyKey.key(output, true))); + } + private void invalidateDirectory(Artifact directoryArtifact) { invalidateDirectory(rootedPath(directoryArtifact)); } @@ -461,7 +485,11 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe // Add a new file to the directory and see that the value is rebuilt. RootedPath file3 = createFile(childOf(directoryArtifact, "foo.txt")); - invalidateDirectory(directoryArtifact); + if (directoryArtifact.isSourceArtifact()) { + invalidateDirectory(directoryArtifact); + } else { + invalidateOutputArtifact(directoryArtifact); + } ResolvedFile expected3 = regularFileForTesting(file3); RecursiveFilesystemTraversalValue v2 = traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3); @@ -474,16 +502,23 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe progressReceiver.clear(); // Edit a file in the directory and see that the value is rebuilt. - appendToFile(file1, "bar"); - RecursiveFilesystemTraversalValue v3 = - traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3); - assertThat(progressReceiver.invalidations).contains(rftvSkyKey(traversalRoot)); - assertThat(progressReceiver.evaluations).contains(v3); - assertThat(v3).isNotEqualTo(v2); - // Directories always have the same hash code, but that is fine because their contents are also - // part of the RecursiveFilesystemTraversalValue, so v2 and v3 are unequal. - assertTraversalRootHashesAreEqual(v2, v3); - progressReceiver.clear(); + RecursiveFilesystemTraversalValue v3; + if (directoryArtifact.isSourceArtifact()) { + SkyKey toInvalidate = FileStateValue.key(file1); + appendToFile(file1, toInvalidate, "bar"); + v3 = traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3); + assertThat(progressReceiver.invalidations).contains(rftvSkyKey(traversalRoot)); + assertThat(progressReceiver.evaluations).contains(v3); + assertThat(v3).isNotEqualTo(v2); + // Directories always have the same hash code, but that is fine because their contents are + // also part of the RecursiveFilesystemTraversalValue, so v2 and v3 are unequal. + assertTraversalRootHashesAreEqual(v2, v3); + progressReceiver.clear(); + } else { + // Dependency checking of output directories is unsound. Specifically, the directory mtime + // is not changed when a contained file is modified. + v3 = v2; + } // Add a new file *outside* of the directory and see that the value is *not* rebuilt. Artifact someFile = sourceArtifact("somewhere/else/a.file"); @@ -851,4 +886,25 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe assertThat(error.getException()).isInstanceOf(FileOperationException.class); assertThat(error.getException()).hasMessageThat().contains("Symlink cycle"); } + + private static class ArtifactFakeFunction implements SkyFunction { + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + OwnedArtifact ownedArtifact = (OwnedArtifact) skyKey.argument(); + Artifact artifact = ownedArtifact.getArtifact(); + try { + return FileArtifactValue.create(artifact.getPath()); + } catch (IOException e) { + throw new SkyFunctionException(e, Transience.PERSISTENT){}; + } + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + } } |