Check for additions to the directory in new_local_repository
Fixes #806.
RELNOTES: External repository correctness fix: adding a new file/directory as a
child of a new_local_repository is now noticed.
--
MOS_MIGRATED_REVID=120557511
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 be7826c..66942f3 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 @@
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 @@
@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 @@
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 @@
}
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 5756d51..9ada38d 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 @@
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 @@
* {@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 @@
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 1fd1b5f..d9c0b0b 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.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.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 @@
* {@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 @@
}
/**
+ * 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 7731c1d..3f61fe6 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 @@
* 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 d55a029..792cc5f 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.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 @@
* @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 @@
}
ExternalFilesHelper cloneWithFreshExternalFilesKnowledge() {
- return new ExternalFilesHelper(pkgLocator, externalFileAction);
+ return new ExternalFilesHelper(pkgLocator, externalFileAction, directories);
}
FileType getAndNoteFileType(RootedPath rootedPath) {
@@ -152,7 +158,7 @@
*/
@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 @@
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 ca8dfad..25bfc92 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 @@
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 76b2c11..db0744d 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 @@
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 @@
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 aa6dcd9..22a8dbe 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 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 @@
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 7e1e1cb..b046950 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 @@
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 @@
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 da88d1f..0dcc58f 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.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 @@
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 6be70e2..75a9429 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 @@
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 @@
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 95bf419..850a510 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.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 @@
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 07edc60..df83967 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 @@
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 507e93a..b2a02fb 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 @@
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 @@
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 e4e15ef..35afe3c 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 @@
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 @@
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 4a70e16..4141a8c 100755
--- a/src/test/shell/bazel/external_correctness_test.sh
+++ b/src/test/shell/bazel/external_correctness_test.sh
@@ -287,4 +287,34 @@
}
+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"
