Inject BlazeDirectories into RepositoryDelegatorFunction directly, instead of depending on a precomputed value. BlazeDirectories don't change over the lifetime of the Blaze server, and certainly not over the lifetime of a SkyframeExecutor instance, which already had a reference to them.

Step 2.

PiperOrigin-RevId: 169711443

4 files changed, 40 insertions, 22 deletions

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 648874f23c..b065806674 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
@@ -66,6 +66,7 @@ import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.ServerBuilder;
 import com.google.devtools.build.lib.runtime.WorkspaceBuilder;
+import com.google.devtools.build.lib.skyframe.MutableSupplier;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
@@ -79,6 +80,7 @@ import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
@@ -92,10 +94,11 @@ public class BazelRepositoryModule extends BlazeModule {
   private final ImmutableMap<String, RepositoryFunction> repositoryHandlers;
   private final AtomicBoolean isFetch = new AtomicBoolean(false);
   private final SkylarkRepositoryFunction skylarkRepositoryFunction;
-  private final RepositoryDelegatorFunction delegator;
   private final RepositoryCache repositoryCache = new RepositoryCache();
   private final HttpDownloader httpDownloader = new HttpDownloader(repositoryCache);
   private final MavenDownloader mavenDownloader = new MavenDownloader(repositoryCache);
+  private final MutableSupplier<Map<String, String>> clientEnvironmentSupplier =
+      new MutableSupplier<>();
   private ImmutableMap<RepositoryName, PathFragment> overrides = ImmutableMap.of();
   private FileSystem filesystem;
 
@@ -116,8 +119,6 @@ public class BazelRepositoryModule extends BlazeModule {
             .put(AndroidNdkRepositoryRule.NAME, new AndroidNdkRepositoryFunction())
             .put(MavenServerRule.NAME, new MavenServerRepositoryFunction())
             .build();
-    this.delegator = new RepositoryDelegatorFunction(
-        repositoryHandlers, skylarkRepositoryFunction, isFetch);
   }
 
   /**
@@ -160,7 +161,14 @@ public class BazelRepositoryModule extends BlazeModule {
     builder.addCustomDirtinessChecker(REPOSITORY_VALUE_CHECKER);
     // Create the repository function everything flows through.
     builder.addSkyFunction(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction());
-    builder.addSkyFunction(SkyFunctions.REPOSITORY_DIRECTORY, delegator);
+    builder.addSkyFunction(
+        SkyFunctions.REPOSITORY_DIRECTORY,
+        new RepositoryDelegatorFunction(
+            repositoryHandlers,
+            skylarkRepositoryFunction,
+            isFetch,
+            clientEnvironmentSupplier,
+            directories));
     builder.addSkyFunction(MavenServerFunction.NAME, new MavenServerFunction());
     filesystem = directories.getFileSystem();
   }
@@ -184,7 +192,7 @@ public class BazelRepositoryModule extends BlazeModule {
 
   @Override
   public void beforeCommand(CommandEnvironment env) {
-    delegator.setClientEnvironment(env.getActionClientEnv());
+    clientEnvironmentSupplier.set(env.getActionClientEnv());
     PackageCacheOptions pkgOptions = env.getOptions().getOptions(PackageCacheOptions.class);
     isFetch.set(pkgOptions != null && pkgOptions.fetch);
 
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 6f6573e0f1..15eaa374b7 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
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.packages.RuleFormatter;
 import com.google.devtools.build.lib.rules.ExternalPackageUtil;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
 import com.google.devtools.build.lib.skyframe.FileValue;
-import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -42,6 +41,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
 /**
@@ -72,17 +72,21 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
   // command is a fetch. Remote repository lookups are only allowed during fetches.
   private final AtomicBoolean isFetch;
 
-  private Map<String, String> clientEnvironment;
+  private final BlazeDirectories directories;
 
-  public RepositoryDelegatorFunction(ImmutableMap<String, RepositoryFunction> handlers,
-      @Nullable RepositoryFunction skylarkHandler, AtomicBoolean isFetch) {
+  private final Supplier<Map<String, String>> clientEnvironmentSupplier;
+
+  public RepositoryDelegatorFunction(
+      ImmutableMap<String, RepositoryFunction> handlers,
+      @Nullable RepositoryFunction skylarkHandler,
+      AtomicBoolean isFetch,
+      Supplier<Map<String, String>> clientEnvironmentSupplier,
+      BlazeDirectories directories) {
     this.handlers = handlers;
     this.skylarkHandler = skylarkHandler;
     this.isFetch = isFetch;
-  }
-
-  public void setClientEnvironment(Map<String, String> clientEnvironment) {
-    this.clientEnvironment = clientEnvironment;
+    this.clientEnvironmentSupplier = clientEnvironmentSupplier;
+    this.directories = directories;
   }
 
   private void setupRepositoryRoot(Path repoRoot) throws RepositoryFunctionException {
@@ -98,7 +102,6 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws SkyFunctionException, InterruptedException {
     RepositoryName repositoryName = (RepositoryName) skyKey.argument();
-    BlazeDirectories directories = PrecomputedValue.BLAZE_DIRECTORIES.get(env);
     Map<RepositoryName, PathFragment> overrides = REPOSITORY_OVERRIDES.get(env);
     if (env.valuesMissing()) {
       return null;
@@ -133,7 +136,7 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
       return RepositoryDirectoryValue.NO_SUCH_REPOSITORY_VALUE;
     }
 
-    handler.setClientEnvironment(clientEnvironment);
+    handler.setClientEnvironment(clientEnvironmentSupplier.get());
 
     byte[] ruleSpecificData = handler.getRuleSpecificMarkerData(rule, env);
     if (ruleSpecificData == null) {
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 2a937bb235..fa12d24362 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
@@ -123,6 +123,7 @@ public abstract class AbstractPackageLoader implements PackageLoader {
   /** Abstract base class of a builder for {@link PackageLoader} instances. */
   public abstract static class Builder {
     protected final Path workspaceDir;
+    protected final BlazeDirectories directories;
     protected RuleClassProvider ruleClassProvider = getDefaultRuleClassProvider();
     protected Reporter reporter = new Reporter(new EventBus());
     protected Map<SkyFunctionName, SkyFunction> extraSkyFunctions = new HashMap<>();
@@ -133,6 +134,12 @@ public abstract class AbstractPackageLoader implements PackageLoader {
 
     protected Builder(Path workspaceDir) {
       this.workspaceDir = workspaceDir;
+      // The 'installBase' and 'outputBase' directories won't be meaningfully used by
+      // WorkspaceFileFunction, so we pass in a dummy Path.
+      // TODO(nharmata): Refactor WorkspaceFileFunction to make this a non-issue.
+      Path devNull = workspaceDir.getFileSystem().getPath("/dev/null");
+      directories =
+          new BlazeDirectories(new ServerDirectories(devNull, devNull), workspaceDir, "blaze");
     }
 
     public Builder setRuleClassProvider(RuleClassProvider ruleClassProvider) {
@@ -188,13 +195,8 @@ public abstract class AbstractPackageLoader implements PackageLoader {
     this.pkgLocatorRef = new AtomicReference<>(pkgLocator);
     this.legacyGlobbingThreads = builder.legacyGlobbingThreads;
     this.skyframeThreads = builder.skyframeThreads;
+    this.directories = builder.directories;
 
-    // The 'installBase' and 'outputBase' directories won't be meaningfully used by
-    // WorkspaceFileFunction, so we pass in a dummy Path.
-    // TODO(nharmata): Refactor WorkspaceFileFunction to make this a non-issue.
-    Path devNull = workspaceDir.getFileSystem().getPath("/dev/null");
-    this.directories =
-        new BlazeDirectories(new ServerDirectories(devNull, devNull), workspaceDir, "blaze");
     this.externalFilesHelper = new ExternalFilesHelper(
         pkgLocatorRef,
         ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
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 fb95707874..159b69bedc 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
@@ -56,7 +56,12 @@ public class BazelPackageLoader extends AbstractPackageLoader {
             SkyFunctions.LOCAL_REPOSITORY_LOOKUP,
             new LocalRepositoryLookupFunction(),
             SkyFunctions.REPOSITORY_DIRECTORY,
-            new RepositoryDelegatorFunction(repositoryHandlers, null, new AtomicBoolean(true)),
+            new RepositoryDelegatorFunction(
+                repositoryHandlers,
+                null,
+                new AtomicBoolean(true),
+                ImmutableMap::of,
+                builder.directories),
             SkyFunctions.REPOSITORY,
             new RepositoryLoaderFunction()));