diff options
16 files changed, 192 insertions, 65 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java index be7826cd28..66942f390d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java @@ -17,10 +17,15 @@ package com.google.devtools.build.lib.rules.repository; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.skyframe.DirectoryListingValue; +import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; @@ -38,7 +43,7 @@ public class NewLocalRepositoryFunction extends RepositoryFunction { @Override public SkyValue fetch( Rule rule, Path outputDirectory, BlazeDirectories directories, Environment env) - throws SkyFunctionException { + throws SkyFunctionException { NewRepositoryBuildFileHandler buildFileHandler = new NewRepositoryBuildFileHandler(directories.getWorkspace()); @@ -48,9 +53,29 @@ public class NewLocalRepositoryFunction extends RepositoryFunction { PathFragment pathFragment = getTargetPath(rule, directories.getWorkspace()); + FileSystem fs = directories.getOutputBase().getFileSystem(); + Path path = fs.getPath(pathFragment); + + // fetch() creates symlinks to each child under 'path' and DiffAwareness handles checking all + // of these files and directories for changes. However, if a new file/directory is added + // directly under 'path', Bazel doesn't know that this has to be symlinked in. Thus, this + // creates a dependency on the contents of the 'path' directory. + SkyKey dirKey = DirectoryListingValue.key( + RootedPath.toRootedPath(fs.getRootDirectory(), path)); + DirectoryListingValue directoryValue; + try { + directoryValue = (DirectoryListingValue) env.getValueOrThrow( + dirKey, InconsistentFilesystemException.class); + } catch (InconsistentFilesystemException e) { + throw new RepositoryFunctionException( + new IOException(e), SkyFunctionException.Transience.PERSISTENT); + } + if (directoryValue == null) { + return null; + } + // Link x/y/z to /some/path/to/y/z. - if (!symlinkLocalRepositoryContents( - outputDirectory, directories.getOutputBase().getFileSystem().getPath(pathFragment))) { + if (!symlinkLocalRepositoryContents(outputDirectory, fs.getPath(pathFragment))) { return null; } @@ -68,7 +93,7 @@ public class NewLocalRepositoryFunction extends RepositoryFunction { } createWorkspaceFile(outputDirectory, rule); - return RepositoryDirectoryValue.create(outputDirectory); + return RepositoryDirectoryValue.createWithSourceDirectory(outputDirectory, directoryValue); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java index 5756d51753..9ada38de87 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryValue.java @@ -16,28 +16,43 @@ package com.google.devtools.build.lib.rules.repository; import com.google.common.base.Objects; import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.skyframe.DirectoryListingValue; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import javax.annotation.Nullable; + /** * A local view of an external repository. */ public class RepositoryDirectoryValue implements SkyValue { private final Path path; private final boolean fetchingDelayed; + @Nullable + private final DirectoryListingValue sourceDir; - private RepositoryDirectoryValue(Path path, boolean fetchingDelayed) { + private RepositoryDirectoryValue( + Path path, boolean fetchingDelayed, DirectoryListingValue sourceDir) { this.path = path; this.fetchingDelayed = fetchingDelayed; + this.sourceDir = sourceDir; } /** * Creates an immutable external repository. */ public static RepositoryDirectoryValue create(Path repositoryDirectory) { - return new RepositoryDirectoryValue(repositoryDirectory, false); + return new RepositoryDirectoryValue(repositoryDirectory, false, null); + } + + /** + * new_local_repositories + */ + public static RepositoryDirectoryValue createWithSourceDirectory( + Path repositoryDirectory, DirectoryListingValue sourceDir) { + return new RepositoryDirectoryValue(repositoryDirectory, false, sourceDir); } /** @@ -45,7 +60,7 @@ public class RepositoryDirectoryValue implements SkyValue { * {@code --nofetch} command line option. */ public static RepositoryDirectoryValue fetchingDelayed(Path repositoryDirectory) { - return new RepositoryDirectoryValue(repositoryDirectory, true); + return new RepositoryDirectoryValue(repositoryDirectory, true, null); } /** @@ -70,14 +85,15 @@ public class RepositoryDirectoryValue implements SkyValue { if (other instanceof RepositoryDirectoryValue) { RepositoryDirectoryValue otherValue = (RepositoryDirectoryValue) other; - return path.equals(otherValue.path); + return Objects.equal(path, otherValue.path) + && Objects.equal(sourceDir, otherValue.sourceDir); } return false; } @Override public int hashCode() { - return Objects.hashCode(path); + return Objects.hashCode(path, sourceDir); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index 1fd1b5fccc..d9c0b0b697 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.skyframe.DirectoryListingValue; import com.google.devtools.build.lib.skyframe.FileSymlinkException; import com.google.devtools.build.lib.skyframe.FileValue; import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException; @@ -35,6 +36,7 @@ import com.google.devtools.build.lib.skyframe.WorkspaceFileValue; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -80,6 +82,7 @@ import javax.annotation.Nullable; * {@link RepositoryDirectoryValue} is invalidated using the usual Skyframe route. */ public abstract class RepositoryFunction { + /** * Exception thrown when something goes wrong accessing a remote repository. * @@ -350,6 +353,69 @@ public abstract class RepositoryFunction { } /** + * For files that are under $OUTPUT_BASE/external, add a dependency on the corresponding rule + * so that if the WORKSPACE file changes, the File/DirectoryStateValue will be re-evaluated. + * + * Note that: + * - We don't add a dependency on the parent directory at the package root boundary, so + * the only transitive dependencies from files inside the package roots to external files + * are through symlinks. So the upwards transitive closure of external files is small. + * - The only way other than external repositories for external source files to get into the + * skyframe graph in the first place is through symlinks outside the package roots, which we + * neither want to encourage nor optimize for since it is not common. So the set of external + * files is small. + */ + public static void addExternalFilesDependencies( + RootedPath rootedPath, BlazeDirectories directories, Environment env) + throws IOException { + Path externalRepoDir = getExternalRepositoryDirectory(directories); + PathFragment repositoryPath = rootedPath.asPath().relativeTo(externalRepoDir); + if (repositoryPath.segmentCount() == 0) { + // We are the top of the repository path (<outputBase>/external), not in an actual external + // repository path. + return; + } + String repositoryName = repositoryPath.getSegment(0); + + Rule repositoryRule; + try { + repositoryRule = RepositoryFunction.getRule(repositoryName, env); + } catch (RepositoryFunction.RepositoryNotFoundException ex) { + // The repository we are looking for does not exist so we should depend on the whole + // WORKSPACE file. In that case, the call to RepositoryFunction#getRule(String, Environment) + // already requested all repository functions from the WORKSPACE file from Skyframe as part + // of the resolution. Therefore we are safe to ignore that Exception. + return; + } catch (RepositoryFunction.RepositoryFunctionException ex) { + // This should never happen. + throw new IllegalStateException( + "Repository " + repositoryName + " cannot be resolved for path " + rootedPath, ex); + } + if (repositoryRule == null) { + return; + } + + // new_local_repository needs a dependency on the directory that `path` points to, as the + // external/repo-name DirStateValue has a logical dependency on that directory that is not + // reflected in the SkyFrame tree, since it's not symlinked to it or anything. + if (repositoryRule.getRuleClass().equals(NewLocalRepositoryRule.NAME) + && repositoryPath.segmentCount() == 1) { + PathFragment pathDir = RepositoryFunction.getTargetPath( + repositoryRule, directories.getWorkspace()); + FileSystem fs = directories.getWorkspace().getFileSystem(); + SkyKey dirKey = DirectoryListingValue.key( + RootedPath.toRootedPath(fs.getRootDirectory(), fs.getPath(pathDir))); + try { + env.getValueOrThrow( + dirKey, IOException.class, FileSymlinkException.class, + InconsistentFilesystemException.class); + } catch (FileSymlinkException | InconsistentFilesystemException e) { + throw new IOException(e.getMessage()); + } + } + } + + /** * Returns the RuleDefinition class for this type of repository. */ public abstract Class<? extends RuleDefinition> getRuleDefinition(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java index 7731c1dd4e..3f61fe660e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java @@ -54,7 +54,7 @@ public abstract class DirectoryListingValue implements SkyValue { * from a directory listing on its parent directory). */ @ThreadSafe - static SkyKey key(RootedPath directoryUnderRoot) { + public static SkyKey key(RootedPath directoryUnderRoot) { return SkyKey.create(SkyFunctions.DIRECTORY_LISTING, directoryUnderRoot); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java index d55a0297f4..792cc5fa13 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; @@ -20,16 +21,17 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; +import java.io.IOException; import java.util.concurrent.atomic.AtomicReference; /** Common utilities for dealing with files outside the package roots. */ public class ExternalFilesHelper { private final AtomicReference<PathPackageLocator> pkgLocator; private final ExternalFileAction externalFileAction; + private final BlazeDirectories directories; // These variables are set to true from multiple threads, but only read in the main thread. // So volatility or an AtomicBoolean is not needed. @@ -42,18 +44,22 @@ public class ExternalFilesHelper { * @param errorOnExternalFiles If files outside of package paths should be allowed. */ public ExternalFilesHelper( - AtomicReference<PathPackageLocator> pkgLocator, boolean errorOnExternalFiles) { + AtomicReference<PathPackageLocator> pkgLocator, boolean errorOnExternalFiles, + BlazeDirectories directories) { this( pkgLocator, errorOnExternalFiles ? ExternalFileAction.ERROR_OUT - : ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES); + : ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES, + directories); } private ExternalFilesHelper(AtomicReference<PathPackageLocator> pkgLocator, - ExternalFileAction externalFileAction) { + ExternalFileAction externalFileAction, + BlazeDirectories directories) { this.pkgLocator = pkgLocator; this.externalFileAction = externalFileAction; + this.directories = directories; } private enum ExternalFileAction { @@ -117,7 +123,7 @@ public class ExternalFilesHelper { } ExternalFilesHelper cloneWithFreshExternalFilesKnowledge() { - return new ExternalFilesHelper(pkgLocator, externalFileAction); + return new ExternalFilesHelper(pkgLocator, externalFileAction, directories); } FileType getAndNoteFileType(RootedPath rootedPath) { @@ -152,7 +158,7 @@ public class ExternalFilesHelper { */ @ThreadSafe public void maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment env) - throws FileOutsidePackageRootsException { + throws IOException { FileType fileType = getAndNoteFileType(rootedPath); if (fileType == FileType.INTERNAL) { return; @@ -166,39 +172,6 @@ public class ExternalFilesHelper { Preconditions.checkState( externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES, externalFileAction); - - // For files that are under $OUTPUT_BASE/external, add a dependency on the corresponding rule - // so that if the WORKSPACE file changes, the File/DirectoryStateValue will be re-evaluated. - // - // Note that: - // - We don't add a dependency on the parent directory at the package root boundary, so - // the only transitive dependencies from files inside the package roots to external files - // are through symlinks. So the upwards transitive closure of external files is small. - // - The only way other than external repositories for external source files to get into the - // skyframe graph in the first place is through symlinks outside the package roots, which we - // neither want to encourage nor optimize for since it is not common. So the set of external - // files is small. - - Path externalRepoDir = pkgLocator.get().getOutputBase().getRelative(Label.EXTERNAL_PATH_PREFIX); - PathFragment repositoryPath = rootedPath.asPath().relativeTo(externalRepoDir); - if (repositoryPath.segmentCount() == 0) { - // We are the top of the repository path (<outputBase>/external), not in an actual external - // repository path. - return; - } - String repositoryName = repositoryPath.getSegment(0); - - try { - RepositoryFunction.getRule(repositoryName, env); - } catch (RepositoryFunction.RepositoryNotFoundException ex) { - // The repository we are looking for does not exist so we should depend on the whole - // WORKSPACE file. In that case, the call to RepositoryFunction#getRule(String, Environment) - // already requested all repository functions from the WORKSPACE file from Skyframe as part of - // the resolution. Therefore we are safe to ignore that Exception. - } catch (RepositoryFunction.RepositoryFunctionException ex) { - // This should never happen. - throw new IllegalStateException( - "Repository " + repositoryName + " cannot be resolved for path " + rootedPath, ex); - } + RepositoryFunction.addExternalFilesDependencies(rootedPath, directories, env); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index ca8dfadd2d..25bfc92688 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -310,7 +310,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { binTools, (ConfiguredRuleClassProvider) pkgFactory.getRuleClassProvider()); this.artifactFactory.set(skyframeBuildView.getArtifactFactory()); - this.externalFilesHelper = new ExternalFilesHelper(pkgLocator, this.errorOnExternalFiles); + this.externalFilesHelper = new ExternalFilesHelper( + pkgLocator, this.errorOnExternalFiles, directories); } private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions( 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 76b2c11e03..db0744db75 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 @@ -71,7 +71,9 @@ abstract class ArtifactFunctionTestCase { setupRoot(new CustomInMemoryFs()); AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>(new PathPackageLocator( root.getFileSystem().getPath("/outputbase"), ImmutableList.of(root))); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + BlazeDirectories directories = new BlazeDirectories(root, root, root); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, directories); differencer = new RecordingDifferencer(); evaluator = new InMemoryMemoizingEvaluator( @@ -95,7 +97,7 @@ abstract class ArtifactFunctionTestCase { new WorkspaceFileFunction( TestRuleClassProvider.getRuleClassProvider(), new PackageFactory(TestRuleClassProvider.getRuleClassProvider()), - new BlazeDirectories(root, root, root))) + directories)) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()) .build(), differencer); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index aa6dcd974d..22a8dbeb73 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertTrue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.testing.EqualsTester; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; @@ -59,7 +60,8 @@ public class ContainingPackageLookupFunctionTest extends FoundationTestCase { AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>(new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory))); deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, new BlazeDirectories(rootDirectory, rootDirectory, rootDirectory)); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put(SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction(deletedPackages)); 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 7e1e1cb040..b0469509b1 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 @@ -122,8 +122,9 @@ public class FileFunctionTest { private SequentialBuildDriver makeDriver(boolean errorOnExternalFiles) { AtomicReference<PathPackageLocator> pkgLocatorRef = new AtomicReference<>(pkgLocator); + BlazeDirectories directories = new BlazeDirectories(pkgRoot, outputBase, pkgRoot); ExternalFilesHelper externalFilesHelper = - new ExternalFilesHelper(pkgLocatorRef, errorOnExternalFiles); + new ExternalFilesHelper(pkgLocatorRef, errorOnExternalFiles, directories); differencer = new RecordingDifferencer(); MemoizingEvaluator evaluator = new InMemoryMemoizingEvaluator( @@ -152,7 +153,7 @@ public class FileFunctionTest { new WorkspaceFileFunction( TestRuleClassProvider.getRuleClassProvider(), new PackageFactory(TestRuleClassProvider.getRuleClassProvider()), - new BlazeDirectories(pkgRoot, outputBase, pkgRoot))) + directories)) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()) .build(), differencer); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java index da88d1f7f4..0dcc58f0dc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.actions.FilesetTraversalParams; import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode; import com.google.devtools.build.lib.actions.FilesetTraversalParamsFactory; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.NullEventHandler; @@ -82,7 +83,8 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase { new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory))); AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, new BlazeDirectories(outputBase, outputBase, rootDirectory)); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index 6be70e2c7a..75a9429f09 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -103,7 +103,9 @@ public class FilesystemValueCheckerTest { AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>(new PathPackageLocator( fs.getPath("/output_base"), ImmutableList.of(pkgRoot))); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + BlazeDirectories directories = new BlazeDirectories(pkgRoot, pkgRoot, pkgRoot); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, directories); skyFunctions.put(SkyFunctions.FILE_STATE, new FileStateFunction( new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper)); skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator)); @@ -121,7 +123,7 @@ public class FilesystemValueCheckerTest { skyFunctions.put(SkyFunctions.WORKSPACE_FILE, new WorkspaceFileFunction(TestRuleClassProvider.getRuleClassProvider(), new PackageFactory(TestRuleClassProvider.getRuleClassProvider()), - new BlazeDirectories(pkgRoot, pkgRoot, pkgRoot))); + directories)); skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()); differencer = new RecordingDifferencer(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java index 95bf419ed8..850a510877 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.testing.EqualsTester; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; @@ -122,7 +123,8 @@ public abstract class GlobFunctionTest { private Map<SkyFunctionName, SkyFunction> createFunctionMap() { AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, new BlazeDirectories(root, root, root)); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put(SkyFunctions.GLOB, new GlobFunction(alwaysUseDirListing())); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index 07edc60c1d..df83967caf 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -72,8 +72,9 @@ public class PackageLookupFunctionTest extends FoundationTestCase { AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>( new PathPackageLocator(outputBase, ImmutableList.of(emptyPackagePath, rootDirectory))); deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); BlazeDirectories directories = new BlazeDirectories(rootDirectory, outputBase, rootDirectory); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, directories); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put(SkyFunctions.PACKAGE_LOOKUP, 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 507e93a8f2..b2a02fba91 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 @@ -85,7 +85,9 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory))); AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + BlazeDirectories directories = new BlazeDirectories(rootDirectory, outputBase, rootDirectory); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, directories); Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); @@ -110,7 +112,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe skyFunctions.put(SkyFunctions.WORKSPACE_FILE, new WorkspaceFileFunction(TestRuleClassProvider.getRuleClassProvider(), new PackageFactory(TestRuleClassProvider.getRuleClassProvider()), - new BlazeDirectories(rootDirectory, outputBase, rootDirectory))); + directories)); skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()); progressReceiver = new RecordingEvaluationProgressReceiver(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index e4e15ef894..35afe3ccc6 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -139,7 +139,9 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>(new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory))); AtomicReference<TimestampGranularityMonitor> tsgmRef = new AtomicReference<>(tsgm); - ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(pkgLocator, false); + BlazeDirectories directories = new BlazeDirectories(rootDirectory, outputBase, rootDirectory); + ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper( + pkgLocator, false, directories); differencer = new RecordingDifferencer(); ActionExecutionStatusReporter statusReporter = @@ -174,7 +176,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { new WorkspaceFileFunction( TestRuleClassProvider.getRuleClassProvider(), new PackageFactory(TestRuleClassProvider.getRuleClassProvider()), - new BlazeDirectories(rootDirectory, outputBase, rootDirectory))) + directories)) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction()) .build(), differencer, diff --git a/src/test/shell/bazel/external_correctness_test.sh b/src/test/shell/bazel/external_correctness_test.sh index 4a70e16523..4141a8ccd2 100755 --- a/src/test/shell/bazel/external_correctness_test.sh +++ b/src/test/shell/bazel/external_correctness_test.sh @@ -287,4 +287,34 @@ EOF } +function test_top_level_dir_changes() { + mkdir -p r/subdir m + touch r/one r/subdir/two + + cat > m/WORKSPACE <<'EOF' +new_local_repository( + name = "r", + path = "../r", + build_file_content = """ +genrule( + name = "fg", + cmd = "ls $(SRCS) > $@", + srcs=glob(["**"]), + outs = ["fg.out"], +)""", +) +EOF + cd m + bazel build @r//:fg &> $TEST_log || \ + fail "Expected build to succeed" + touch ../r/three + bazel build @r//:fg &> $TEST_log || \ + fail "Expected build to succeed" + assert_contains "external/r/three" bazel-genfiles/external/r/fg.out + touch ../r/subdir/four + bazel build @r//:fg &> $TEST_log || \ + fail "Expected build to succeed" + assert_contains "external/r/subdir/four" bazel-genfiles/external/r/fg.out +} + run_suite "//external correctness tests" |