Make files and directories under managed directories be correctly processed by SequencedSkyframeExecutor.
TL;DR - two main changes: how to invalidate changed managed directories files, and how to force owning external repository to be evaluated before managed directories files.
- In ExternalFilesHelper, introduce one more file type - EXTERNAL_REPO_IN_USER_DIRECTORY, for the files under managed directories.
- For files under managed directories, require owning RepositoryDirectoryValue to be evaluated first.
- For correct dirtying of files under managed directories, both with watchfs flag and without, the following should be taken into account: not only that new values for external repositories and managed directories files can not be injected at the stage of dirtying, but also files that used to be under managed directories on the previous evaluator invocation.
The latter are still cached in evaluator, and the fact that they used to depend on their RepositoryDirectory values would prevent injection of the new values for them.
To meet those conditions, in SequencedSkyframeExecutor.handleChangedFiles() filtering of the going-to-be-injected files is added.
(The change can not be done inside ExternalDirtinessChecker only, as then it does not affect watchfs=true case).
- ManagedDirectoriesBlackBoxTest added to demonstrate and validate the functionality of managed directories.
PiperOrigin-RevId: 246496823
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 1d0fa39..961a95a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -51,6 +51,7 @@
import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl.ManagedDirectoriesListener;
import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
@@ -70,6 +71,7 @@
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryBootstrap;
import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -87,7 +89,6 @@
/** Adds support for fetching external code. */
public class BazelRepositoryModule extends BlazeModule {
-
// Default location (relative to output user root) of the repository cache.
public static final String DEFAULT_CACHE_LOCATION = "cache/repos/v1";
@@ -107,12 +108,28 @@
private FileSystem filesystem;
// We hold the precomputed value of the managed directories here, so that the dependency
// on WorkspaceFileValue is not registered for each FileStateValue.
- private final ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge =
- new ManagedDirectoriesKnowledgeImpl();
+ private final ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge;
public BazelRepositoryModule() {
this.skylarkRepositoryFunction = new SkylarkRepositoryFunction(httpDownloader);
this.repositoryHandlers = repositoryRules(httpDownloader, mavenDownloader);
+ ManagedDirectoriesListener listener =
+ repositoryNamesWithManagedDirs -> {
+ Set<String> conflicting =
+ overrides.keySet().stream()
+ .filter(repositoryNamesWithManagedDirs::contains)
+ .map(RepositoryName::getName)
+ .collect(Collectors.toSet());
+ if (!conflicting.isEmpty()) {
+ String message =
+ "Overriding repositories is not allowed"
+ + " for the repositories with managed directories.\n"
+ + "The following overridden external repositories have managed directories: "
+ + String.join(", ", conflicting.toArray(new String[0]));
+ throw new AbruptExitException(message, ExitCode.COMMAND_LINE_ERROR);
+ }
+ };
+ managedDirectoriesKnowledge = new ManagedDirectoriesKnowledgeImpl(listener);
}
public static ImmutableMap<String, RepositoryFunction> repositoryRules(
@@ -153,10 +170,7 @@
@Override
public void workspaceInit(
BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) {
- builder.setWorkspaceFileHeaderListener(
- value ->
- managedDirectoriesKnowledge.setManagedDirectories(
- value != null ? value.getManagedDirectories() : ImmutableMap.of()));
+ builder.setManagedDirectoriesKnowledge(managedDirectoriesKnowledge);
RepositoryDirectoryDirtinessChecker customDirtinessChecker =
new RepositoryDirectoryDirtinessChecker(managedDirectoriesKnowledge);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java
index 66ca156..2af4b8b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledge.java
@@ -16,8 +16,9 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.RootedPath;
import javax.annotation.Nullable;
/**
@@ -29,12 +30,18 @@
* <p>Having managed directories as a separate component (and not SkyValue) allows to skip recording
* the dependency in Skyframe for each FileStateValue and DirectoryListingStateValue.
*/
-public interface ManagedDirectoriesKnowledge {
+public interface ManagedDirectoriesKnowledge extends WorkspaceFileHeaderListener {
ManagedDirectoriesKnowledge NO_MANAGED_DIRECTORIES =
new ManagedDirectoriesKnowledge() {
+ @Override
+ public boolean workspaceHeaderReloaded(
+ @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) {
+ return false;
+ }
+
@Nullable
@Override
- public RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old) {
+ public RepositoryName getOwnerRepository(PathFragment relativePathFragment) {
return null;
}
@@ -44,8 +51,15 @@
}
};
+ /**
+ * Returns the owning repository for the incrementally updated path, or null.
+ *
+ * @param relativePathFragment path to check, relative to workspace root
+ * @return RepositoryName or null if there is no owning repository
+ */
@Nullable
- RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old);
+ RepositoryName getOwnerRepository(PathFragment relativePathFragment);
+ /** Returns managed directories for the passed repository. */
ImmutableSet<PathFragment> getManagedDirectories(RepositoryName repositoryName);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java
index 08b44fa..b2344dd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java
@@ -14,61 +14,37 @@
package com.google.devtools.build.lib.rules.repository;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.RootedPath;
import java.util.Comparator;
import java.util.Map;
-import java.util.NavigableMap;
+import java.util.Objects;
import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/** Managed directories component. {@link ManagedDirectoriesKnowledge} */
public class ManagedDirectoriesKnowledgeImpl implements ManagedDirectoriesKnowledge {
- private final AtomicReference<ImmutableSortedMap<PathFragment, RepositoryName>>
- managedDirectoriesRef = new AtomicReference<>(ImmutableSortedMap.of());
- private final AtomicReference<Map<RepositoryName, ImmutableSet<PathFragment>>> repoToDirMapRef =
- new AtomicReference<>(ImmutableMap.of());
+ private final ManagedDirectoriesListener listener;
- /**
- * During build commands execution, Skyframe caches the states of files (FileStateValue) and
- * directory listings (DirectoryListingStateValue). In the case when the files/directories are
- * under a managed directory or inside an external repository, evaluation of file/directory
- * listing states requires that the RepositoryDirectoryValue of the owning external repository is
- * evaluated beforehand. (So that the repository rule generates the files.) So there is a
- * dependency on RepositoryDirectoryValue for files under managed directories and external
- * repositories. This dependency is recorded by Skyframe,
- *
- * <p>From the other side, by default Skyframe injects the new values of changed files already at
- * the stage of checking what files have changed. Only the values without any dependencies can be
- * injected into Skyframe. Skyframe can be specifically instructed to not inject new values and
- * only register them as changed.
- *
- * <p>When the values of managed directories change, some files appear to become files under
- * managed directories, or they are no longer files under managed directories. This implies that
- * corresponding file/directory listing states gain the dependency (RepositoryDirectoryValue) or
- * they lose this dependency. In both cases, we should prevent Skyframe from injecting those new
- * values of file/directory listing states at the stage of checking changed files.
- *
- * <p>That is why we need to keep track of the previous value of the managed directories.
- */
- private final AtomicReference<ImmutableSortedMap<PathFragment, RepositoryName>>
- oldManagedDirectoriesRef = new AtomicReference<>(ImmutableSortedMap.of());
+ private ImmutableSortedMap<PathFragment, RepositoryName> dirToRepoMap = ImmutableSortedMap.of();
+ private ImmutableSortedMap<RepositoryName, ImmutableSet<PathFragment>> repoToDirMap =
+ ImmutableSortedMap.of();
+
+ public ManagedDirectoriesKnowledgeImpl(ManagedDirectoriesListener listener) {
+ this.listener = listener;
+ }
@Override
@Nullable
- public RepositoryName getOwnerRepository(RootedPath rootedPath, boolean old) {
- PathFragment relativePath = rootedPath.getRootRelativePath();
- NavigableMap<PathFragment, RepositoryName> map =
- old ? oldManagedDirectoriesRef.get() : managedDirectoriesRef.get();
- Map.Entry<PathFragment, RepositoryName> entry = map.floorEntry(relativePath);
- if (entry != null && relativePath.startsWith(entry.getKey())) {
+ public RepositoryName getOwnerRepository(PathFragment relativePathFragment) {
+ Map.Entry<PathFragment, RepositoryName> entry = dirToRepoMap.floorEntry(relativePathFragment);
+ if (entry != null && relativePathFragment.startsWith(entry.getKey())) {
return entry.getValue();
}
return null;
@@ -76,26 +52,53 @@
@Override
public ImmutableSet<PathFragment> getManagedDirectories(RepositoryName repositoryName) {
- ImmutableSet<PathFragment> pathFragments = repoToDirMapRef.get().get(repositoryName);
+ ImmutableSet<PathFragment> pathFragments = repoToDirMap.get(repositoryName);
return pathFragments != null ? pathFragments : ImmutableSet.of();
}
- public void setManagedDirectories(ImmutableMap<PathFragment, RepositoryName> map) {
- oldManagedDirectoriesRef.set(managedDirectoriesRef.get());
- ImmutableSortedMap<PathFragment, RepositoryName> pathsMap = ImmutableSortedMap.copyOf(map);
- managedDirectoriesRef.set(pathsMap);
+ @Override
+ public boolean workspaceHeaderReloaded(
+ @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue)
+ throws AbruptExitException {
+ if (Objects.equals(oldValue, newValue)) {
+ listener.onManagedDirectoriesRefreshed(repoToDirMap.keySet());
+ return false;
+ }
+ Map<PathFragment, RepositoryName> oldDirToRepoMap = dirToRepoMap;
+ refreshMappings(newValue);
+ if (!Objects.equals(oldDirToRepoMap, dirToRepoMap)) {
+ listener.onManagedDirectoriesRefreshed(repoToDirMap.keySet());
+ return true;
+ }
+ return false;
+ }
+
+ private void refreshMappings(@Nullable WorkspaceFileValue newValue) {
+ if (newValue == null) {
+ dirToRepoMap = ImmutableSortedMap.of();
+ repoToDirMap = ImmutableSortedMap.of();
+ return;
+ }
+
+ dirToRepoMap = ImmutableSortedMap.copyOf(newValue.getManagedDirectories());
Map<RepositoryName, Set<PathFragment>> reposMap = Maps.newHashMap();
- for (Map.Entry<PathFragment, RepositoryName> entry : pathsMap.entrySet()) {
+ for (Map.Entry<PathFragment, RepositoryName> entry : dirToRepoMap.entrySet()) {
RepositoryName repositoryName = entry.getValue();
reposMap.computeIfAbsent(repositoryName, name -> Sets.newTreeSet()).add(entry.getKey());
}
- ImmutableSortedMap.Builder<RepositoryName, ImmutableSet<PathFragment>> builder =
+ ImmutableSortedMap.Builder<RepositoryName, ImmutableSet<PathFragment>> reposMapBuilder =
new ImmutableSortedMap.Builder<>(Comparator.comparing(RepositoryName::getName));
for (Map.Entry<RepositoryName, Set<PathFragment>> entry : reposMap.entrySet()) {
- builder.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue()));
+ reposMapBuilder.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue()));
}
- repoToDirMapRef.set(builder.build());
+ repoToDirMap = reposMapBuilder.build();
+ }
+
+ /** Interface allows {@link BazelRepositoryModule} to react to managed directories refreshes. */
+ public interface ManagedDirectoriesListener {
+ void onManagedDirectoriesRefreshed(Set<RepositoryName> repositoryNames)
+ throws AbruptExitException;
}
}
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 2b2332b..a996df9 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
@@ -99,10 +99,10 @@
* {@link RepositoryDelegatorFunction} has to know how to catch.</p>
*/
public static class RepositoryFunctionException extends SkyFunctionException {
+
public RepositoryFunctionException(NoSuchPackageException cause, Transience transience) {
super(cause, transience);
}
-
/**
* Error reading or writing to the filesystem.
*/
@@ -117,11 +117,11 @@
super(cause, transience);
}
}
-
/**
* Exception thrown when something a repository rule cannot be found.
*/
public static final class RepositoryNotFoundException extends RepositoryFunctionException {
+
public RepositoryNotFoundException(String repositoryName) {
super(
new BuildFileContainsErrorsException(
@@ -130,7 +130,6 @@
Transience.PERSISTENT);
}
}
-
/**
* An exception thrown when a dependency is missing to notify the SkyFunction from an evaluation.
*/
@@ -140,7 +139,6 @@
super(Location.BUILTIN, "Internal exception");
}
}
-
/**
* repository functions can throw the result of this function to notify the RepositoryFunction
* that a dependency was missing and the evaluation of the function must be restarted.
@@ -528,7 +526,7 @@
*/
public static void addExternalFilesDependencies(
RootedPath rootedPath, boolean isDirectory, BlazeDirectories directories, Environment env)
- throws IOException, InterruptedException {
+ throws InterruptedException {
Path externalRepoDir = getExternalRepositoryDirectory(directories);
PathFragment repositoryPath = rootedPath.asPath().relativeTo(externalRepoDir);
if (repositoryPath.isEmpty()) {
@@ -579,7 +577,6 @@
// Alternatively, the repository might still be provided by an override. Therefore, in
// any case, register the dependency on the repository overrides.
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.get(env);
- return;
} catch (ExternalPackageException ex) {
// This should never happen.
throw new IllegalStateException(
@@ -588,6 +585,22 @@
}
/**
+ * For paths that are under managed directories, we require that the corresponding FileStateValue
+ * or DirectoryListingStateValue is evaluated only after RepositoryDirectoryValue is evaluated.
+ * This way we guarantee that the repository rule is given a chance to update the managed
+ * directory before the files under the managed directory are accessed.
+ *
+ * <p>We do not need to require anything else (comparing to dependencies required for external
+ * repositories files), as overriding external repositories with managed directories is currently
+ * forbidden; also, we do not have do perform special checks for local_repository targets, since
+ * such targets cannot have managed directories by definition.
+ */
+ public static void addManagedDirectoryDependencies(RepositoryName repositoryName, Environment env)
+ throws InterruptedException {
+ env.getValue(RepositoryDirectoryValue.key(repositoryName));
+ }
+
+ /**
* Sets up a mapping of environment variables to use.
*/
public void setClientEnvironment(Map<String, String> clientEnvironment) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
index bf10273..a86a406 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
@@ -23,8 +23,8 @@
import com.google.devtools.build.lib.exec.BinTools;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.profiler.memory.AllocationTracker;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.skyframe.DiffAwareness;
-import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutorFactory;
import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -53,7 +53,7 @@
private final ImmutableList.Builder<SkyValueDirtinessChecker> customDirtinessCheckers =
ImmutableList.builder();
private AllocationTracker allocationTracker;
- private WorkspaceFileHeaderListener workspaceFileHeaderListener;
+ private ManagedDirectoriesKnowledge managedDirectoriesKnowledge;
WorkspaceBuilder(BlazeDirectories directories, BinTools binTools) {
this.directories = directories;
@@ -82,7 +82,7 @@
diffAwarenessFactories.build(),
skyFunctions.build(),
customDirtinessCheckers.build(),
- workspaceFileHeaderListener);
+ managedDirectoriesKnowledge);
return new BlazeWorkspace(
runtime,
directories,
@@ -157,9 +157,9 @@
return this;
}
- public WorkspaceBuilder setWorkspaceFileHeaderListener(
- WorkspaceFileHeaderListener workspaceFileHeaderListener) {
- this.workspaceFileHeaderListener = workspaceFileHeaderListener;
+ public WorkspaceBuilder setManagedDirectoriesKnowledge(
+ ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
+ this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
return this;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
index c3d3ce9..8e8b5bb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.UnixGlob.FilesystemCalls;
@@ -51,10 +52,17 @@
RootedPath dirRootedPath = (RootedPath) skyKey.argument();
try {
- externalFilesHelper.maybeHandleExternalFile(dirRootedPath, true, env);
+ FileType fileType = externalFilesHelper.maybeHandleExternalFile(dirRootedPath, true, env);
if (env.valuesMissing()) {
return null;
}
+ if (fileType == FileType.EXTERNAL_REPO
+ || fileType == FileType.EXTERNAL_IN_MANAGED_DIRECTORY) {
+ // Do not use syscallCache as files under repositories get generated during the build,
+ // while syscallCache is used independently from Skyframe and generally assumes
+ // the file system is frozen at the beginning of the build command.
+ return DirectoryListingStateValue.create(dirRootedPath);
+ }
return DirectoryListingStateValue.create(
syscallCache.get().readdir(dirRootedPath.asPath(), Symlinks.NOFOLLOW));
} catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
index ce4de19..860f437 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
@@ -106,7 +106,11 @@
}
}
- /** Checks files outside of the package roots for changes. */
+ /**
+ * Serves for tracking whether there are external and output files {@see ExternalFilesKnowledge}.
+ * Filtering of files, for which the new values should not be injected into evaluator, is done in
+ * SequencedSkyframeExecutor.handleChangedFiles().
+ */
static final class ExternalDirtinessChecker extends BasicFilesystemDirtinessChecker {
private final ExternalFilesHelper externalFilesHelper;
private final EnumSet<FileType> fileTypesToCheck;
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 abc7577..1fcbc4e 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
@@ -14,12 +14,17 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.FileStateValue;
+import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -45,40 +50,59 @@
private boolean anyOutputFilesSeen = false;
private boolean anyNonOutputExternalFilesSeen = false;
+ private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge;
+
private ExternalFilesHelper(
AtomicReference<PathPackageLocator> pkgLocator,
ExternalFileAction externalFileAction,
BlazeDirectories directories,
- int maxNumExternalFilesToLog) {
+ int maxNumExternalFilesToLog,
+ ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
this.pkgLocator = pkgLocator;
this.externalFileAction = externalFileAction;
this.directories = directories;
this.maxNumExternalFilesToLog = maxNumExternalFilesToLog;
+ this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
}
public static ExternalFilesHelper create(
AtomicReference<PathPackageLocator> pkgLocator,
ExternalFileAction externalFileAction,
- BlazeDirectories directories) {
+ BlazeDirectories directories,
+ ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
return IN_TEST
- ? createForTesting(pkgLocator, externalFileAction, directories)
+ ? createForTesting(pkgLocator, externalFileAction, directories, managedDirectoriesKnowledge)
: new ExternalFilesHelper(
pkgLocator,
externalFileAction,
directories,
- /*maxNumExternalFilesToLog=*/ 100);
+ /*maxNumExternalFilesToLog=*/ 100,
+ managedDirectoriesKnowledge);
}
public static ExternalFilesHelper createForTesting(
AtomicReference<PathPackageLocator> pkgLocator,
ExternalFileAction externalFileAction,
BlazeDirectories directories) {
+ return createForTesting(
+ pkgLocator,
+ externalFileAction,
+ directories,
+ ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES);
+ }
+
+ private static ExternalFilesHelper createForTesting(
+ AtomicReference<PathPackageLocator> pkgLocator,
+ ExternalFileAction externalFileAction,
+ BlazeDirectories directories,
+ ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
return new ExternalFilesHelper(
pkgLocator,
externalFileAction,
directories,
// These log lines are mostly spam during unit and integration tests.
- /*maxNumExternalFilesToLog=*/ 0);
+ /*maxNumExternalFilesToLog=*/ 0,
+ managedDirectoriesKnowledge);
}
@@ -105,12 +129,12 @@
/** Classification of a path encountered by Bazel. */
public enum FileType {
- /** A path inside the package roots or in an external repository. */
+ /** A path inside the package roots. */
INTERNAL,
/**
- * A non {@link #EXTERNAL_REPO} path outside the package roots about which we may make no other
- * assumptions.
+ * A non {@link #EXTERNAL_REPO} or {@link #EXTERNAL_IN_MANAGED_DIRECTORY} path outside the
+ * package roots about which we may make no other assumptions.
*/
EXTERNAL,
@@ -132,9 +156,22 @@
/**
* A path in the part of Bazel's output tree that contains (/ symlinks to) to external
- * repositories.
+ * repositories. Every such path under the external repository is generated by the execution of
+ * the corresponding repository rule, so these paths should not be cached by Skyframe before the
+ * RepositoryDirectoryValue is computed.
*/
EXTERNAL_REPO,
+
+ /**
+ * A path is under one of the managed directories. Managed directories are user-owned
+ * directories, which can be incrementally updated by repository rules, so that the updated
+ * files are visible for the actions in the same build.
+ *
+ * <p>Every such path under the managed directory is generated or updated by the execution of
+ * the corresponding repository rule, so these paths should not be cached by Skyframe before the
+ * RepositoryDirectoryValue is computed. {@link ManagedDirectoriesKnowledge}
+ */
+ EXTERNAL_IN_MANAGED_DIRECTORY,
}
/**
@@ -168,10 +205,35 @@
ExternalFilesHelper cloneWithFreshExternalFilesKnowledge() {
return new ExternalFilesHelper(
- pkgLocator, externalFileAction, directories, maxNumExternalFilesToLog);
+ pkgLocator,
+ externalFileAction,
+ directories,
+ maxNumExternalFilesToLog,
+ managedDirectoriesKnowledge);
}
- FileType getAndNoteFileType(RootedPath rootedPath) {
+ public FileType getAndNoteFileType(RootedPath rootedPath) {
+ return getFileTypeAndRepository(rootedPath).getFirst();
+ }
+
+ private Pair<FileType, RepositoryName> getFileTypeAndRepository(RootedPath rootedPath) {
+ RepositoryName repositoryName =
+ managedDirectoriesKnowledge.getOwnerRepository(rootedPath.getRootRelativePath());
+ if (repositoryName != null) {
+ anyNonOutputExternalFilesSeen = true;
+ return Pair.of(FileType.EXTERNAL_IN_MANAGED_DIRECTORY, repositoryName);
+ }
+ FileType fileType = detectFileType(rootedPath);
+ if (FileType.EXTERNAL == fileType || FileType.EXTERNAL_REPO == fileType) {
+ anyNonOutputExternalFilesSeen = true;
+ }
+ if (FileType.OUTPUT == fileType) {
+ anyOutputFilesSeen = true;
+ }
+ return Pair.of(fileType, null);
+ }
+
+ private FileType detectFileType(RootedPath rootedPath) {
PathPackageLocator packageLocator = pkgLocator.get();
if (packageLocator.getPathEntries().contains(rootedPath.getRoot())) {
return FileType.INTERNAL;
@@ -204,27 +266,39 @@
* a {@link NonexistentImmutableExternalFileException} instead.
*/
@ThreadSafe
- void maybeHandleExternalFile(
+ FileType maybeHandleExternalFile(
RootedPath rootedPath, boolean isDirectory, SkyFunction.Environment env)
throws NonexistentImmutableExternalFileException, IOException, InterruptedException {
- FileType fileType = getAndNoteFileType(rootedPath);
- if (fileType == FileType.INTERNAL) {
- return;
+ Pair<FileType, RepositoryName> pair = getFileTypeAndRepository(rootedPath);
+
+ FileType fileType = Preconditions.checkNotNull(pair.getFirst());
+ switch (fileType) {
+ case EXTERNAL_IN_MANAGED_DIRECTORY:
+ Preconditions.checkState(
+ externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+ externalFileAction);
+ RepositoryFunction.addManagedDirectoryDependencies(pair.getSecond(), env);
+ break;
+ case INTERNAL:
+ break;
+ case EXTERNAL:
+ if (numExternalFilesLogged.incrementAndGet() < maxNumExternalFilesToLog) {
+ logger.info("Encountered an external path " + rootedPath);
+ }
+ // fall through
+ case OUTPUT:
+ if (externalFileAction
+ == ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS) {
+ throw new NonexistentImmutableExternalFileException();
+ }
+ break;
+ case EXTERNAL_REPO:
+ Preconditions.checkState(
+ externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+ externalFileAction);
+ RepositoryFunction.addExternalFilesDependencies(rootedPath, isDirectory, directories, env);
+ break;
}
- if (fileType == FileType.OUTPUT || fileType == FileType.EXTERNAL) {
- if (externalFileAction
- == ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS) {
- throw new NonexistentImmutableExternalFileException();
- }
- if (fileType == FileType.EXTERNAL
- && numExternalFilesLogged.incrementAndGet() < maxNumExternalFilesToLog) {
- logger.info("Encountered an external path " + rootedPath);
- }
- return;
- }
- Preconditions.checkState(
- externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
- externalFileAction);
- RepositoryFunction.addExternalFilesDependencies(rootedPath, isDirectory, directories, env);
+ return fileType;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
index 289c943..8ece6ee 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.devtools.build.lib.actions.FileStateValue;
+import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.UnixGlob.FilesystemCalls;
@@ -50,10 +51,15 @@
RootedPath rootedPath = (RootedPath) skyKey.argument();
try {
- externalFilesHelper.maybeHandleExternalFile(rootedPath, false, env);
+ FileType fileType = externalFilesHelper.maybeHandleExternalFile(rootedPath, false, env);
if (env.valuesMissing()) {
return null;
}
+ if (fileType == FileType.EXTERNAL_REPO
+ || fileType == FileType.EXTERNAL_IN_MANAGED_DIRECTORY) {
+ // do not use syscallCache as files under repositories get generated during the build
+ return FileStateValue.create(rootedPath, tsgm.get());
+ }
return FileStateValue.create(rootedPath, syscallCache.get(), tsgm.get());
} catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) {
return FileStateValue.NONEXISTENT_FILE_STATE_NODE;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 08f5fa1..8130c4d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -62,7 +62,9 @@
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.query2.AqueryActionFilter;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.DiffAwarenessManager.ProcessableModifiedFileSet;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.ExternalDirtinessChecker;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.MissingDiffDirtinessChecker;
@@ -86,6 +88,7 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.BuildDriver;
import com.google.devtools.build.skyframe.Differencer;
+import com.google.devtools.build.skyframe.Differencer.Diff;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.GraphInconsistencyReceiver;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
@@ -109,7 +112,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
@@ -149,7 +151,8 @@
private Duration sourceDiffCheckingDuration = Duration.ofSeconds(-1L);
private Duration outputTreeDiffCheckingDuration = Duration.ofSeconds(-1L);
- @Nullable private final WorkspaceFileHeaderListener workspaceFileHeaderListener;
+ // If this is null then workspace header pre-calculation won't happen.
+ @Nullable private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge;
private SequencedSkyframeExecutor(
Consumer<SkyframeExecutor> skyframeExecutorConsumerOnInit,
@@ -170,7 +173,7 @@
ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
BuildOptions defaultBuildOptions,
MutableArtifactFactorySupplier mutableArtifactFactorySupplier,
- @Nullable WorkspaceFileHeaderListener workspaceFileHeaderListener) {
+ @Nullable ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
super(
skyframeExecutorConsumerOnInit,
evaluatorSupplier,
@@ -193,10 +196,11 @@
new PackageProgressReceiver(),
mutableArtifactFactorySupplier,
new ConfiguredTargetProgressReceiver(),
- /*nonexistentFileReceiver=*/ null);
+ /*nonexistentFileReceiver=*/ null,
+ managedDirectoriesKnowledge);
this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
this.customDirtinessCheckers = customDirtinessCheckers;
- this.workspaceFileHeaderListener = workspaceFileHeaderListener;
+ this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
}
@Override
@@ -330,8 +334,10 @@
TimestampGranularityMonitor tsgm = this.tsgm.get();
modifiedFiles = 0;
- if (workspaceFileHeaderListener != null) {
- refreshWorkspaceHeader(eventHandler);
+ boolean managedDirectoriesChanged =
+ managedDirectoriesKnowledge != null && refreshWorkspaceHeader(eventHandler);
+ if (managedDirectoriesChanged) {
+ invalidateCachedWorkspacePathsStates();
}
Map<Root, DiffAwarenessManager.ProcessableModifiedFileSet> modifiedFilesByPathEntry =
@@ -347,12 +353,47 @@
modifiedFilesByPathEntry.put(pathEntry, modifiedFileSet);
}
}
- handleDiffsWithCompleteDiffInformation(tsgm, modifiedFilesByPathEntry);
- handleDiffsWithMissingDiffInformation(eventHandler, tsgm, pathEntriesWithoutDiffInformation,
- checkOutputFiles);
+ handleDiffsWithCompleteDiffInformation(
+ tsgm, modifiedFilesByPathEntry, managedDirectoriesChanged);
+ handleDiffsWithMissingDiffInformation(
+ eventHandler,
+ tsgm,
+ pathEntriesWithoutDiffInformation,
+ checkOutputFiles,
+ managedDirectoriesChanged);
handleClientEnvironmentChanges();
}
+ /**
+ * Skyframe caches the states of files (FileStateValue) and directory listings
+ * (DirectoryListingStateValue). In the case when the files/directories are under a managed
+ * directory or inside an external repository, evaluation of file/directory listing states
+ * requires that RepositoryDirectoryValue of the owning external repository is evaluated
+ * beforehand. (So that the repository rule generates the files.) So there is a dependency on
+ * RepositoryDirectoryValue for files under managed directories and external repositories. This
+ * dependency is recorded by Skyframe.
+ *
+ * <p>From the other side, by default Skyframe injects the new values of changed files already at
+ * the stage of checking what files have changed. Only values without any dependencies can be
+ * injected into Skyframe.
+ *
+ * <p>When the values of managed directories change, whether a file is under a managed directory
+ * can change. This implies that corresponding file/directory listing states gain the dependency
+ * (RepositoryDirectoryValue) or they lose this dependency. In both cases, we should prevent
+ * Skyframe from injecting those new values of file/directory listing states at the stage of
+ * checking changed files, because the files have not been generated yet.
+ *
+ * <p>The selected approach is to invalidate PACKAGE_LOCATOR_DEPENDENT_VALUES, which includes
+ * invalidating all cached file/directory listing state values. Additionally, no new
+ * FileStateValues and DirectoryStateValues should be injected.
+ */
+ private void invalidateCachedWorkspacePathsStates() {
+ logger.info(
+ "Invalidating cached packages and paths states"
+ + " because managed directories configuration has changed.");
+ invalidate(SkyFunctionName.functionIsIn(PACKAGE_LOCATOR_DEPENDENT_VALUES));
+ }
+
/** Invalidates entries in the client environment. */
private void handleClientEnvironmentChanges() {
// Remove deleted client environmental variables.
@@ -382,15 +423,18 @@
*/
private void handleDiffsWithCompleteDiffInformation(
TimestampGranularityMonitor tsgm,
- Map<Root, DiffAwarenessManager.ProcessableModifiedFileSet> modifiedFilesByPathEntry)
+ Map<Root, ProcessableModifiedFileSet> modifiedFilesByPathEntry,
+ boolean managedDirectoriesChanged)
throws InterruptedException {
for (Root pathEntry : ImmutableSet.copyOf(modifiedFilesByPathEntry.keySet())) {
DiffAwarenessManager.ProcessableModifiedFileSet processableModifiedFileSet =
modifiedFilesByPathEntry.get(pathEntry);
ModifiedFileSet modifiedFileSet = processableModifiedFileSet.getModifiedFileSet();
Preconditions.checkState(!modifiedFileSet.treatEverythingAsModified(), pathEntry);
- handleChangedFiles(ImmutableList.of(pathEntry),
- getDiff(tsgm, modifiedFileSet.modifiedSourceFiles(), pathEntry));
+ handleChangedFiles(
+ ImmutableList.of(pathEntry),
+ getDiff(tsgm, modifiedFileSet.modifiedSourceFiles(), pathEntry),
+ managedDirectoriesChanged);
processableModifiedFileSet.markProcessed();
}
}
@@ -402,9 +446,9 @@
private void handleDiffsWithMissingDiffInformation(
ExtendedEventHandler eventHandler,
TimestampGranularityMonitor tsgm,
- Set<Pair<Root, DiffAwarenessManager.ProcessableModifiedFileSet>>
- pathEntriesWithoutDiffInformation,
- boolean checkOutputFiles)
+ Set<Pair<Root, ProcessableModifiedFileSet>> pathEntriesWithoutDiffInformation,
+ boolean checkOutputFiles,
+ boolean managedDirectoriesChanged)
throws InterruptedException {
ExternalFilesKnowledge externalFilesKnowledge =
externalFilesHelper.getExternalFilesKnowledge();
@@ -464,7 +508,7 @@
new ExternalDirtinessChecker(tmpExternalFilesHelper, fileTypesToCheck),
new MissingDiffDirtinessChecker(diffPackageRootsUnderWhichToCheck)))));
}
- handleChangedFiles(diffPackageRootsUnderWhichToCheck, diff);
+ handleChangedFiles(diffPackageRootsUnderWhichToCheck, diff, managedDirectoriesChanged);
for (Pair<Root, DiffAwarenessManager.ProcessableModifiedFileSet> pair :
pathEntriesWithoutDiffInformation) {
@@ -478,18 +522,33 @@
}
private void handleChangedFiles(
- Collection<Root> diffPackageRootsUnderWhichToCheck, Differencer.Diff diff) {
- Collection<SkyKey> changedKeysWithoutNewValues = diff.changedKeysWithoutNewValues();
+ Collection<Root> diffPackageRootsUnderWhichToCheck,
+ Diff diff,
+ boolean managedDirectoriesChanged) {
+ int numWithoutNewValues = diff.changedKeysWithoutNewValues().size();
+ Iterable<SkyKey> keysToBeChangedLaterInThisBuild = diff.changedKeysWithoutNewValues();
Map<SkyKey, SkyValue> changedKeysWithNewValues = diff.changedKeysWithNewValues();
- logDiffInfo(diffPackageRootsUnderWhichToCheck, changedKeysWithoutNewValues,
+ // If managed directories settings changed, do not inject any new values, just invalidate
+ // keys of the changed values. {@link #invalidateCachedWorkspacePathsStates()}
+ if (managedDirectoriesChanged) {
+ numWithoutNewValues += changedKeysWithNewValues.size();
+ keysToBeChangedLaterInThisBuild =
+ Iterables.concat(keysToBeChangedLaterInThisBuild, changedKeysWithNewValues.keySet());
+ changedKeysWithNewValues = ImmutableMap.of();
+ }
+
+ logDiffInfo(
+ diffPackageRootsUnderWhichToCheck,
+ keysToBeChangedLaterInThisBuild,
+ numWithoutNewValues,
changedKeysWithNewValues);
- recordingDiffer.invalidate(changedKeysWithoutNewValues);
+ recordingDiffer.invalidate(keysToBeChangedLaterInThisBuild);
recordingDiffer.inject(changedKeysWithNewValues);
- modifiedFiles += getNumberOfModifiedFiles(changedKeysWithoutNewValues);
+ modifiedFiles += getNumberOfModifiedFiles(keysToBeChangedLaterInThisBuild);
modifiedFiles += getNumberOfModifiedFiles(changedKeysWithNewValues.keySet());
- incrementalBuildMonitor.accrue(changedKeysWithoutNewValues);
+ incrementalBuildMonitor.accrue(keysToBeChangedLaterInThisBuild);
incrementalBuildMonitor.accrue(changedKeysWithNewValues.keySet());
}
@@ -497,9 +556,10 @@
private static void logDiffInfo(
Iterable<Root> pathEntries,
- Collection<SkyKey> changedWithoutNewValue,
+ Iterable<SkyKey> changedWithoutNewValue,
+ int numWithoutNewValues,
Map<SkyKey, ? extends SkyValue> changedWithNewValue) {
- int numModified = changedWithNewValue.size() + changedWithoutNewValue.size();
+ int numModified = changedWithNewValue.size() + numWithoutNewValues;
StringBuilder result = new StringBuilder("DiffAwareness found ")
.append(numModified)
.append(" modified source files and directory listings");
@@ -826,10 +886,8 @@
* and call the listener, if the value has changed. Needed for incremental update of user-owned
* directories by repository rules.
*/
- private void refreshWorkspaceHeader(ExtendedEventHandler eventHandler)
+ private boolean refreshWorkspaceHeader(ExtendedEventHandler eventHandler)
throws InterruptedException, AbruptExitException {
- Preconditions.checkNotNull(workspaceFileHeaderListener);
-
Root workspaceRoot = Root.fromPath(directories.getWorkspace());
RootedPath workspacePath =
RootedPath.toRootedPath(workspaceRoot, LabelConstants.WORKSPACE_FILE_NAME);
@@ -840,9 +898,7 @@
maybeInvalidateWorkspaceFileStateValue(workspacePath);
WorkspaceFileValue newValue =
(WorkspaceFileValue) evaluateSingleValue(workspaceFileKey, eventHandler);
- if (!Objects.equals(newValue, oldValue)) {
- workspaceFileHeaderListener.workspaceHeaderChanged(newValue);
- }
+ return managedDirectoriesKnowledge.workspaceHeaderReloaded(oldValue, newValue);
}
// We only check the FileStateValue of the WORKSPACE file; we do not support the case
@@ -901,13 +957,12 @@
protected ActionKeyContext actionKeyContext;
protected ImmutableList<BuildInfoFactory> buildInfoFactories;
protected BuildOptions defaultBuildOptions;
-
private ImmutableSet<PathFragment> hardcodedBlacklistedPackagePrefixes;
private PathFragment additionalBlacklistedPackagePrefixesFile;
private CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy;
private List<BuildFileName> buildFilesByPriority;
private ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile;
- private WorkspaceFileHeaderListener workspaceFileHeaderListener;
+ @Nullable private ManagedDirectoriesKnowledge managedDirectoriesKnowledge;
// Fields with default values.
private ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions = ImmutableMap.of();
@@ -954,16 +1009,11 @@
actionOnIOExceptionReadingBuildFile,
defaultBuildOptions,
mutableArtifactFactorySupplier,
- workspaceFileHeaderListener);
+ managedDirectoriesKnowledge);
skyframeExecutor.init();
return skyframeExecutor;
}
- public Builder modify(Consumer<Builder> consumer) {
- consumer.accept(this);
- return this;
- }
-
public Builder setPkgFactory(PackageFactory pkgFactory) {
this.pkgFactory = pkgFactory;
return this;
@@ -1051,27 +1101,28 @@
this.mutableArtifactFactorySupplier = mutableArtifactFactorySupplier;
return this;
}
-
public Builder setSkyframeExecutorConsumerOnInit(
Consumer<SkyframeExecutor> skyframeExecutorConsumerOnInit) {
this.skyframeExecutorConsumerOnInit = skyframeExecutorConsumerOnInit;
return this;
}
- public Builder setWorkspaceFileHeaderListener(
- WorkspaceFileHeaderListener workspaceFileHeaderListener) {
- this.workspaceFileHeaderListener = workspaceFileHeaderListener;
+ public Builder setManagedDirectoriesKnowledge(
+ @Nullable ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
+ this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
return this;
}
}
/**
- * Listener class to subscribe for WORKSPACE file header changes.
+ * Listener class to subscribe for WORKSPACE file header refreshes.
*
* <p>Changes to WORKSPACE file header are computed before the files difference is computed in
* {@link #handleDiffs(ExtendedEventHandler, boolean, OptionsProvider)}
*/
public interface WorkspaceFileHeaderListener {
- void workspaceHeaderChanged(@Nullable WorkspaceFileValue newValue);
+ boolean workspaceHeaderReloaded(
+ @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue)
+ throws AbruptExitException;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
index ca668ea..33f5a61 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
@@ -21,7 +21,7 @@
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.packages.PackageFactory;
-import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -49,7 +49,7 @@
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
Iterable<SkyValueDirtinessChecker> customDirtinessCheckers,
- @Nullable WorkspaceFileHeaderListener workspaceFileHeaderListener) {
+ @Nullable ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
return BazelSkyframeExecutorConstants.newBazelSkyframeExecutorBuilder()
.setPkgFactory(pkgFactory)
.setFileSystem(fileSystem)
@@ -61,7 +61,7 @@
.setDiffAwarenessFactories(diffAwarenessFactories)
.setExtraSkyFunctions(extraSkyFunctions)
.setCustomDirtinessCheckers(customDirtinessCheckers)
- .setWorkspaceFileHeaderListener(workspaceFileHeaderListener)
+ .setManagedDirectoriesKnowledge(managedDirectoriesKnowledge)
.build();
}
}
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 e89873b..ced09be 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
@@ -70,6 +70,7 @@
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
+import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -128,6 +129,7 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.rules.repository.ResolvedFileFunction;
import com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction;
import com.google.devtools.build.lib.runtime.KeepGoingOption;
@@ -381,7 +383,7 @@
FileSystem fileSystem,
BlazeDirectories directories,
ActionKeyContext actionKeyContext,
- WorkspaceStatusAction.Factory workspaceStatusActionFactory,
+ Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
ExternalFileAction externalFileAction,
@@ -396,7 +398,8 @@
@Nullable PackageProgressReceiver packageProgress,
MutableArtifactFactorySupplier artifactResolverSupplier,
@Nullable ConfiguredTargetProgressReceiver configuredTargetProgress,
- @Nullable NonexistentFileReceiver nonexistentFileReceiver) {
+ @Nullable NonexistentFileReceiver nonexistentFileReceiver,
+ @Nullable ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
// Strictly speaking, these arguments are not required for initialization, but all current
// callsites have them at hand, so we might as well set them during construction.
this.skyframeExecutorConsumerOnInit = skyframeExecutorConsumerOnInit;
@@ -443,7 +446,13 @@
this.artifactFactory = artifactResolverSupplier;
this.artifactFactory.set(skyframeBuildView.getArtifactFactory());
this.externalFilesHelper =
- ExternalFilesHelper.create(pkgLocator, externalFileAction, directories);
+ ExternalFilesHelper.create(
+ pkgLocator,
+ externalFileAction,
+ directories,
+ managedDirectoriesKnowledge != null
+ ? managedDirectoriesKnowledge
+ : ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES);
this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy;
this.buildFilesByPriority = buildFilesByPriority;
this.actionOnIOExceptionReadingBuildFile = actionOnIOExceptionReadingBuildFile;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
index 504e4a4..e7c7106 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
@@ -20,7 +20,7 @@
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.packages.PackageFactory;
-import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -52,6 +52,6 @@
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
Iterable<SkyValueDirtinessChecker> customDirtinessCheckers,
- WorkspaceFileHeaderListener workspaceFileHeaderListener)
+ ManagedDirectoriesKnowledge managedDirectoriesKnowledge)
throws AbruptExitException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 240eeec..69072d4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -43,6 +43,7 @@
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.skyframe.ASTFileLookupFunction;
import com.google.devtools.build.lib.skyframe.BlacklistedPackagePrefixesFunction;
import com.google.devtools.build.lib.skyframe.ContainingPackageLookupFunction;
@@ -232,7 +233,11 @@
public final PackageLoader build() {
validate();
externalFilesHelper =
- ExternalFilesHelper.create(pkgLocatorRef, externalFileAction, directories);
+ ExternalFilesHelper.create(
+ pkgLocatorRef,
+ externalFileAction,
+ directories,
+ ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES);
return buildImpl();
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index cc6615d..f7f44d9 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -132,6 +132,7 @@
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
@@ -141,7 +142,6 @@
import com.google.devtools.build.lib.skyframe.PackageRootsNoSymlinkCreation;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
-import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
@@ -281,7 +281,7 @@
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
- .setWorkspaceFileHeaderListener(getWorkspaceFileListener())
+ .setManagedDirectoriesKnowledge(getManagedDirectoriesKnowledge())
.build();
TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
skyframeExecutor.injectExtraPrecomputedValues(extraPrecomputedValues);
@@ -322,7 +322,7 @@
return getAnalysisMock().createRuleClassProvider();
}
- protected WorkspaceFileHeaderListener getWorkspaceFileListener() {
+ protected ManagedDirectoriesKnowledge getManagedDirectoriesKnowledge() {
return null;
}
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD b/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD
index e1a1f82..ebb2110 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD
@@ -21,6 +21,24 @@
)
java_test(
+ name = "ManagedDirectoriesBlackBoxTest",
+ timeout = "moderate",
+ srcs = ["manageddirs/ManagedDirectoriesBlackBoxTest.java"],
+ resources = [
+ "manageddirs/.bazelignore",
+ "manageddirs/BUILD.test",
+ "manageddirs/WORKSPACE.test",
+ "manageddirs/package.json",
+ "manageddirs/test_rule.bzl",
+ "manageddirs/use_node_modules.bzl",
+ ],
+ tags = ["black_box_test"],
+ deps = [
+ ":common_deps",
+ ],
+)
+
+java_test(
name = "PythonBlackBoxTest",
timeout = "moderate",
srcs = ["PythonBlackBoxTest.java"],
@@ -35,6 +53,7 @@
name = "black_box_tests",
tags = ["black_box_test"],
tests = [
+ "ManagedDirectoriesBlackBoxTest",
"PythonBlackBoxTest",
"//src/test/java/com/google/devtools/build/lib/blackbox/framework:framework_tests",
"//src/test/java/com/google/devtools/build/lib/blackbox/junit:TimeoutTestWatcherTest",
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/.bazelignore b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/.bazelignore
new file mode 100644
index 0000000..b512c09
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/.bazelignore
@@ -0,0 +1 @@
+node_modules
\ No newline at end of file
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test
new file mode 100644
index 0000000..175527c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test
@@ -0,0 +1,20 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+load(":test_rule.bzl", "test_rule")
+
+test_rule(
+ name = "test_generated_deps",
+ module_source = "@generated_node_modules//:example-module",
+ version = "0.2.0"
+)
\ No newline at end of file
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java
new file mode 100644
index 0000000..28fa0c7
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java
@@ -0,0 +1,405 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.blackbox.tests.manageddirs;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.blackbox.framework.BuilderRunner;
+import com.google.devtools.build.lib.blackbox.framework.PathUtils;
+import com.google.devtools.build.lib.blackbox.framework.ProcessResult;
+import com.google.devtools.build.lib.blackbox.junit.AbstractBlackBoxTest;
+import com.google.devtools.build.lib.util.ResourceFileLoader;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests for managed directories. */
+public class ManagedDirectoriesBlackBoxTest extends AbstractBlackBoxTest {
+ private static final List<String> FILES =
+ Lists.newArrayList(
+ "BUILD.test",
+ "WORKSPACE.test",
+ ".bazelignore",
+ "package.json",
+ "test_rule.bzl",
+ "use_node_modules.bzl");
+ private Random random;
+ private Integer currentDebugId;
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ random = new Random(17);
+ super.setUp();
+ }
+
+ @Test
+ public void testBuildProject() throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testBuildProjectFetchNotRecalled() throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+ buildExpectRepositoryRuleNotCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testWithoutFlag() throws Exception {
+ generateProject();
+ ProcessResult result = context().bazel().shouldFail().build("//...");
+ assertThat(result.errString())
+ .contains(
+ "parameter 'managed_directories' is experimental and thus unavailable"
+ + " with the current flags.");
+ }
+
+ private BuilderRunner bazel() {
+ return bazel(false);
+ }
+
+ private BuilderRunner bazel(boolean watchFs) {
+ currentDebugId = random.nextInt();
+ String[] flags =
+ watchFs
+ ? new String[] {"--experimental_allow_incremental_repository_updates", "--watchfs=true"}
+ : new String[] {"--experimental_allow_incremental_repository_updates"};
+ return context().bazel().withFlags(flags).withEnv("DEBUG_ID", String.valueOf(currentDebugId));
+ }
+
+ @Test
+ public void testChangeOfFileTextUnderNodeModules() throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+
+ Path nodeModules = context().getWorkDir().resolve("node_modules");
+ Path modulePackageJson = nodeModules.resolve("example-module/package.json");
+ assertThat(modulePackageJson.toFile().exists()).isTrue();
+
+ // Assert that non-structural changes are not detected.
+ PathUtils.append(modulePackageJson, "# comment");
+
+ buildExpectRepositoryRuleNotCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testLoadIsNotCalledForManagedDirectories() throws Exception {
+ generateProject();
+ Path workspaceFile = context().getWorkDir().resolve(WORKSPACE);
+ PathUtils.append(workspaceFile, "load('@non_existing//:target.bzl', 'some_symbol')");
+
+ // Test that there is error when loading, so we parsed managed directories successfully.
+ ProcessResult result = bazel().shouldFail().build("//...");
+ assertThat(findPattern(result, "ERROR: Failed to load Starlark extension")).isTrue();
+ }
+
+ @Test
+ public void testWithBazelTools() throws Exception {
+ generateProject();
+ Path workspaceFile = context().getWorkDir().resolve(WORKSPACE);
+ PathUtils.append(
+ workspaceFile,
+ "load(\"@bazel_tools//tools/build_defs/repo:http.bzl\", \"http_archive\", \"http_file\")");
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testAddManagedDirectoriesLater() throws Exception {
+ // Start the server, have things cached.
+ context().write("BUILD", "");
+ bazel().build("//...");
+
+ // Now that we generate the project and have managed directories updated, we are also testing,
+ // that managed directories are re-read correctly from the changed file.
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+
+ // Test everything got cached.
+ buildExpectRepositoryRuleNotCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testFilesUnderChangedManagedDirectoriesRefreshed() throws Exception {
+ doTestFilesUnderManagedDirectoriesRefreshed(false);
+ }
+
+ @Test
+ public void testFilesUnderChangedManagedDirectoriesRefreshedWatchFs() throws Exception {
+ doTestFilesUnderManagedDirectoriesRefreshed(true);
+ }
+
+ private void doTestFilesUnderManagedDirectoriesRefreshed(boolean watchFs) throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled(false, watchFs);
+ checkProjectFiles();
+
+ // Now remove the ManagedDirectories, and change the package version - it should still work.
+ List<String> properWorkspaceText = context().read("WORKSPACE");
+
+ context()
+ .write(
+ "WORKSPACE",
+ "workspace(name = \"fine_grained_user_modules\")",
+ "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")",
+ "generate_fine_grained_node_modules(name = \"generated_node_modules\",",
+ "package_json = \"//:package.json\",)");
+ Path packageJson =
+ PathUtils.resolve(context().getWorkDir(), "node_modules", "example-module", "package.json");
+ assertThat(packageJson.toFile().exists()).isTrue();
+
+ // Now we are building it without managed directories, both managed directories and
+ // RepositoryDirectoryValue will be dirty - we expect repository rule to be called again.
+ buildExpectRepositoryRuleCalled(false, watchFs);
+ checkProjectFiles();
+
+ // Now change files directly in generated area, and build.
+ List<String> oldPackageJson = PathUtils.readFile(packageJson);
+ PathUtils.writeFile(
+ packageJson,
+ "{",
+ " \"license\": \"MIT\",",
+ " \"main\": \"example-module.js\",",
+ " \"name\": \"example-module\",",
+ " \"repository\": {",
+ " \"type\": \"git\",",
+ " \"url\": \"aaa\",",
+ " },",
+ " \"version\": \"7.7.7\"",
+ "}");
+ Path build = context().getWorkDir().resolve("BUILD");
+ List<String> oldBuild = PathUtils.readFile(build);
+ PathUtils.writeFile(
+ build,
+ "load(\":test_rule.bzl\", \"test_rule\")",
+ "test_rule(",
+ " name = \"test_generated_deps\",",
+ " module_source = \"@generated_node_modules//:example-module\",",
+ " version = \"7.7.7\"",
+ ")");
+
+ // Test rule inputs has changed, so the build is not cached; however, the repository rule
+ // is not rerun, since it's inputs (including managed directories settings) were not changed,
+ // so debug_id is the same.
+ buildExpectRepositoryRuleNotCalled();
+ checkProjectFiles("7.7.7");
+
+ // And is cached.
+ buildExpectRepositoryRuleNotCalled();
+
+ // Now change just the managed directories and see the generated version comes up.
+ PathUtils.writeFile(
+ context().getWorkDir().resolve(WORKSPACE), properWorkspaceText.toArray(new String[0]));
+ PathUtils.writeFile(build, oldBuild.toArray(new String[0]));
+ buildExpectRepositoryRuleCalled(false, watchFs);
+ checkProjectFiles("0.2.0");
+ }
+
+ @Test
+ public void testManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneously()
+ throws Exception {
+ doTestManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneously(false);
+ }
+
+ @Test
+ public void testManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneouslyWatchFs()
+ throws Exception {
+ doTestManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneously(true);
+ }
+
+ private void doTestManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneously(
+ boolean watchFs) throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled(false, watchFs);
+ checkProjectFiles();
+
+ // Modify managed directories somehow.
+ context()
+ .write(
+ "WORKSPACE",
+ "workspace(name = \"fine_grained_user_modules\",",
+ "managed_directories = {'@generated_node_modules': ['node_modules', 'something']})",
+ "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")",
+ "generate_fine_grained_node_modules(name = \"generated_node_modules\",",
+ "package_json = \"//:package.json\",)");
+ Path packageJson =
+ PathUtils.resolve(context().getWorkDir(), "node_modules", "example-module", "package.json");
+ assertThat(packageJson.toFile().exists()).isTrue();
+
+ // Modify generated package.json under the managed directory.
+ List<String> oldPackageJson = PathUtils.readFile(packageJson);
+ PathUtils.writeFile(
+ packageJson,
+ "{",
+ " \"license\": \"MIT\",",
+ " \"main\": \"example-module.js\",",
+ " \"name\": \"example-module\",",
+ " \"repository\": {",
+ " \"type\": \"git\",",
+ " \"url\": \"aaa\",",
+ " },",
+ " \"version\": \"7.7.7\"",
+ "}");
+ // Expect files under managed directories be regenerated
+ // and changes under managed directories be lost.
+ buildExpectRepositoryRuleCalled(false, watchFs);
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testRepositoryOverrideWithManagedDirectories() throws Exception {
+ generateProject();
+
+ Path override = context().getTmpDir().resolve("override");
+ PathUtils.writeFile(override.resolve(WORKSPACE));
+ // Just define some similar target.
+ PathUtils.writeFile(
+ override.resolve("BUILD"),
+ "genrule(",
+ " name = \"example-module\",",
+ " srcs = [],",
+ " cmd = \"touch $(location package.json)\",",
+ " outs = [\"package.json\"],",
+ " visibility = ['//visibility:public'],",
+ ")");
+
+ BuilderRunner bazel =
+ bazel().withFlags("--override_repository=generated_node_modules=" + override.toString());
+ ProcessResult result = bazel.shouldFail().build("@generated_node_modules//:example-module");
+ assertThat(result.errString())
+ .contains(
+ "ERROR: Overriding repositories is not allowed"
+ + " for the repositories with managed directories."
+ + "\nThe following overridden external repositories"
+ + " have managed directories: @generated_node_modules");
+
+ // Assert the result stays the same even when managed directories has not changed.
+ result = bazel.shouldFail().build("@generated_node_modules//:example-module");
+ assertThat(result.errString())
+ .contains(
+ "ERROR: Overriding repositories is not allowed"
+ + " for the repositories with managed directories."
+ + "\nThe following overridden external repositories"
+ + " have managed directories: @generated_node_modules");
+ }
+
+ private void generateProject() throws IOException {
+ for (String fileName : FILES) {
+ String text = ResourceFileLoader.loadResource(ManagedDirectoriesBlackBoxTest.class, fileName);
+ assertThat(text).isNotNull();
+ assertThat(text).isNotEmpty();
+ fileName =
+ fileName.endsWith(".test") ? fileName.substring(0, fileName.length() - 5) : fileName;
+ context().write(fileName, text);
+ }
+ }
+
+ private void checkProjectFiles() throws IOException {
+ checkProjectFiles("0.2.0");
+ }
+
+ private void checkProjectFiles(String version) throws IOException {
+ Path nodeModules = context().getWorkDir().resolve("node_modules");
+ assertThat(nodeModules.toFile().exists()).isTrue();
+ assertThat(nodeModules.toFile().isDirectory()).isTrue();
+
+ Path exampleModule = nodeModules.resolve("example-module");
+ assertThat(exampleModule.toFile().exists()).isTrue();
+ assertThat(exampleModule.toFile().isDirectory()).isTrue();
+
+ Path packageJson = exampleModule.resolve("package.json");
+ assertThat(packageJson.toFile().exists()).isTrue();
+ assertThat(packageJson.toFile().isDirectory()).isFalse();
+
+ List<String> text = PathUtils.readFile(packageJson);
+ assertThat(text.stream().anyMatch(s -> s.trim().equals("\"name\": \"example-module\",")))
+ .isTrue();
+ String versionString = String.format("\"version\": \"%s\"", version);
+ assertThat(text.stream().anyMatch(s -> s.trim().equals(versionString))).isTrue();
+ }
+
+ private String getDebugId(BuilderRunner bazel) throws Exception {
+ Path path = context().resolveExecRootPath(bazel, "external/generated_node_modules/debug_id");
+ List<String> lines = PathUtils.readFile(path);
+ assertThat(lines.size()).isEqualTo(1);
+ return lines.get(0);
+ }
+
+ private ProcessResult buildExpectRepositoryRuleCalled() throws Exception {
+ return buildExpectRepositoryRuleCalled(false, false);
+ }
+
+ private ProcessResult buildExpectRepositoryRuleCalled(boolean debug, boolean watchFs)
+ throws Exception {
+ BuilderRunner bazel = bazel(watchFs);
+ if (debug) {
+ bazel.enableDebug();
+ }
+ ProcessResult result = bazel.build("//...");
+ buildSucceeded(result);
+ debugIdShouldBeUpdated(bazel);
+ return result;
+ }
+
+ private ProcessResult buildExpectRepositoryRuleNotCalled() throws Exception {
+ return buildExpectRepositoryRuleNotCalled(false);
+ }
+
+ private ProcessResult buildExpectRepositoryRuleNotCalled(boolean debug) throws Exception {
+ BuilderRunner bazel = bazel();
+ if (debug) {
+ bazel.enableDebug();
+ }
+ ProcessResult result = bazel.build("//...");
+ buildSucceeded(result);
+ debugIdShouldNotBeUpdated(bazel);
+ return result;
+ }
+
+ private void debugIdShouldBeUpdated(BuilderRunner bazel) throws Exception {
+ assertThat(getDebugId(bazel)).isEqualTo(String.valueOf(currentDebugId));
+ }
+
+ private void debugIdShouldNotBeUpdated(BuilderRunner bazel) throws Exception {
+ assertThat(getDebugId(bazel)).isNotEqualTo(String.valueOf(currentDebugId));
+ }
+
+ private void buildSucceeded(ProcessResult result) {
+ assertThat(findPattern(result, "INFO: Build completed successfully")).isTrue();
+ }
+
+ private void buildFailed(ProcessResult result) {
+ assertThat(findPattern(result, "FAILED: Build did NOT complete successfully")).isTrue();
+ }
+
+ private boolean findPattern(ProcessResult result, String pattern) {
+ String[] lines = result.errString().split("\n");
+ return Arrays.stream(lines).anyMatch(s -> s.contains(pattern));
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test
new file mode 100644
index 0000000..bf0f85b
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test
@@ -0,0 +1,22 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+workspace(name = "fine_grained_user_modules",
+managed_directories = {'@generated_node_modules': ['node_modules']})
+
+load(":use_node_modules.bzl", "generate_fine_grained_node_modules")
+
+generate_fine_grained_node_modules(
+ name = "generated_node_modules",
+ package_json = "//:package.json",
+)
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json
new file mode 100644
index 0000000..abcb5a5
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json
@@ -0,0 +1,8 @@
+{
+ "name": "fine_grained_user_modules",
+ "version": "0.0.1",
+ "description": "prototype for using user's node_modules",
+ "dependencies": {
+ "example-module": "0.2.0"
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl
new file mode 100644
index 0000000..02c57aa
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl
@@ -0,0 +1,49 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+""" Test rule for testing results of NPM package manager interaction mock
+"""
+
+def _test_rule(ctx):
+ out = ctx.actions.declare_file("out.txt")
+ files = ctx.attr.module_source.files
+ found = False
+ for file_ in files:
+ if file_.basename == "package.json":
+ compare_version(ctx.actions, file_, out, ctx.attr.version)
+ found = True
+ break
+ if not found:
+ fail("Not found package.json")
+ return [DefaultInfo(files = depset([out]))]
+
+test_rule = rule(
+ implementation = _test_rule,
+ attrs = {
+ "module_source": attr.label(),
+ "version": attr.string(),
+ },
+)
+
+def compare_version(action_factory, file_, out, expected_version):
+ action_factory.run_shell(
+ mnemonic = "getVersion",
+ inputs = [file_],
+ outputs = [out],
+ command = """result=$(cat ./{file} | grep '"version": "{expected}"' || exit 1) \
+&& echo $result > ./{out}""".format(
+ file = file_.path,
+ out = out.path,
+ expected = expected_version,
+ ),
+ )
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl
new file mode 100644
index 0000000..27b4257
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl
@@ -0,0 +1,93 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+""" NPM package manager interaction mock
+"""
+
+def _generate_fine_grained_node_modules(rctx):
+ print("--fetch--")
+ package_json_path = rctx.path(rctx.attr.package_json)
+
+ rctx.file(
+ "script.sh",
+ """
+root_dir="$1/node_modules"
+mkdir $root_dir
+mkdir $root_dir/example-module
+cat > "$root_dir/example-module/package.json" <<EOF
+{
+ "license": "MIT",
+ "main": "example-module.js",
+ "name": "example-module",
+ "repository": {
+ "type": "git",
+ "url": "aaa"
+ },
+ "version": "0.2.0"
+}
+EOF
+echo "ok"
+""",
+ executable = True,
+ )
+ cmd(rctx, "echo $DEBUG_ID > debug_id && echo 'ok'")
+ cmd(rctx, "./script.sh " + str(package_json_path.dirname))
+
+ node_modules_root = rctx.path(str(package_json_path) + "/../node_modules")
+ build_file_lines = []
+ for path_ in node_modules_root.readdir():
+ name = path_.basename
+ rctx.symlink(path_, rctx.path(name))
+ build_file_lines += ["""filegroup(name = "{name}", srcs = glob(["{name}/**"]), visibility = ["//visibility:public"])""".format(name = name)]
+
+ rctx.file("BUILD", "\n".join(build_file_lines))
+
+generate_fine_grained_node_modules = repository_rule(
+ implementation = _generate_fine_grained_node_modules,
+ attrs = {
+ "package_json": attr.label(),
+ },
+)
+
+def cmd(
+ repository_ctx,
+ command,
+ environment = None):
+ """Executes a command, returns stdout if succeed and throw an error if it fails.
+
+ Doesn't escape the result!
+
+ Args:
+ repository_ctx: repository context
+ command: command parts array
+ environment: dict with environment variables
+
+ Returns:
+ process stdout as a string
+ """
+ if environment:
+ result = repository_ctx.execute(["bash", "-c", command], environment = environment)
+ else:
+ result = repository_ctx.execute(["bash", "-c", command])
+ if result.return_code != 0:
+ fail("non-zero exit code: %d, command %s, stderr: (%s)" % (
+ result.return_code,
+ command,
+ result.stderr,
+ ))
+ stripped_stdout = result.stdout.strip()
+ if not stripped_stdout:
+ fail(
+ "empty output from command %s, stderr: (%s)" % (command, result.stderr),
+ )
+ return stripped_stdout
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
index c728520..f693bea 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.repository;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import com.google.common.base.Optional;
@@ -97,7 +98,7 @@
private RepositoryDelegatorFunction delegatorFunction;
private Path overrideDirectory;
private SequentialBuildDriver driver;
- private ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge;
+ private TestManagedDirectoriesKnowledge managedDirectoriesKnowledge;
private RecordingDifferencer differencer;
private TestSkylarkRepositoryFunction testSkylarkRepositoryFunction;
private Path rootPath;
@@ -111,7 +112,7 @@
rootPath,
/* defaultSystemJavabase= */ null,
TestConstants.PRODUCT_NAME);
- managedDirectoriesKnowledge = new ManagedDirectoriesKnowledgeImpl();
+ managedDirectoriesKnowledge = new TestManagedDirectoriesKnowledge();
HttpDownloader downloader = Mockito.mock(HttpDownloader.class);
RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction();
testSkylarkRepositoryFunction = new TestSkylarkRepositoryFunction(downloader);
@@ -239,7 +240,7 @@
@Test
public void testRepositoryDirtinessChecker() throws Exception {
TimestampGranularityMonitor tsgm = new TimestampGranularityMonitor(new ManualClock());
- ManagedDirectoriesKnowledgeImpl knowledge = new ManagedDirectoriesKnowledgeImpl();
+ TestManagedDirectoriesKnowledge knowledge = new TestManagedDirectoriesKnowledge();
RepositoryDirectoryDirtinessChecker checker =
new RepositoryDirectoryDirtinessChecker(knowledge);
@@ -380,4 +381,32 @@
return super.fetch(rule, outputDirectory, directories, env, markerData, key);
}
}
+
+ private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge {
+
+ private ImmutableMap<PathFragment, RepositoryName> map = ImmutableMap.of();
+
+ public void setManagedDirectories(ImmutableMap<PathFragment, RepositoryName> map) {
+ this.map = map;
+ }
+
+ @Nullable
+ @Override
+ public RepositoryName getOwnerRepository(PathFragment relativePathFragment) {
+ return map.get(relativePathFragment);
+ }
+
+ @Override
+ public ImmutableSet<PathFragment> getManagedDirectories(RepositoryName repositoryName) {
+ return map.keySet().stream()
+ .filter(path -> repositoryName.equals(map.get(path)))
+ .collect(toImmutableSet());
+ }
+
+ @Override
+ public boolean workspaceHeaderReloaded(
+ @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) {
+ throw new IllegalStateException();
+ }
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index a69e001..8dcdf8e 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -20,11 +20,13 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
@@ -34,8 +36,10 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl;
+import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl.ManagedDirectoriesListener;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
-import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor.WorkspaceFileHeaderListener;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.testutil.MoreAsserts;
@@ -52,6 +56,8 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
+import java.util.Objects;
+import java.util.Set;
import javax.annotation.Nullable;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
@@ -74,7 +80,7 @@
private ExternalPackageFunction externalSkyFunc;
private WorkspaceASTFunction astSkyFunc;
private FakeFileValue fakeWorkspaceFileValue;
- private TestWorkspaceFileListener testWorkspaceFileListener;
+ private TestManagedDirectoriesKnowledge testManagedDirectoriesKnowledge;
static class FakeFileValue extends FileValue {
private boolean exists;
@@ -136,9 +142,9 @@
}
@Override
- protected WorkspaceFileHeaderListener getWorkspaceFileListener() {
- testWorkspaceFileListener = new TestWorkspaceFileListener();
- return testWorkspaceFileListener;
+ protected ManagedDirectoriesKnowledge getManagedDirectoriesKnowledge() {
+ testManagedDirectoriesKnowledge = new TestManagedDirectoriesKnowledge();
+ return testManagedDirectoriesKnowledge;
}
@Override
@@ -309,6 +315,67 @@
}
@Test
+ public void setTestManagedDirectoriesKnowledge() throws Exception {
+ PrecomputedValue precomputedValue =
+ (PrecomputedValue)
+ getEnv().getValue(PrecomputedValue.STARLARK_SEMANTICS.getKeyForTesting());
+ StarlarkSemantics semantics =
+ (StarlarkSemantics) Preconditions.checkNotNull(precomputedValue).get();
+ Injectable injectable = getSkyframeExecutor().injectable();
+ try {
+ StarlarkSemantics semanticsWithManagedDirectories =
+ StarlarkSemantics.builderWithDefaults()
+ .experimentalAllowIncrementalRepositoryUpdates(true)
+ .build();
+ PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semanticsWithManagedDirectories);
+
+ TestManagedDirectoriesListener listener = new TestManagedDirectoriesListener();
+ ManagedDirectoriesKnowledgeImpl knowledge = new ManagedDirectoriesKnowledgeImpl(listener);
+
+ RepositoryName one = RepositoryName.create("@repo1");
+ RepositoryName two = RepositoryName.create("@repo2");
+ RepositoryName three = RepositoryName.create("@repo3");
+
+ PathFragment pf1 = PathFragment.create("dir1");
+ PathFragment pf2 = PathFragment.create("dir2");
+ PathFragment pf3 = PathFragment.create("dir3");
+
+ assertThat(knowledge.getManagedDirectories(one)).isEmpty();
+ assertThat(knowledge.getOwnerRepository(pf1)).isNull();
+
+ WorkspaceFileValue workspaceFileValue = createWorkspaceFileValueForTest();
+ boolean isChanged = knowledge.workspaceHeaderReloaded(null, workspaceFileValue);
+
+ assertThat(isChanged).isTrue();
+ assertThat(listener.getRepositoryNames()).containsExactly(one, two);
+
+ assertThat(knowledge.getManagedDirectories(one)).containsExactly(pf1, pf2);
+ assertThat(knowledge.getManagedDirectories(two)).containsExactly(pf3);
+ assertThat(knowledge.getManagedDirectories(three)).isEmpty();
+
+ assertThat(knowledge.getOwnerRepository(pf1)).isEqualTo(one);
+ assertThat(knowledge.getOwnerRepository(pf2)).isEqualTo(one);
+ assertThat(knowledge.getOwnerRepository(pf3)).isEqualTo(two);
+
+ // Nothing changed, let's test the behavior.
+ listener.reset();
+ isChanged = knowledge.workspaceHeaderReloaded(workspaceFileValue, workspaceFileValue);
+ assertThat(isChanged).isFalse();
+ assertThat(listener.getRepositoryNames()).containsExactly(one, two);
+
+ assertThat(knowledge.getManagedDirectories(one)).containsExactly(pf1, pf2);
+ assertThat(knowledge.getManagedDirectories(two)).containsExactly(pf3);
+ assertThat(knowledge.getManagedDirectories(three)).isEmpty();
+
+ assertThat(knowledge.getOwnerRepository(pf1)).isEqualTo(one);
+ assertThat(knowledge.getOwnerRepository(pf2)).isEqualTo(one);
+ assertThat(knowledge.getOwnerRepository(pf3)).isEqualTo(two);
+ } finally {
+ PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semantics);
+ }
+ }
+
+ @Test
public void testManagedDirectories() throws Exception {
PrecomputedValue precomputedValue =
(PrecomputedValue)
@@ -323,21 +390,7 @@
.build();
PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semanticsWithManagedDirectories);
- WorkspaceFileValue workspaceFileValue =
- parseWorkspaceFileValue(
- "workspace(",
- " name = 'rr',",
- " managed_directories = {'@repo1': ['dir1', 'dir2'], '@repo2': ['dir3/dir1/..']}",
- ")");
- ImmutableMap<PathFragment, RepositoryName> managedDirectories =
- workspaceFileValue.getManagedDirectories();
- assertThat(managedDirectories).isNotNull();
- assertThat(managedDirectories).hasSize(3);
- assertThat(managedDirectories)
- .containsExactly(
- PathFragment.create("dir1"), RepositoryName.create("@repo1"),
- PathFragment.create("dir2"), RepositoryName.create("@repo1"),
- PathFragment.create("dir3"), RepositoryName.create("@repo2"));
+ createWorkspaceFileValueForTest();
assertManagedDirectoriesParsingError(
"{'@repo1': 'dir1', '@repo2': ['dir3']}",
@@ -384,6 +437,26 @@
}
}
+ private WorkspaceFileValue createWorkspaceFileValueForTest()
+ throws IOException, InterruptedException, LabelSyntaxException {
+ WorkspaceFileValue workspaceFileValue =
+ parseWorkspaceFileValue(
+ "workspace(",
+ " name = 'rr',",
+ " managed_directories = {'@repo1': ['dir1', 'dir2'], '@repo2': ['dir3/dir1/..']}",
+ ")");
+ ImmutableMap<PathFragment, RepositoryName> managedDirectories =
+ workspaceFileValue.getManagedDirectories();
+ assertThat(managedDirectories).isNotNull();
+ assertThat(managedDirectories).hasSize(3);
+ assertThat(managedDirectories)
+ .containsExactly(
+ PathFragment.create("dir1"), RepositoryName.create("@repo1"),
+ PathFragment.create("dir2"), RepositoryName.create("@repo1"),
+ PathFragment.create("dir3"), RepositoryName.create("@repo2"));
+ return workspaceFileValue;
+ }
+
private void assertManagedDirectoriesParsingError(
String managedDirectoriesValue, String expectedError)
throws IOException, InterruptedException {
@@ -511,23 +584,39 @@
createWorkspaceFile("workspace(name = 'old')");
skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE);
- assertThat(testWorkspaceFileListener.getLastWorkspaceName()).isEqualTo("old");
- assertThat(testWorkspaceFileListener.getCnt()).isEqualTo(1);
+ assertThat(testManagedDirectoriesKnowledge.getLastWorkspaceName()).isEqualTo("old");
+ assertThat(testManagedDirectoriesKnowledge.getCnt()).isEqualTo(1);
createWorkspaceFile("workspace(name = 'changed')");
skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE);
- assertThat(testWorkspaceFileListener.getLastWorkspaceName()).isEqualTo("changed");
- assertThat(testWorkspaceFileListener.getCnt()).isEqualTo(2);
+ assertThat(testManagedDirectoriesKnowledge.getLastWorkspaceName()).isEqualTo("changed");
+ assertThat(testManagedDirectoriesKnowledge.getCnt()).isEqualTo(2);
}
- private static class TestWorkspaceFileListener implements WorkspaceFileHeaderListener {
+ private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge {
private String lastWorkspaceName;
private int cnt = 0;
+ @Nullable
@Override
- public void workspaceHeaderChanged(@Nullable WorkspaceFileValue newValue) {
+ public RepositoryName getOwnerRepository(PathFragment relativePathFragment) {
+ return null;
+ }
+
+ @Override
+ public ImmutableSet<PathFragment> getManagedDirectories(RepositoryName repositoryName) {
+ return null;
+ }
+
+ @Override
+ public boolean workspaceHeaderReloaded(
+ @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) {
+ if (Objects.equals(oldValue, newValue)) {
+ return false;
+ }
++cnt;
lastWorkspaceName = newValue != null ? newValue.getPackage().getWorkspaceName() : null;
+ return true;
}
private String getLastWorkspaceName() {
@@ -538,4 +627,22 @@
return cnt;
}
}
+
+ private static class TestManagedDirectoriesListener implements ManagedDirectoriesListener {
+ @Nullable private Set<RepositoryName> repositoryNames;
+
+ @Override
+ public void onManagedDirectoriesRefreshed(Set<RepositoryName> repositoryNames) {
+ this.repositoryNames = repositoryNames;
+ }
+
+ @Nullable
+ public Set<RepositoryName> getRepositoryNames() {
+ return repositoryNames;
+ }
+
+ public void reset() {
+ repositoryNames = null;
+ }
+ }
}
