[9.0.0] Fix and consolidate repo env handling

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -84,7 +84,6 @@
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.runtime.CommonCommandOptions;
 import com.google.devtools.build.lib.runtime.InfoItem;
 import com.google.devtools.build.lib.runtime.ProcessWrapper;
 import com.google.devtools.build.lib.runtime.RemoteRepoContentsCache;
@@ -132,9 +131,9 @@ public class BazelRepositoryModule extends BlazeModule {
       ImmutableMap.of();
 
   private final RepositoryCache repositoryCache = new RepositoryCache();
-  private final MutableSupplier<Map<String, String>> repoEnvironmentSupplier =
+  private final MutableSupplier<ImmutableMap<String, String>> repoEnvSupplier =
       new MutableSupplier<>();
-  private final MutableSupplier<Map<String, String>> clientEnvironmentSupplier =
+  private final MutableSupplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier =
       new MutableSupplier<>();
   private boolean fetchDisabled = false;
   private ImmutableMap<String, PathFragment> overrides = ImmutableMap.of();
@@ -158,9 +157,9 @@ public class BazelRepositoryModule extends BlazeModule {
   private RepoSpecFunction repoSpecFunction;
   private YankedVersionsFunction yankedVersionsFunction;
 
-  private final VendorCommand vendorCommand = new VendorCommand(clientEnvironmentSupplier);
+  private final VendorCommand vendorCommand = new VendorCommand(nonstrictRepoEnvSupplier);
   private final RegistryFactoryImpl registryFactory =
-      new RegistryFactoryImpl(clientEnvironmentSupplier);
+      new RegistryFactoryImpl(nonstrictRepoEnvSupplier);
 
   @Nullable private CredentialModule credentialModule;
 
@@ -213,13 +212,12 @@ public void workspaceInit(
 
     repositoryFetchFunction =
         new RepositoryFetchFunction(
-            repoEnvironmentSupplier,
-            clientEnvironmentSupplier,
+            repoEnvSupplier,
+            nonstrictRepoEnvSupplier,
             directories,
             repositoryCache.getRepoContentsCache());
     singleExtensionEvalFunction =
-        new SingleExtensionEvalFunction(
-            directories, repoEnvironmentSupplier, clientEnvironmentSupplier);
+        new SingleExtensionEvalFunction(directories, repoEnvSupplier, nonstrictRepoEnvSupplier);
 
     if (builtinModules == null) {
       builtinModules = ModuleFileFunction.getBuiltinModules();
@@ -282,13 +280,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     this.yankedVersionsFunction.setDownloadManager(downloadManager);
     this.vendorCommand.setDownloadManager(downloadManager);
 
-    CommonCommandOptions commandOptions = env.getOptions().getOptions(CommonCommandOptions.class);
-    if (commandOptions.useStrictRepoEnv) {
-      repoEnvironmentSupplier.set(env.getRepoEnvFromOptions());
-    } else {
-      repoEnvironmentSupplier.set(env.getRepoEnv());
-    }
-    clientEnvironmentSupplier.set(env.getRepoEnv());
+    repoEnvSupplier.set(env.getRepoEnv());
+    nonstrictRepoEnvSupplier.set(env.getNonstrictRepoEnv());
     PackageOptions pkgOptions = env.getOptions().getOptions(PackageOptions.class);
     fetchDisabled = pkgOptions != null && !pkgOptions.fetch;
 
@@ -716,6 +709,7 @@ public ImmutableList<Injected> getPrecomputedValues() {
       lastRegistryInvalidation = now;
     }
     return ImmutableList.of(
+        PrecomputedValue.injected(PrecomputedValue.REPO_ENV, repoEnvSupplier.get()),
         PrecomputedValue.injected(RepoDefinitionFunction.REPOSITORY_OVERRIDES, overrides),
         PrecomputedValue.injected(ModuleFileFunction.INJECTED_REPOSITORIES, injections),
         PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, moduleOverrides),

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -289,7 +289,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_function",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:environment_variable_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
@@ -334,7 +334,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repo_definition",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository:utils",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -345,6 +344,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_failed_exception",
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:repo_environment_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
         "//src/main/java/com/google/devtools/build/lib/util:os",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java

@@ -38,7 +38,7 @@
 import com.google.devtools.build.lib.profiler.SilentCloseable;
 import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
 import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
-import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
+import com.google.devtools.build.lib.skyframe.EnvironmentVariableValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -79,8 +79,8 @@ private static class ModuleResolutionComputeState implements Environment.SkyKeyC
   @Nullable
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws BazelModuleResolutionFunctionException, InterruptedException {
-    ClientEnvironmentValue allowedYankedVersionsFromEnv =
-        (ClientEnvironmentValue)
+    EnvironmentVariableValue allowedYankedVersionsFromEnv =
+        (EnvironmentVariableValue)
             env.getValue(ClientEnvironmentFunction.key(BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
     if (allowedYankedVersionsFromEnv == null) {
       return null;
@@ -90,7 +90,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     try {
       allowedYankedVersions =
           YankedVersionsUtil.parseAllowedYankedVersions(
-              allowedYankedVersionsFromEnv.getValue(),
+              allowedYankedVersionsFromEnv.value(),
               Objects.requireNonNull(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.get(env)));
     } catch (ExternalDepsException e) {
       throw new BazelModuleResolutionFunctionException(e, Transience.PERSISTENT);

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java

@@ -26,7 +26,6 @@
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
-import java.util.Map;
 import java.util.Optional;
 import javax.annotation.Nullable;
 import net.starlark.java.annot.Param;
@@ -59,8 +58,8 @@ protected ModuleExtensionContext(
       Path workingDirectory,
       BlazeDirectories directories,
       Environment env,
-      Map<String, String> repoEnvVariables,
-      Map<String, String> clientEnvVariables,
+      ImmutableMap<String, String> repoEnv,
+      ImmutableMap<String, String> nonstrictRepoEnv,
       DownloadManager downloadManager,
       double timeoutScaling,
       @Nullable ProcessWrapper processWrapper,
@@ -76,8 +75,8 @@ protected ModuleExtensionContext(
         workingDirectory,
         directories,
         env,
-        repoEnvVariables,
-        clientEnvVariables,
+        repoEnv,
+        nonstrictRepoEnv,
         downloadManager,
         timeoutScaling,
         processWrapper,

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

@@ -57,7 +57,7 @@
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.Success;
 import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
 import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
-import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
+import com.google.devtools.build.lib.skyframe.EnvironmentVariableValue;
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
@@ -171,8 +171,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       return computeForRootModule(starlarkSemantics, env, SymbolGenerator.create(skyKey));
     }
 
-    ClientEnvironmentValue allowedYankedVersionsFromEnv =
-        (ClientEnvironmentValue)
+    EnvironmentVariableValue allowedYankedVersionsFromEnv =
+        (EnvironmentVariableValue)
             env.getValue(
                 ClientEnvironmentFunction.key(
                     YankedVersionsUtil.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java

@@ -23,16 +23,15 @@
 import com.google.devtools.build.lib.vfs.Path;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.Map;
 import java.util.Optional;
 import java.util.function.Supplier;
 
 /** Prod implementation of {@link RegistryFactory}. */
 public class RegistryFactoryImpl implements RegistryFactory {
-  private final Supplier<Map<String, String>> clientEnvironmentSupplier;
+  private final Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier;
 
-  public RegistryFactoryImpl(Supplier<Map<String, String>> clientEnvironmentSupplier) {
-    this.clientEnvironmentSupplier = clientEnvironmentSupplier;
+  public RegistryFactoryImpl(Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier) {
+    this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier;
   }
 
   @Override
@@ -75,7 +74,7 @@ public Registry createRegistry(
     }
     return new IndexRegistry(
         uri,
-        clientEnvironmentSupplier.get(),
+        nonstrictRepoEnvSupplier.get(),
         knownFileHashes,
         knownFileHashesMode,
         previouslySelectedYankedVersions,

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java

@@ -22,7 +22,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.bazel.repository.RepositoryUtils;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
 import com.google.devtools.build.lib.bazel.repository.starlark.NeedsSkyframeRestartException;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
@@ -41,18 +40,17 @@
 import com.google.devtools.build.lib.skyframe.BzlLoadFailedException;
 import com.google.devtools.build.lib.skyframe.BzlLoadFunction;
 import com.google.devtools.build.lib.skyframe.BzlLoadValue;
+import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.WorkerSkyKeyComputeState;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
-import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Mutability;
@@ -73,8 +71,8 @@ final class RegularRunnableExtension implements RunnableExtension {
   private final ModuleExtension extension;
   private final ImmutableMap<String, Optional<String>> staticEnvVars;
   private final BlazeDirectories directories;
-  private final Supplier<Map<String, String>> repoEnvironmentSupplier;
-  private final Supplier<Map<String, String>> clientEnvironmentSupplier;
+  private final ImmutableMap<String, String> repoEnv;
+  private final ImmutableMap<String, String> nonstrictRepoEnv;
   private final double timeoutScaling;
   @Nullable private final ProcessWrapper processWrapper;
   @Nullable private final RepositoryRemoteExecutor repositoryRemoteExecutor;
@@ -85,8 +83,8 @@ final class RegularRunnableExtension implements RunnableExtension {
       ModuleExtension extension,
       ImmutableMap<String, Optional<String>> staticEnvVars,
       BlazeDirectories directories,
-      Supplier<Map<String, String>> repoEnvironmentSupplier,
-      Supplier<Map<String, String>> clientEnvironmentSupplier,
+      ImmutableMap<String, String> repoEnv,
+      ImmutableMap<String, String> nonstrictRepoEnv,
       double timeoutScaling,
       @Nullable ProcessWrapper processWrapper,
       @Nullable RepositoryRemoteExecutor repositoryRemoteExecutor,
@@ -95,8 +93,8 @@ final class RegularRunnableExtension implements RunnableExtension {
     this.extension = extension;
     this.staticEnvVars = staticEnvVars;
     this.directories = directories;
-    this.repoEnvironmentSupplier = repoEnvironmentSupplier;
-    this.clientEnvironmentSupplier = clientEnvironmentSupplier;
+    this.repoEnv = repoEnv;
+    this.nonstrictRepoEnv = nonstrictRepoEnv;
     this.timeoutScaling = timeoutScaling;
     this.processWrapper = processWrapper;
     this.repositoryRemoteExecutor = repositoryRemoteExecutor;
@@ -144,8 +142,8 @@ static RegularRunnableExtension load(
       StarlarkSemantics starlarkSemantics,
       Environment env,
       BlazeDirectories directories,
-      Supplier<Map<String, String>> repoEnvironmentSupplier,
-      Supplier<Map<String, String>> clientEnvironmentSupplier,
+      ImmutableMap<String, String> repoEnv,
+      ImmutableMap<String, String> nonstrictRepoEnv,
       double timeoutScaling,
       @Nullable ProcessWrapper processWrapper,
       @Nullable RepositoryRemoteExecutor repositoryRemoteExecutor,
@@ -181,7 +179,8 @@ static RegularRunnableExtension load(
     }
 
     ImmutableMap<String, Optional<String>> staticEnvVars =
-        RepositoryUtils.getEnvVarValues(env, ImmutableSet.copyOf(extension.envVariables()));
+        RepoEnvironmentFunction.getEnvironmentView(
+            env, ImmutableSet.copyOf(extension.envVariables()));
     if (staticEnvVars == null) {
       return null;
     }
@@ -190,8 +189,8 @@ static RegularRunnableExtension load(
         extension,
         staticEnvVars,
         directories,
-        repoEnvironmentSupplier,
-        clientEnvironmentSupplier,
+        repoEnv,
+        nonstrictRepoEnv,
         timeoutScaling,
         processWrapper,
         repositoryRemoteExecutor,
@@ -356,8 +355,8 @@ private ModuleExtensionContext createContext(
         workingDirectory,
         directories,
         env,
-        repoEnvironmentSupplier.get(),
-        clientEnvironmentSupplier.get(),
+        repoEnv,
+        nonstrictRepoEnv,
         downloadManager,
         timeoutScaling,
         processWrapper,

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java

@@ -42,7 +42,6 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -58,8 +57,8 @@
  */
 public class SingleExtensionEvalFunction implements SkyFunction {
   private final BlazeDirectories directories;
-  private final Supplier<Map<String, String>> repoEnvironmentSupplier;
-  private final Supplier<Map<String, String>> clientEnvironmentSupplier;
+  private final Supplier<ImmutableMap<String, String>> repoEnvSupplier;
+  private final Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier;
 
   private double timeoutScaling = 1.0;
   @Nullable private ProcessWrapper processWrapper = null;
@@ -68,11 +67,11 @@ public class SingleExtensionEvalFunction implements SkyFunction {
 
   public SingleExtensionEvalFunction(
       BlazeDirectories directories,
-      Supplier<Map<String, String>> repoEnvironmentSupplier,
-      Supplier<Map<String, String>> clientEnvironmentSupplier) {
+      Supplier<ImmutableMap<String, String>> repoEnvSupplier,
+      Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier) {
     this.directories = directories;
-    this.repoEnvironmentSupplier = repoEnvironmentSupplier;
-    this.clientEnvironmentSupplier = clientEnvironmentSupplier;
+    this.repoEnvSupplier = repoEnvSupplier;
+    this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier;
   }
 
   public void setDownloadManager(DownloadManager downloadManager) {
@@ -123,8 +122,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                 starlarkSemantics,
                 env,
                 directories,
-                repoEnvironmentSupplier,
-                clientEnvironmentSupplier,
+                repoEnvSupplier.get(),
+                nonstrictRepoEnvSupplier.get(),
                 timeoutScaling,
                 processWrapper,
                 repositoryRemoteExecutor,

src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD

@@ -38,7 +38,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:tidy",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:vendor",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository:repo_definition",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repo_definition_value",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",

src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java

@@ -111,12 +111,12 @@
 public final class VendorCommand implements BlazeCommand {
   public static final String NAME = "vendor";
 
-  private final Supplier<Map<String, String>> clientEnvironmentSupplier;
+  private final Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier;
   @Nullable private VendorManager vendorManager = null;
   @Nullable private DownloadManager downloadManager;
 
-  public VendorCommand(Supplier<Map<String, String>> clientEnvironmentSupplier) {
-    this.clientEnvironmentSupplier = clientEnvironmentSupplier;
+  public VendorCommand(Supplier<ImmutableMap<String, String>> nonstrictRepoEnvSupplier) {
+    this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier;
   }
 
   public void setDownloadManager(DownloadManager downloadManager) {
@@ -383,7 +383,7 @@ private void vendor(CommandEnvironment env, ImmutableList<RepositoryName> reposT
           vendorManager.vendorRegistryUrl(
               url,
               downloadManager.downloadAndReadOneUrlForBzlmod(
-                  url, clientEnvironmentSupplier.get(), checksum));
+                  url, nonstrictRepoEnvSupplier.get(), checksum));
         } catch (IOException e) {
           throw new IOException(
               String.format(