Treat existence of managed directories as a part of repository dirtiness.
- If managed directory is declared for the external repository and does not exist (probably was deleted by user), make external repository dirty and re-fetch it.
- Add tests to demonstrate the added behavior to ManagedDirectoriesBlackBoxTest.
- Closes #8487.
Closes #8564.
PiperOrigin-RevId: 251882207
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 961a95a..5c94b2e 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
@@ -1,4 +1,4 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
+// 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.
@@ -11,6 +11,7 @@
// 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.bazel;
@@ -173,7 +174,8 @@
builder.setManagedDirectoriesKnowledge(managedDirectoriesKnowledge);
RepositoryDirectoryDirtinessChecker customDirtinessChecker =
- new RepositoryDirectoryDirtinessChecker(managedDirectoriesKnowledge);
+ new RepositoryDirectoryDirtinessChecker(
+ directories.getWorkspace(), managedDirectoriesKnowledge);
builder.addCustomDirtinessChecker(customDirtinessChecker);
// Create the repository function everything flows through.
builder.addSkyFunction(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
index 7d3ddcf..8e77200 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -1,4 +1,4 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
+// 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.
@@ -11,9 +11,12 @@
// 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.rules.repository;
+import static com.google.devtools.build.lib.rules.repository.RepositoryDirectoryDirtinessChecker.managedDirectoriesExist;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
@@ -178,11 +181,13 @@
// generally fast and they do not depend on non-local data, so it does not make much sense to
// try to cache them from across server instances.
boolean fetchLocalRepositoryAlways = isFetch.get() && handler.isLocal(rule);
- if (!fetchLocalRepositoryAlways) {
+ if (!fetchLocalRepositoryAlways
+ && managedDirectoriesExist(directories.getWorkspace(), managedDirectories)) {
// For the non-local repositories, check if they are already up-to-date:
// 1) unconditional fetching is not enabled, AND
// 2) repository directory exists, AND
- // 3) marker file correctly describes the current repository state
+ // 3) marker file correctly describes the current repository state, AND
+ // 4) managed directories, mapped to the repository, exist
if (doNotFetchUnconditionally && repoRoot.exists()) {
byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env);
if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java
index bbc0eaf..309b51d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java
@@ -11,14 +11,18 @@
// 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.rules.repository;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Objects;
@@ -31,15 +35,17 @@
* <li>Only dirties {@link RepositoryDirectoryValue}s, if they were produced in a {@code
* --nofetch} build, so that they are re-created on subsequent {@code --fetch} builds. The
* alternative solution would be to reify the value of the flag as a Skyframe value.
- * <li>Dirties repositories, if their managed directories were changed.
+ * <li>Dirties repositories, if their managed directories were changed or do not exist.
* </ul>
*/
@VisibleForTesting
public class RepositoryDirectoryDirtinessChecker extends SkyValueDirtinessChecker {
+ private final Path workspaceRoot;
private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge;
public RepositoryDirectoryDirtinessChecker(
- ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
+ Path workspaceRoot, ManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
+ this.workspaceRoot = workspaceRoot;
this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
}
@@ -71,6 +77,15 @@
repositoryValue.getManagedDirectories())) {
return DirtyResult.dirty(skyValue);
}
+
+ if (!managedDirectoriesExist(workspaceRoot, repositoryValue.getManagedDirectories())) {
+ return DirtyResult.dirty(skyValue);
+ }
return DirtyResult.notDirty(skyValue);
}
+
+ static boolean managedDirectoriesExist(
+ Path workspaceRoot, ImmutableSet<PathFragment> managedDirectories) {
+ return managedDirectories.stream().allMatch(pf -> workspaceRoot.getRelative(pf).exists());
+ }
}
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
index c5677c9..2cbbf74 100644
--- 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
@@ -4,13 +4,14 @@
// 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
+// 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;
@@ -50,6 +51,39 @@
}
@Test
+ public void testNodeModulesDeleted() throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+
+ Path nodeModules = context().getWorkDir().resolve("node_modules");
+ assertThat(nodeModules.toFile().isDirectory()).isTrue();
+ PathUtils.deleteTree(nodeModules);
+
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+ }
+
+ @Test
+ public void testNodeModulesDeletedAndRecreated() throws Exception {
+ generateProject();
+ buildExpectRepositoryRuleCalled();
+ checkProjectFiles();
+
+ Path nodeModules = context().getWorkDir().resolve("node_modules");
+ assertThat(nodeModules.toFile().isDirectory()).isTrue();
+
+ Path nodeModulesBackup = context().getWorkDir().resolve("node_modules_backup");
+ PathUtils.copyTree(nodeModules, nodeModulesBackup);
+ PathUtils.deleteTree(nodeModules);
+
+ PathUtils.copyTree(nodeModulesBackup, nodeModules);
+
+ buildExpectRepositoryRuleNotCalled();
+ checkProjectFiles();
+ }
+
+ @Test
public void testBuildProjectFetchNotRecalled() throws Exception {
generateProject();
buildExpectRepositoryRuleCalled();
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 f693bea..39cea3f 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
@@ -1,4 +1,4 @@
-// Copyright 2017 The Bazel Authors. All rights reserved.
+// 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.
@@ -11,6 +11,7 @@
// 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.rules.repository;
@@ -77,9 +78,11 @@
import com.google.devtools.build.skyframe.SequentialBuildDriver;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
+import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import java.io.IOException;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
@@ -115,7 +118,8 @@
managedDirectoriesKnowledge = new TestManagedDirectoriesKnowledge();
HttpDownloader downloader = Mockito.mock(HttpDownloader.class);
RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction();
- testSkylarkRepositoryFunction = new TestSkylarkRepositoryFunction(downloader);
+ testSkylarkRepositoryFunction =
+ new TestSkylarkRepositoryFunction(rootPath, downloader, managedDirectoriesKnowledge);
ImmutableMap<String, RepositoryFunction> repositoryHandlers =
ImmutableMap.of(LocalRepositoryRule.NAME, localRepositoryFunction);
delegatorFunction =
@@ -243,7 +247,7 @@
TestManagedDirectoriesKnowledge knowledge = new TestManagedDirectoriesKnowledge();
RepositoryDirectoryDirtinessChecker checker =
- new RepositoryDirectoryDirtinessChecker(knowledge);
+ new RepositoryDirectoryDirtinessChecker(rootPath, knowledge);
RepositoryName repositoryName = RepositoryName.create("@repo");
RepositoryDirectoryValue.Key key = RepositoryDirectoryValue.key(repositoryName);
@@ -272,12 +276,18 @@
assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isTrue();
+ Path managedDirectoryM = rootPath.getRelative("m");
+ assertThat(managedDirectoryM.createDirectory()).isTrue();
+
knowledge.setManagedDirectories(
ImmutableMap.of(PathFragment.create("m"), RepositoryName.create("@other")));
assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isTrue();
knowledge.setManagedDirectories(ImmutableMap.of(PathFragment.create("m"), repositoryName));
assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isFalse();
+
+ managedDirectoryM.deleteTree();
+ assertThat(checker.check(key, withManagedDirectories, tsgm).isDirty()).isTrue();
}
@Test
@@ -313,9 +323,23 @@
testSkylarkRepositoryFunction.reset();
loadRepo("repo1");
+ // Nothing changed, fetch does not happen.
assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isFalse();
testSkylarkRepositoryFunction.reset();
+ // Delete managed directory, fetch should happen again.
+ Path managedDirectory = rootPath.getRelative("dir1");
+ managedDirectory.deleteTree();
+ loadRepo("repo1");
+ assertThat(testSkylarkRepositoryFunction.isFetchCalled()).isTrue();
+ testSkylarkRepositoryFunction.reset();
+
+ // Change managed directories declaration, fetch should happen.
+ // NB: we are making sure that managed directories exist to check only the declaration changes
+ // were percepted.
+ rootPath.getRelative("dir1").createDirectory();
+ rootPath.getRelative("dir2").createDirectory();
+
managedDirectoriesKnowledge.setManagedDirectories(
ImmutableMap.of(
PathFragment.create("dir1"),
@@ -354,9 +378,16 @@
private static class TestSkylarkRepositoryFunction extends SkylarkRepositoryFunction {
private boolean fetchCalled = false;
+ private final Path workspaceRoot;
+ private final TestManagedDirectoriesKnowledge managedDirectoriesKnowledge;
- private TestSkylarkRepositoryFunction(HttpDownloader downloader) {
+ private TestSkylarkRepositoryFunction(
+ Path workspaceRoot,
+ HttpDownloader downloader,
+ TestManagedDirectoriesKnowledge managedDirectoriesKnowledge) {
super(downloader);
+ this.workspaceRoot = workspaceRoot;
+ this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
}
public void reset() {
@@ -378,7 +409,18 @@
SkyKey key)
throws RepositoryFunctionException, InterruptedException {
fetchCalled = true;
- return super.fetch(rule, outputDirectory, directories, env, markerData, key);
+ RepositoryDirectoryValue.Builder builder =
+ super.fetch(rule, outputDirectory, directories, env, markerData, key);
+ ImmutableSet<PathFragment> managedDirectories =
+ managedDirectoriesKnowledge.getManagedDirectories((RepositoryName) key.argument());
+ try {
+ for (PathFragment managedDirectory : managedDirectories) {
+ workspaceRoot.getRelative(managedDirectory).createDirectory();
+ }
+ } catch (IOException e) {
+ throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+ }
+ return builder;
}
}
