Support unconditional fetching of repositories
Make all external repositories depend on an additional SkyValue controllable
via commands, so support unconditional fetching of all external repositories,
as it is needed by the the `sync` command.
Improves on #5175, provides a work around for #4907.
Change-Id: I30033614c1a2fad3f1363b85ff69cf92f697c255
PiperOrigin-RevId: 200543985
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 6d486b0..e336b3a 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
@@ -293,7 +293,12 @@
@Override
public ImmutableList<Injected> getPrecomputedValues() {
return ImmutableList.of(
- PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides));
+ PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides),
+ // That key will be reinjected by the sync command with a universally unique identifier.
+ // Nevertheless, we need to provide a default value for other commands.
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY));
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
index c5c3085..e9f2183 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
@@ -13,12 +13,14 @@
// limitations under the License.
package com.google.devtools.build.lib.bazel.commands;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
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.packages.Rule;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
+import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.runtime.BlazeCommand;
import com.google.devtools.build.lib.runtime.BlazeCommandResult;
@@ -26,6 +28,7 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.KeepGoingOption;
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.WorkspaceFileValue;
import com.google.devtools.build.lib.util.AbruptExitException;
@@ -55,6 +58,11 @@
try {
env.setupPackageCache(options, env.getRuntime().getDefaultsPackageContent());
SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
+ skyframeExecutor.injectExtraPrecomputedValues(
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ env.getCommandId().toString())));
// Obtain the key for the top-level WORKSPACE file
SkyKey packageLookupKey = PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER);
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 db74f31..a590c0b 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
@@ -56,6 +56,12 @@
public static final Precomputed<Map<RepositoryName, PathFragment>> REPOSITORY_OVERRIDES =
new Precomputed<>(PrecomputedValue.Key.create("repository_overrides"));
+ public static final Precomputed<String> DEPENDENCY_FOR_UNCONDITIONAL_FETCHING =
+ new Precomputed<>(
+ PrecomputedValue.Key.create("dependency_for_unconditional_repository_fetching"));
+
+ public static final String DONT_FETCH_UNCONDITIONALLY = "";
+
// The marker file version is inject in the rule key digest so the rule key is always different
// when we decide to update the format.
private static final int MARKER_FILE_VERSION = 3;
@@ -107,6 +113,10 @@
if (env.valuesMissing()) {
return null;
}
+ String fetchUnconditionally = DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.get(env);
+ if (env.valuesMissing()) {
+ return null;
+ }
Path repoRoot = RepositoryFunction.getExternalRepositoryDirectory(directories)
.getRelative(repositoryName.strippedName());
@@ -170,9 +180,11 @@
if (env.valuesMissing()) {
return null;
}
- if (markerHash != null && repoRoot.exists()) {
- // Now that we know that it exists, we can declare a Skyframe dependency on the repository
- // root.
+ if (DONT_FETCH_UNCONDITIONALLY.equals(fetchUnconditionally)
+ && markerHash != null
+ && repoRoot.exists()) {
+ // Now that we know that it exists and that we should not fetch unconditionally, we can
+ // declare a Skyframe dependency on the repository root.
RepositoryFunction.getRepositoryDirectory(repoRoot, env);
if (env.valuesMissing()) {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
index ac55f39..a92d080 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
@@ -101,7 +101,10 @@
PrecomputedValue.injected(PrecomputedValue.ACTION_ENV, ImmutableMap.of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
- Suppliers.ofInstance(ImmutableMap.of())));
+ Suppliers.ofInstance(ImmutableMap.of())),
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY));
return builder;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 811c130..4477724 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -215,9 +215,15 @@
ImmutableMap.<String, String>of(),
ImmutableMap.<String, String>of(),
new TimestampGranularityMonitor(BlazeClock.instance()));
- skyframeExecutor.injectExtraPrecomputedValues(ImmutableList.of(PrecomputedValue.injected(
- RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
- ImmutableMap.<RepositoryName, PathFragment>of())));
+ skyframeExecutor.injectExtraPrecomputedValues(
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
+ ImmutableMap.<RepositoryName, PathFragment>of()),
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY)));
+
packageManager = skyframeExecutor.getPackageManager();
buildView = new BuildViewForTesting(directories, ruleClassProvider, skyframeExecutor, null);
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 6f4f078..cc16cd4 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
@@ -233,10 +233,14 @@
mutableActionGraph = new MapBasedActionGraph(actionKeyContext);
ruleClassProvider = getRuleClassProvider();
- ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues = ImmutableList.of(
- PrecomputedValue.injected(
- RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
- ImmutableMap.<RepositoryName, PathFragment>of()));
+ ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues =
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
+ ImmutableMap.<RepositoryName, PathFragment>of()),
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY));
PackageFactory.BuilderForTesting pkgFactoryBuilder =
analysisMock
.getPackageFactoryBuilderForTesting(directories)
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index 1c314f4..4899920 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -133,9 +133,14 @@
BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE,
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider));
TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
- skyframeExecutor.injectExtraPrecomputedValues(ImmutableList.of(PrecomputedValue.injected(
- RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
- ImmutableMap.<RepositoryName, PathFragment>of())));
+ skyframeExecutor.injectExtraPrecomputedValues(
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
+ ImmutableMap.<RepositoryName, PathFragment>of()),
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY)));
PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
packageCacheOptions.showLoadingProgress = true;
packageCacheOptions.globbingThreads = 7;
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 8bde0bc..cc4a332 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
@@ -137,6 +137,8 @@
ImmutableMap.<RepositoryName, PathFragment>builder()
.put(RepositoryName.createFromValidStrippedName("foo"), overrideDirectory.asFragment())
.build());
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set(
+ differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY);
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
index 4c845af..e02117f 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
@@ -153,6 +153,8 @@
PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(
differencer, ImmutableMap.<RepositoryName, PathFragment>of());
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set(
+ differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY);
}
private ContainingPackageLookupValue lookupContainingPackage(String packageName)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
index 7562559..fa6f05c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
@@ -167,6 +167,8 @@
PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(
differencer, ImmutableMap.<RepositoryName, PathFragment>of());
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set(
+ differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY);
}
protected PackageLookupValue lookupPackage(String packageName) throws InterruptedException {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
index 85decff..06c4363 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
@@ -109,9 +109,14 @@
ImmutableMap.<String, String>of(),
ImmutableMap.<String, String>of(),
new TimestampGranularityMonitor(null));
- skyframeExecutor.injectExtraPrecomputedValues(ImmutableList.of(PrecomputedValue.injected(
- RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
- ImmutableMap.<RepositoryName, PathFragment>of())));
+ skyframeExecutor.injectExtraPrecomputedValues(
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
+ ImmutableMap.<RepositoryName, PathFragment>of()),
+ PrecomputedValue.injected(
+ RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY)));
scratch.file(ADDITIONAL_BLACKLISTED_PACKAGE_PREFIXES_FILE_PATH_STRING);
}
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index b3039d4..72abc3d 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -390,6 +390,7 @@
size = "medium",
srcs = ["workspace_resolved_test.sh"],
data = [":test-deps"],
+ shard_count = 3,
)
sh_test(
diff --git a/src/test/shell/bazel/workspace_resolved_test.sh b/src/test/shell/bazel/workspace_resolved_test.sh
index ace762c..8504bb1 100755
--- a/src/test/shell/bazel/workspace_resolved_test.sh
+++ b/src/test/shell/bazel/workspace_resolved_test.sh
@@ -20,6 +20,7 @@
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }
test_result_recorded() {
+ mkdir result_recorded && cd result_recorded
rm -rf fetchrepo
mkdir fetchrepo
cd fetchrepo
@@ -53,6 +54,9 @@
bazel clean --expunge
bazel build --experimental_repository_resolved_file=../repo.bzl @ext//... \
|| fail "Expected success"
+ # some of the file systems on our test machines are really slow to
+ # notice the creation of a file---even after the call to sync(1).
+ bazel shutdown; sync; sleep 10
# Verify that bazel can read the generated repo.bzl file and that it contains
# the expected information
@@ -87,6 +91,7 @@
}
test_sync_calls_all() {
+ mkdir sync_calls_all && cd sync_calls_all
rm -rf fetchrepo
mkdir fetchrepo
rm -f repo.bzl
@@ -118,6 +123,9 @@
bazel clean --expunge
bazel sync --experimental_repository_resolved_file=../repo.bzl
+ # some of the file systems on our test machines are really slow to
+ # notice the creation of a file---even after the call to sync(1).
+ bazel shutdown; sync; sleep 10
cd ..
echo; cat repo.bzl; echo
@@ -138,4 +146,59 @@
bazel build :a :b :c :d || fail "Expected all 4 repositories to be present"
}
+test_sync_call_invalidates() {
+ mkdir sync_call_invalidates && cd sync_call_invalidates
+ rm -rf fetchrepo
+ mkdir fetchrepo
+ rm -f repo.bzl
+ touch BUILD
+ cat > rule.bzl <<'EOF'
+def _rule_impl(ctx):
+ ctx.file("BUILD", """
+genrule(
+ name = "it",
+ outs = ["it.txt"],
+ cmd = "echo hello world > $@",
+)
+""")
+ ctx.file("WORKSPACE", "")
+
+trivial_rule = repository_rule(
+ implementation = _rule_impl,
+ attrs = {},
+)
+EOF
+ cat > WORKSPACE <<'EOF'
+load("//:rule.bzl", "trivial_rule")
+
+trivial_rule(name = "a")
+trivial_rule(name = "b")
+EOF
+
+ bazel build @a//... @b//...
+ echo; echo sync run; echo
+ bazel sync --experimental_repository_resolved_file=../repo.bzl
+ # some of the file systems on our test machines are really slow to
+ # notice the creation of a file---even after the call to sync(1).
+ bazel shutdown; sync; sleep 10
+
+ cd ..
+ echo; cat repo.bzl; echo
+ touch WORKSPACE
+ cat > BUILD <<'EOF'
+load("//:repo.bzl", "resolved")
+
+names = [entry["original_attributes"]["name"] for entry in resolved]
+
+[
+ genrule(
+ name = name,
+ outs = [ "%s.txt" % (name,) ],
+ cmd = "echo %s > $@" % (name,),
+ ) for name in names
+]
+EOF
+ bazel build :a :b || fail "Expected both repositories to be present"
+}
+
run_suite "workspace_resolved_test tests"
