Fix test flakiness due to the filesystem mutations done during the 'fetch' work done by BazelPackageBuilderHelperForTesting#sanityCheckBazelPackageLoader.

We fix this flakiness by _not_ having RepositoryDelegatorFunction unconditionally 'fetch' local repos. We then have to make a change to BazelPackageLoader test to reinstate these mutations, since it was only working before due to the unconditional filesystem mutations done here :p

RELNOTES: None
PiperOrigin-RevId: 190222830

2 files changed, 18 insertions, 10 deletions

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 a4b6c9a16b..f8f1c26af2 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
@@ -145,10 +145,10 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
     }
     String ruleKey = computeRuleKey(rule, ruleSpecificData);
     Map<String, String> markerData = new TreeMap<>();
-    if (handler.isLocal(rule)) {
-      // Local repositories are always fetched because the operation is generally fast and they do
-      // not depend on non-local data, so it does not make much sense to try to cache from across
-      // server instances.
+     if (isFetch.get() && handler.isLocal(rule)) {
+      // Local repositories are fetched regardless of the marker file because the operation is
+      // generally fast and they do not depend on non-local data, so it does not make much sense to
+      // try to cache from across server instances.
       setupRepositoryRoot(repoRoot);
       RepositoryDirectoryValue.Builder localRepo =
           handler.fetch(rule, repoRoot, directories, env, markerData);
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 cdaa07c1eb..1393fbf7dd 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
@@ -54,7 +54,11 @@ import java.util.concurrent.atomic.AtomicReference;
 public class BazelPackageLoader extends AbstractPackageLoader {
   /** Returns a fresh {@link Builder} instance. */
   public static Builder builder(Path workspaceDir, Path installBase, Path outputBase) {
-    Builder builder = new Builder(workspaceDir, installBase, outputBase);
+    // Prevent PackageLoader from fetching any remote repositories; these should only be fetched by
+    // Bazel before calling PackageLoader.
+    AtomicBoolean isFetch = new AtomicBoolean(false);
+
+    Builder builder = new Builder(workspaceDir, installBase, outputBase, isFetch);
 
     RepositoryCache repositoryCache = new RepositoryCache();
     HttpDownloader httpDownloader = new HttpDownloader(repositoryCache);
@@ -63,10 +67,6 @@ public class BazelPackageLoader extends AbstractPackageLoader {
     ImmutableMap<String, RepositoryFunction> repositoryHandlers =
         BazelRepositoryModule.repositoryRules(httpDownloader, new MavenDownloader(repositoryCache));
 
-    // Prevent PackageLoader from fetching any remote repositories; these should only be fetched by
-    // Bazel before calling PackageLoader.
-    AtomicBoolean isFetch = new AtomicBoolean(false);
-
     builder.addExtraSkyFunctions(
         ImmutableMap.<SkyFunctionName, SkyFunction>builder()
             .put(
@@ -105,6 +105,8 @@ public class BazelPackageLoader extends AbstractPackageLoader {
     private static final ConfiguredRuleClassProvider DEFAULT_RULE_CLASS_PROVIDER =
         createRuleClassProvider();
 
+    private final AtomicBoolean isFetch;
+
     private static ConfiguredRuleClassProvider createRuleClassProvider() {
       ConfiguredRuleClassProvider.Builder classProvider = new ConfiguredRuleClassProvider.Builder();
       new BazelRepositoryModule().initializeRuleClasses(classProvider);
@@ -112,8 +114,9 @@ public class BazelPackageLoader extends AbstractPackageLoader {
       return classProvider.build();
     }
 
-    private Builder(Path workspaceDir, Path installBase, Path outputBase) {
+    private Builder(Path workspaceDir, Path installBase, Path outputBase, AtomicBoolean isFetch) {
       super(workspaceDir, installBase, outputBase);
+      this.isFetch = isFetch;
     }
 
     @Override
@@ -131,6 +134,11 @@ public class BazelPackageLoader extends AbstractPackageLoader {
       return DEFAULT_RULE_CLASS_PROVIDER.getDefaultsPackageContent(
           InvocationPolicy.getDefaultInstance());
     }
+
+    Builder setFetchForTesting() {
+      this.isFetch.set(true);
+      return this;
+    }
   }
 
   private BazelPackageLoader(Builder builder) {