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 f60177c23913da..fa69ca7449e9de 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 @@ -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> repoEnvironmentSupplier = + private final MutableSupplier> repoEnvSupplier = new MutableSupplier<>(); - private final MutableSupplier> clientEnvironmentSupplier = + private final MutableSupplier> nonstrictRepoEnvSupplier = new MutableSupplier<>(); private boolean fetchDisabled = false; private ImmutableMap 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 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), diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 9cc5f7184253cc..21b195804b41ec 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java index bc971cc7f183fe..2bdef586a1694d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java +++ b/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); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java index ef86ba77913aac..603202a5b43465 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java +++ b/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 repoEnvVariables, - Map clientEnvVariables, + ImmutableMap repoEnv, + ImmutableMap nonstrictRepoEnv, DownloadManager downloadManager, double timeoutScaling, @Nullable ProcessWrapper processWrapper, @@ -76,8 +75,8 @@ protected ModuleExtensionContext( workingDirectory, directories, env, - repoEnvVariables, - clientEnvVariables, + repoEnv, + nonstrictRepoEnv, downloadManager, timeoutScaling, processWrapper, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index e921ad4cc14e40..1ff76bc298e119 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/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)); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java index 20a659072d238b..b1a64ef3dfd24c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java +++ b/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> clientEnvironmentSupplier; + private final Supplier> nonstrictRepoEnvSupplier; - public RegistryFactoryImpl(Supplier> clientEnvironmentSupplier) { - this.clientEnvironmentSupplier = clientEnvironmentSupplier; + public RegistryFactoryImpl(Supplier> nonstrictRepoEnvSupplier) { + this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier; } @Override @@ -75,7 +74,7 @@ public Registry createRegistry( } return new IndexRegistry( uri, - clientEnvironmentSupplier.get(), + nonstrictRepoEnvSupplier.get(), knownFileHashes, knownFileHashesMode, previouslySelectedYankedVersions, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java index 4bfce39095dc6a..5f57c9f540b722 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java +++ b/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> staticEnvVars; private final BlazeDirectories directories; - private final Supplier> repoEnvironmentSupplier; - private final Supplier> clientEnvironmentSupplier; + private final ImmutableMap repoEnv; + private final ImmutableMap 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> staticEnvVars, BlazeDirectories directories, - Supplier> repoEnvironmentSupplier, - Supplier> clientEnvironmentSupplier, + ImmutableMap repoEnv, + ImmutableMap 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> repoEnvironmentSupplier, - Supplier> clientEnvironmentSupplier, + ImmutableMap repoEnv, + ImmutableMap nonstrictRepoEnv, double timeoutScaling, @Nullable ProcessWrapper processWrapper, @Nullable RepositoryRemoteExecutor repositoryRemoteExecutor, @@ -181,7 +179,8 @@ static RegularRunnableExtension load( } ImmutableMap> 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, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 1d31e871dd2916..aea1b754a10a97 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/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> repoEnvironmentSupplier; - private final Supplier> clientEnvironmentSupplier; + private final Supplier> repoEnvSupplier; + private final Supplier> 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> repoEnvironmentSupplier, - Supplier> clientEnvironmentSupplier) { + Supplier> repoEnvSupplier, + Supplier> 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, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD index b9acc60e427204..675493c0fc6090 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java index 32fceb52fadd60..3e0f6e69b1e2cf 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java +++ b/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> clientEnvironmentSupplier; + private final Supplier> nonstrictRepoEnvSupplier; @Nullable private VendorManager vendorManager = null; @Nullable private DownloadManager downloadManager; - public VendorCommand(Supplier> clientEnvironmentSupplier) { - this.clientEnvironmentSupplier = clientEnvironmentSupplier; + public VendorCommand(Supplier> nonstrictRepoEnvSupplier) { + this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier; } public void setDownloadManager(DownloadManager downloadManager) { @@ -383,7 +383,7 @@ private void vendor(CommandEnvironment env, ImmutableList reposT vendorManager.vendorRegistryUrl( url, downloadManager.downloadAndReadOneUrlForBzlmod( - url, clientEnvironmentSupplier.get(), checksum)); + url, nonstrictRepoEnvSupplier.get(), checksum)); } catch (IOException e) { throw new IOException( String.format( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD index 524b9bbe74a866..86567785c2f13c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD @@ -70,6 +70,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:already_reported_exception", "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_subdirectories_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/util", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -138,14 +139,11 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", "//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", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/net/starlark/java/eval", - "//third_party:guava", "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/DigestWriter.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/DigestWriter.java index 3ca3a5ef77c00b..b9473c77b2c62d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/DigestWriter.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/DigestWriter.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.rules.repository.RepoRecordedInput.NeverUpToDateRepoRecordedInput; +import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -176,11 +177,12 @@ private static ImmutableList readMarkerFile( static String computePredeclaredInputHash( Environment env, RepoDefinition repoDefinition, StarlarkSemantics starlarkSemantics) throws InterruptedException { - var unsortedEnviron = RepositoryUtils.getEnvVarValues(env, repoDefinition.repoRule().environ()); - if (unsortedEnviron == null) { + var environ = + RepoEnvironmentFunction.getEnvironmentView(env, repoDefinition.repoRule().environ()); + if (environ == null) { return null; } - var environ = RepoRecordedInput.EnvVar.wrap(unsortedEnviron); + var environInputs = RepoRecordedInput.EnvVar.wrap(environ); var fp = new Fingerprint() .addInt(MARKER_FILE_VERSION) @@ -192,8 +194,8 @@ static String computePredeclaredInputHash( .addString( GsonTypeAdapterUtil.SINGLE_EXTENSION_USAGES_VALUE_GSON.toJson( repoDefinition.attrValues())); - fp.addInt(environ.size()); - environ.forEach( + fp.addInt(environInputs.size()); + environInputs.forEach( (key, value) -> fp.addString(key.toString()).addNullableString(value.orElse(null))); fp.addInt(repoDefinition.repoRule().recordedRepoMappingEntries().cellSet().size()); repoDefinition diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java index bfbab75b949330..fb40b771c0d401 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java @@ -55,6 +55,7 @@ import com.google.devtools.build.lib.skyframe.AlreadyReportedException; import com.google.devtools.build.lib.skyframe.IgnoredSubdirectoriesValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -69,7 +70,6 @@ import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WorkerSkyKeyComputeState; import java.io.IOException; -import java.util.Map; import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; @@ -89,8 +89,8 @@ public final class RepositoryFetchFunction implements SkyFunction { private final BlazeDirectories directories; private final LocalRepoContentsCache repoContentsCache; - private final Supplier> repoEnvironmentSupplier; - private final Supplier> clientEnvironmentSupplier; + private final Supplier> repoEnvSupplier; + private final Supplier> nonstrictRepoEnvSupplier; private double timeoutScaling = 1.0; @Nullable private DownloadManager downloadManager; @@ -100,12 +100,12 @@ public final class RepositoryFetchFunction implements SkyFunction { @Nullable private SyscallCache syscallCache; public RepositoryFetchFunction( - Supplier> repoEnvironmentSupplier, - Supplier> clientEnvironmentSupplier, + Supplier> repoEnvSupplier, + Supplier> nonstrictRepoEnvSupplier, BlazeDirectories directories, LocalRepoContentsCache repoContentsCache) { - this.repoEnvironmentSupplier = repoEnvironmentSupplier; - this.clientEnvironmentSupplier = clientEnvironmentSupplier; + this.repoEnvSupplier = repoEnvSupplier; + this.nonstrictRepoEnvSupplier = nonstrictRepoEnvSupplier; this.directories = directories; this.repoContentsCache = repoContentsCache; } @@ -585,7 +585,7 @@ private FetchResult fetchInternal( StarlarkCallable function = repoDefinition.repoRule().impl(); ImmutableMap> envVarValues = - RepositoryUtils.getEnvVarValues(env, repoDefinition.repoRule().environ()); + RepoEnvironmentFunction.getEnvironmentView(env, repoDefinition.repoRule().environ()); if (envVarValues == null) { return null; } @@ -628,8 +628,8 @@ private FetchResult fetchInternal( outputDirectory, ignoredSubdirectories.asIgnoredSubdirectories(), env, - ImmutableMap.copyOf(repoEnvironmentSupplier.get()), - ImmutableMap.copyOf(clientEnvironmentSupplier.get()), + repoEnvSupplier.get(), + ImmutableMap.copyOf(nonstrictRepoEnvSupplier.get()), downloadManager, timeoutScaling, processWrapper, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryUtils.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryUtils.java index a135db7f8ecaa0..602727b0bf4c46 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryUtils.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryUtils.java @@ -14,23 +14,16 @@ package com.google.devtools.build.lib.bazel.repository; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSortedMap; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; -import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; import com.google.devtools.build.lib.skyframe.PackageLookupValue; -import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; -import java.util.Map; -import java.util.Optional; -import java.util.Set; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Starlark; @@ -69,34 +62,6 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env) return RootedPath.toRootedPath(packageRoot, label.toPathFragment()); } - /** - * Returns the values of the environment variables in the given set as an immutable map while - * registering them as dependencies. - */ - @Nullable - public static ImmutableSortedMap> getEnvVarValues( - Environment env, Set keys) throws InterruptedException { - Map> environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); - if (environ == null) { - return null; - } - Map repoEnvOverride = PrecomputedValue.REPO_ENV.get(env); - if (repoEnvOverride == null) { - return null; - } - - // Only depend on --repo_env values that are specified in keys. - ImmutableMap.Builder> repoEnv = ImmutableMap.builder(); - repoEnv.putAll(environ); - for (String key : keys) { - String value = repoEnvOverride.get(key); - if (value != null) { - repoEnv.put(key, Optional.of(value)); - } - } - return ImmutableSortedMap.copyOf(repoEnv.buildKeepingLast()); - } - protected static Path getExternalRepositoryDirectory(BlazeDirectories directories) { return directories.getOutputBase().getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 9feb1fd4cbf3af..0bd084023108df 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD @@ -43,8 +43,8 @@ java_library( "//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event", "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input", "//src/main/java/com/google/devtools/build/lib/shell", - "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repo_environment_function", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", "//src/main/java/com/google/devtools/build/lib/unsafe:string", "//src/main/java/com/google/devtools/build/lib/util", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java index 6ee0b9abcd7d31..83c9a95d59bb25 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java @@ -60,6 +60,7 @@ import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult; +import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction; import com.google.devtools.build.lib.unsafe.StringUnsafe; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.io.OutErr; @@ -158,8 +159,8 @@ private interface AsyncTask extends SilentCloseable { protected final Path workingDirectory; protected final BlazeDirectories directories; protected final Environment env; - protected final ImmutableMap repoEnvVariables; - protected final ImmutableMap clientEnvVariables; + protected final ImmutableMap repoEnv; + protected final ImmutableMap nonstrictRepoEnv; private final StarlarkOS osObject; protected final DownloadManager downloadManager; protected final double timeoutScaling; @@ -180,8 +181,8 @@ protected StarlarkBaseExternalContext( Path workingDirectory, BlazeDirectories directories, Environment env, - Map repoEnvVariables, - Map clientEnvVariables, + ImmutableMap repoEnv, + ImmutableMap nonstrictRepoEnv, DownloadManager downloadManager, double timeoutScaling, @Nullable ProcessWrapper processWrapper, @@ -192,9 +193,9 @@ protected StarlarkBaseExternalContext( this.workingDirectory = workingDirectory; this.directories = directories; this.env = env; - this.repoEnvVariables = ImmutableMap.copyOf(repoEnvVariables); - this.clientEnvVariables = ImmutableMap.copyOf(clientEnvVariables); - this.osObject = new StarlarkOS(this.repoEnvVariables); + this.repoEnv = repoEnv; + this.nonstrictRepoEnv = nonstrictRepoEnv; + this.osObject = new StarlarkOS(this.repoEnv); this.downloadManager = downloadManager; this.timeoutScaling = timeoutScaling; this.processWrapper = processWrapper; @@ -836,7 +837,7 @@ public Object download( canonicalId, Optional.empty(), outputPath.getPath(), - clientEnvVariables, + nonstrictRepoEnv, identifyingStringForLogging, downloadPhaser, // The repo rule may modify the file after the download, so we cannot guarantee that @@ -1087,7 +1088,7 @@ public StructImpl downloadAndExtract( canonicalId, Optional.of(type), downloadDirectory, - clientEnvVariables, + nonstrictRepoEnv, identifyingStringForLogging, downloadPhaser, // The archive is not going to be modified and not accessible to the user, so its safe @@ -1475,18 +1476,13 @@ private static T nullIfNone(Object object, Class type) { @Nullable public String getEnvironmentValue(String name, Object defaultValue) throws InterruptedException, NeedsSkyframeRestartException { - // Must look up via Skyframe, rather than solely copy from `this.repoEnvVariables`, in order to - // establish a SkyKey dependency relationship. - var nameAndValue = RepositoryUtils.getEnvVarValues(env, ImmutableSet.of(name)); + var nameAndValue = RepoEnvironmentFunction.getEnvironmentView(env, ImmutableSet.of(name)); if (nameAndValue == null) { - return null; + throw new NeedsSkyframeRestartException(); } - // However, to account for --repo_env we take the value from `this.repoEnvVariables`. - // See https://github.com/bazelbuild/bazel/pull/20787#discussion_r1445571248 . var entry = Iterables.getOnlyElement(RepoRecordedInput.EnvVar.wrap(nameAndValue).entrySet()); - String envVarValue = repoEnvVariables.get(name); - recordInput(entry.getKey(), envVarValue); - return envVarValue != null ? envVarValue : nullIfNone(defaultValue, String.class); + recordInput(entry.getKey(), entry.getValue().orElse(null)); + return entry.getValue().orElseGet(() -> nullIfNone(defaultValue, String.class)); } @StarlarkMethod( @@ -1995,7 +1991,7 @@ public StarlarkExecutionResult execute( WorkspaceRuleEvent.newExecuteEvent( args, timeout, - Maps.filterKeys(repoEnvVariables, k -> !removeRepoEnvVariables.contains(k)), + Maps.filterKeys(repoEnv, k -> !removeRepoEnvVariables.contains(k)), forceRepoEnvVariables, workingDirectory.getPathString(), quiet, @@ -2293,7 +2289,7 @@ public StarlarkPath which(String program, StarlarkThread thread) throws EvalExce @Nullable private StarlarkPath findCommandOnPath(String program) throws IOException { - String pathEnvVariable = repoEnvVariables.get("PATH"); + String pathEnvVariable = repoEnv.get("PATH"); if (pathEnvVariable == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkOS.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkOS.java index f4a165e7111c49..0bed6b4133ee90 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkOS.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkOS.java @@ -18,7 +18,6 @@ import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import java.util.Locale; -import java.util.Map; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.StarlarkValue; @@ -31,10 +30,10 @@ @Immutable final class StarlarkOS implements StarlarkValue { - private final ImmutableMap environ; + private final ImmutableMap repoEnv; - StarlarkOS(Map environ) { - this.environ = ImmutableMap.copyOf(environ); + StarlarkOS(ImmutableMap repoEnv) { + this.repoEnv = repoEnv; } @Override @@ -55,7 +54,7 @@ public boolean isImmutable() { module_ctx.getenv instead. """) public ImmutableMap getEnvironmentVariables() { - return environ; + return repoEnv; } @StarlarkMethod( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index e406ffaa79e6e7..9798363771a8c2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java @@ -87,8 +87,8 @@ public StarlarkRepositoryContext( Path outputDirectory, IgnoredSubdirectories ignoredSubdirectories, Environment environment, - ImmutableMap repoEnvVariables, - ImmutableMap clientEnvVariables, + ImmutableMap repoEnv, + ImmutableMap nonstrictRepoEnv, DownloadManager downloadManager, double timeoutScaling, @Nullable ProcessWrapper processWrapper, @@ -101,8 +101,8 @@ public StarlarkRepositoryContext( outputDirectory, directories, environment, - repoEnvVariables, - clientEnvVariables, + repoEnv, + nonstrictRepoEnv, downloadManager, timeoutScaling, processWrapper, diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD index fa4fb579ae54ae..4da7144272dff7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -177,12 +177,12 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator", - "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_value", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:environment_variable_value", "//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys", "//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/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/util", diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java index d59288952d6a51..71ac30b344d2ae 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java @@ -31,12 +31,12 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; -import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue; import com.google.devtools.build.lib.skyframe.DirectoryListingKey; import com.google.devtools.build.lib.skyframe.DirectoryListingValue; import com.google.devtools.build.lib.skyframe.DirectoryTreeDigestValue; +import com.google.devtools.build.lib.skyframe.EnvironmentVariableValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.Fingerprint; @@ -629,17 +629,14 @@ public String toStringInternal() { @Override public SkyKey getSkyKey(BlazeDirectories directories) { - return ActionEnvironmentFunction.key(name); + return RepoEnvironmentFunction.key(name); } @Override public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { - String v = PrecomputedValue.REPO_ENV.get(env).get(name); - if (v == null) { - v = ((ClientEnvironmentValue) env.getValue(getSkyKey(directories))).getValue(); - } + String v = ((EnvironmentVariableValue) env.getValue(getSkyKey(directories))).value(); // Note that `oldValue` can be null if the env var was not set. if (!Objects.equals(oldValue, v)) { return Optional.of( diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 99eb68b49af055..f1f9f0ec4b89ee 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -21,7 +21,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Maps; import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.ActionOutputDirectoryHelper; @@ -101,6 +103,9 @@ *

This class is non-final for mocking purposes. DO NOT extend it in production code. */ public class CommandEnvironment { + private static final ImmutableSet ALWAYS_INHERITED_REPO_ENV = + OS.getCurrent() == OS.WINDOWS ? ImmutableSet.of("PATH", "PATHEXT") : ImmutableSet.of("PATH"); + private final BlazeRuntime runtime; private final BlazeWorkspace workspace; private final BlazeDirectories directories; @@ -114,8 +119,8 @@ public class CommandEnvironment { private final ImmutableMap clientEnv; private final Set visibleActionEnv = new TreeSet<>(); private final Set visibleTestEnv = new TreeSet<>(); - private final Map repoEnv = new TreeMap<>(); - private final Map repoEnvFromOptions = new TreeMap<>(); + private final ImmutableMap repoEnv; + private final ImmutableMap nonstrictRepoEnv; private final TimestampGranularityMonitor timestampGranularityMonitor; private final Thread commandThread; private final Command command; @@ -334,19 +339,12 @@ public void exit(AbruptExitException exception) { ? buildRequestIdOverride : UUID.randomUUID().toString(); - this.repoEnv.putAll(clientEnv); - - Set defaultRepoEnvInherited = new TreeSet<>(); - defaultRepoEnvInherited.add("PATH"); - if (OS.getCurrent() == OS.WINDOWS) { - defaultRepoEnvInherited.add("PATHEXT"); - } - for (String name : defaultRepoEnvInherited) { - String value = clientEnv.get(name); - if (value != null) { - this.repoEnvFromOptions.put(name, value); - } - } + var repoEnvBuilder = + new TreeMap<>( + commandOptions.useStrictRepoEnv + ? Maps.filterKeys(clientEnv, ALWAYS_INHERITED_REPO_ENV::contains) + : clientEnv); + var nonstrictRepoEnvBuilder = new TreeMap<>(clientEnv); // TODO: This only needs to check for loads() rather than analyzes() due to // the effect of --action_env on the repository env. Revert back to @@ -359,8 +357,8 @@ public void exit(AbruptExitException exception) { case Converters.EnvVar.Set(String name, String value) -> { visibleActionEnv.remove(name); if (!options.getOptions(CommonCommandOptions.class).repoEnvIgnoresActionEnv) { - repoEnv.put(name, value); - repoEnvFromOptions.put(name, value); + repoEnvBuilder.put(name, value); + nonstrictRepoEnvBuilder.put(name, value); } } case Converters.EnvVar.Inherit(String name) -> { @@ -369,8 +367,8 @@ public void exit(AbruptExitException exception) { case Converters.EnvVar.Unset(String name) -> { visibleActionEnv.remove(name); if (!options.getOptions(CommonCommandOptions.class).repoEnvIgnoresActionEnv) { - repoEnv.remove(name); - repoEnvFromOptions.remove(name); + repoEnvBuilder.remove(name); + nonstrictRepoEnvBuilder.remove(name); } } } @@ -398,19 +396,21 @@ public void exit(AbruptExitException exception) { if (bazelWorkspace != null) { value = value.replace("%bazel_workspace%", bazelWorkspace); } - repoEnv.put(name, value); - repoEnvFromOptions.put(name, value); + repoEnvBuilder.put(name, value); + nonstrictRepoEnvBuilder.put(name, value); } case Converters.EnvVar.Inherit(String name) -> { - repoEnv.put(name, clientEnv.get(name)); - repoEnvFromOptions.put(name, clientEnv.get(name)); + repoEnvBuilder.put(name, clientEnv.get(name)); + nonstrictRepoEnvBuilder.put(name, clientEnv.get(name)); } case Converters.EnvVar.Unset(String name) -> { - repoEnv.remove(name); - repoEnvFromOptions.remove(name); + repoEnvBuilder.remove(name); + nonstrictRepoEnvBuilder.remove(name); } } } + this.repoEnv = ImmutableMap.copyOf(repoEnvBuilder); + this.nonstrictRepoEnv = ImmutableMap.copyOf(nonstrictRepoEnvBuilder); this.buildResultListener = new BuildResultListener(); this.eventBus.register(this.buildResultListener); @@ -854,7 +854,6 @@ public void syncPackageLoading(OptionsProvider options) packageLocator, commandId, clientEnv, - repoEnvFromOptions, timestampGranularityMonitor, quiescingExecutors, options, @@ -987,21 +986,40 @@ public String determineOutputFileSystem() { } /** - * Returns the repository environment created from the client environment, {@code --repo_env}, and - * {@code --action_env=NAME=VALUE} (when {@code - * --incompatible_repo_env_ignores_action_env=false}). + * Returns the environment for repository rules and module extensions, which is constructed as + * follows: + * + *

    + *
  • the client environment as the base; + *
  • if {@code --experimental_strict_repo_env} is set, only the variables {@code PATH} and, on + * Windows only, {@code PATHEXT} are kept; + *
  • if {@code --noincompatible_repo_env_ignores_action_env} is set, {@code --action_env} is + * applied on top of that; + *
  • finally, {@code --repo_env} is applied on top of that. + *
*/ - public Map getRepoEnv() { - return Collections.unmodifiableMap(repoEnv); + public ImmutableMap getRepoEnv() { + return repoEnv; } /** - * Returns the repository environment created from specific client environment variables ({@code - * PATH} and on Windows {@code PATH_EXT}), {@code --repo_env}, and {@code --action_env=NAME=VALUE} - * (when {@code --incompatible_repo_env_ignores_action_env=false}). + * Returns the environment for inherently local, non-hermetic operations associated with + * repository rules and module extensions, such as credential helpers. It is constructed as + * follows: + * + *
    + *
  • the client environment as the base; + *
  • if {@code --noincompatible_repo_env_ignores_action_env} is set, {@code --action_env} is + * applied on top of that; + *
  • finally, {@code --repo_env} is applied on top of that. + *
+ * + *

This differs from {@link #getRepoEnv()} in that it does not apply {@code + * --experimental_strict_repo_env}, and thus always includes the full client environment as a + * base. */ - public Map getRepoEnvFromOptions() { - return Collections.unmodifiableMap(repoEnvFromOptions); + public ImmutableMap getNonstrictRepoEnv() { + return nonstrictRepoEnv; } /** Returns the file cache to use during this build. */ diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionEnvironmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionEnvironmentFunction.java index 7e5988455a5138..85910bc50cffd5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionEnvironmentFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionEnvironmentFunction.java @@ -43,7 +43,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept Map actionEnv = PrecomputedValue.ACTION_ENV.get(env); String key = (String) skyKey.argument(); if (actionEnv.containsKey(key) && actionEnv.get(key) != null) { - return new ClientEnvironmentValue(actionEnv.get(key)); + return new EnvironmentVariableValue(actionEnv.get(key)); } return env.getValue(ClientEnvironmentFunction.key(key)); } @@ -99,14 +99,14 @@ public static ImmutableMap> getEnvironmentView( ImmutableMap.Builder> result = ImmutableMap.builderWithExpectedSize(skyframeKeys.size()); for (SkyKey key : skyframeKeys) { - ClientEnvironmentValue value = (ClientEnvironmentValue) values.get(key); + EnvironmentVariableValue value = (EnvironmentVariableValue) values.get(key); if (value == null) { BugReport.sendBugReport( new IllegalStateException( "ClientEnvironmentValue " + key + " was missing, this should never happen")); return null; } - result.put(key.argument().toString(), Optional.ofNullable(value.getValue())); + result.put(key.argument().toString(), Optional.ofNullable(value.value())); } return result.buildOrThrow(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 52e537bcf44b2c..f64f60893c5212 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -232,9 +232,9 @@ private SkyValue computeInternal( ImmutableMap.Builder builder = ImmutableMap.builderWithExpectedSize(clientEnvironmentVariablesSet.size()); for (SkyKey depKey : depKeys) { - ClientEnvironmentValue envValue = (ClientEnvironmentValue) clientEnvLookup.get(depKey); - if (envValue.getValue() != null) { - builder.put((String) depKey.argument(), envValue.getValue()); + EnvironmentVariableValue envValue = (EnvironmentVariableValue) clientEnvLookup.get(depKey); + if (envValue.value() != null) { + builder.put((String) depKey.argument(), envValue.value()); } } clientEnv = builder.buildOrThrow(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 67a05d04d57b0a..0e7bc6c17e07df 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -112,7 +112,6 @@ java_library( ":cached_bzl_load_value_and_deps", ":cached_bzl_load_value_and_deps_builder_factory", ":client_environment_function", - ":client_environment_value", ":collect_packages_under_directory_function", ":collect_packages_under_directory_value", ":collect_targets_in_package_function", @@ -132,6 +131,7 @@ java_library( ":directory_listing_function", ":directory_listing_state_value", ":directory_tree_digest_function", + ":environment_variable_value", ":ephemeral_check_if_output_consumed", ":exclusive_test_build_driver_value", ":execution_finished_event", @@ -176,6 +176,7 @@ java_library( ":recursive_package_provider_backed_target_pattern_resolver", ":recursive_pkg_function", ":recursive_pkg_value", + ":repo_environment_function", ":repo_file_function", ":repo_package_args_function", ":repository_mapping_function", @@ -445,7 +446,7 @@ java_library( srcs = ["ActionEnvironmentFunction.java"], deps = [ ":client_environment_function", - ":client_environment_value", + ":environment_variable_value", ":precomputed_value", ":sky_functions", "//src/main/java/com/google/devtools/build/lib/bugreport", @@ -1047,7 +1048,7 @@ java_library( name = "client_environment_function", srcs = ["ClientEnvironmentFunction.java"], deps = [ - ":client_environment_value", + ":environment_variable_value", ":sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", @@ -1057,9 +1058,10 @@ java_library( ) java_library( - name = "client_environment_value", - srcs = ["ClientEnvironmentValue.java"], + name = "environment_variable_value", + srcs = ["EnvironmentVariableValue.java"], deps = [ + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//third_party:jsr305", ], @@ -3338,3 +3340,18 @@ java_library( "//third_party:guava", ], ) + +java_library( + name = "repo_environment_function", + srcs = ["RepoEnvironmentFunction.java"], + deps = [ + ":environment_variable_value", + ":precomputed_value", + ":sky_functions", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:guava", + "//third_party:jsr305", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java index 17871773d4e0b4..258df68b7634b8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java @@ -71,6 +71,6 @@ public ClientEnvironmentFunction(AtomicReference> clientEnv) @Nullable @Override public SkyValue compute(SkyKey key, Environment env) { - return new ClientEnvironmentValue(clientEnv.get().get((String) key.argument())); + return new EnvironmentVariableValue(clientEnv.get().get((String) key.argument())); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java deleted file mode 100644 index 99b98704352875..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2016 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. -// You may obtain a copy of the License at -// -// 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.skyframe; - -import com.google.devtools.build.skyframe.SkyValue; -import java.util.Objects; -import javax.annotation.Nullable; - -/** - * The value of an environmental variable from the client environment. These are invalidated and - * injected by {@link SequencedSkyframeExecutor}. - */ -public final class ClientEnvironmentValue implements SkyValue { - private final String value; - - public ClientEnvironmentValue(@Nullable String value) { - this.value = value; - } - - /** @return the value in the client environment or null if unset in the environment. */ - @Nullable - public String getValue() { - return value; - } - - @Override - public boolean equals(Object o) { - return o instanceof ClientEnvironmentValue clientEnvironmentValue - && Objects.equals(clientEnvironmentValue.value, value); - } - - @Override - public int hashCode() { - return (value != null) ? value.hashCode() : 0; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentVariableValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentVariableValue.java new file mode 100644 index 00000000000000..eea16f29607648 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentVariableValue.java @@ -0,0 +1,28 @@ +// Copyright 2026 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. +// You may obtain a copy of the License at +// +// 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.skyframe; + +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.skyframe.SkyValue; +import javax.annotation.Nullable; + +/** + * The value of an environmental variable from an environment (client env, action env or repository + * env). These are invalidated and injected by {@link SequencedSkyframeExecutor}. + * + * @param value the value in the client environment or null if unset in the environment. + */ +@AutoCodec +public record EnvironmentVariableValue(@Nullable String value) implements SkyValue {} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java index c9b4202c6bce81..ed71adc7515811 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java @@ -18,6 +18,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy; import com.google.devtools.build.lib.packages.RuleVisibility; @@ -90,7 +91,8 @@ public static Injected injected(Precomputed precomputed, T value) { public static final Precomputed> ACTION_ENV = new Precomputed<>("action_env"); - public static final Precomputed> REPO_ENV = new Precomputed<>("repo_env"); + public static final Precomputed> REPO_ENV = + new Precomputed<>("repo_env"); public static final Precomputed PATH_PACKAGE_LOCATOR = new Precomputed<>("path_package_locator"); @@ -100,7 +102,7 @@ public static Injected injected(Precomputed precomputed, T value) { // Unsharable because of complications in deserializing BuildOptions on startup due to caching. public static final Precomputed BASELINE_CONFIGURATION = - new Precomputed<>("baseline_configuration", /*shareable=*/ false); + new Precomputed<>("baseline_configuration", /* shareable= */ false); // Unsharable because of complications in deserializing BuildOptions on startup due to caching. public static final Precomputed BASELINE_EXEC_CONFIGURATION = @@ -116,9 +118,7 @@ public PrecomputedValue(Object value) { this.value = Preconditions.checkNotNull(value); } - /** - * Returns the value of the variable. - */ + /** Returns the value of the variable. */ public Object get() { return value; } @@ -150,7 +150,7 @@ public static final class Precomputed { private final SkyKey key; public Precomputed(String key) { - this(key, /*shareable=*/ true); + this(key, /* shareable= */ true); } private Precomputed(String key, boolean shareable) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoEnvironmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoEnvironmentFunction.java new file mode 100644 index 00000000000000..e20a526c16858a --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepoEnvironmentFunction.java @@ -0,0 +1,103 @@ +// Copyright 2026 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. +// You may obtain a copy of the License at +// +// 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.skyframe; + +import com.google.common.collect.Collections2; +import com.google.common.collect.ImmutableSortedMap; +import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.skyframe.AbstractSkyKey; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.SkyframeLookupResult; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import javax.annotation.Nullable; + +/** + * Skyframe function that provides the effective value for an environment variable in the context of + * repository rules and module extensions. This will be the value from the repo environment as + * provided by {@link com.google.devtools.build.lib.runtime.CommandEnvironment#getRepoEnv()}. + */ +public final class RepoEnvironmentFunction implements SkyFunction { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { + Map repoEnv = PrecomputedValue.REPO_ENV.get(env); + String key = (String) skyKey.argument(); + return new EnvironmentVariableValue(repoEnv.get(key)); + } + + /** Returns the SkyKey to invoke this function for the environment variable {@code variable}. */ + public static Key key(String variable) { + return Key.create(variable); + } + + @VisibleForSerialization + @AutoCodec + static class Key extends AbstractSkyKey { + private static final SkyKeyInterner interner = SkyKey.newInterner(); + + private Key(String arg) { + super(arg); + } + + private static Key create(String arg) { + return interner.intern(new Key(arg)); + } + + @VisibleForSerialization + @AutoCodec.Interner + static Key intern(Key key) { + return interner.intern(key); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE; + } + + @Override + public SkyKeyInterner getSkyKeyInterner() { + return interner; + } + } + + /** + * Returns a map of environment variable key => values, getting them from Skyframe. Returns null + * if and only if some dependencies from Skyframe still need to be resolved. + */ + @Nullable + public static ImmutableSortedMap> getEnvironmentView( + Environment env, Set keys) throws InterruptedException { + var skyKeys = Collections2.transform(keys, RepoEnvironmentFunction::key); + SkyframeLookupResult values = env.getValuesAndExceptions(skyKeys); + if (env.valuesMissing()) { + return null; + } + + var result = ImmutableSortedMap.>naturalOrder(); + for (var key : skyKeys) { + var value = (EnvironmentVariableValue) values.get(key); + if (value == null) { + return null; + } + result.put(key.argument().toString(), Optional.ofNullable(value.value())); + } + return result.buildOrThrow(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 80e0a5c21a2270..91933dd74c04fa 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -263,7 +263,6 @@ public WorkspaceInfoFromDiff sync( PathPackageLocator packageLocator, UUID commandId, Map clientEnv, - Map repoEnvOption, TimestampGranularityMonitor tsgm, QuiescingExecutors executors, OptionsProvider options, @@ -296,7 +295,6 @@ public WorkspaceInfoFromDiff sync( packageLocator, commandId, clientEnv, - repoEnvOption, tsgm, executors, options, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index fc679862749669..2ee9631cd03f31 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -25,6 +25,8 @@ public final class SkyFunctions { SkyFunctionName.createNonHermetic("CLIENT_ENVIRONMENT_VARIABLE"); public static final SkyFunctionName ACTION_ENVIRONMENT_VARIABLE = SkyFunctionName.createHermetic("ACTION_ENVIRONMENT_VARIABLE"); + public static final SkyFunctionName REPOSITORY_ENVIRONMENT_VARIABLE = + SkyFunctionName.createHermetic("REPOSITORY_ENVIRONMENT_VARIABLE"); public static final SkyFunctionName DIRECTORY_LISTING_STATE = SkyFunctionName.createNonHermetic("DIRECTORY_LISTING_STATE"); public static final SkyFunctionName DIRECTORY_LISTING = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index eed51785ec7235..c2c239c96aca90 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -769,6 +769,7 @@ private ImmutableMap skyFunctions() { map.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()); map.put(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction(clientEnv)); map.put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction()); + map.put(SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE, new RepoEnvironmentFunction()); map.put(FileStateKey.FILE_STATE, newFileStateFunction()); map.put(SkyFunctions.DIRECTORY_LISTING_STATE, newDirectoryListingStateFunction()); map.put(FileSymlinkCycleUniquenessFunction.NAME, new FileSymlinkCycleUniquenessFunction()); @@ -2936,7 +2937,6 @@ public WorkspaceInfoFromDiff sync( PathPackageLocator pathPackageLocator, UUID commandId, Map clientEnv, - Map repoEnvOption, TimestampGranularityMonitor tsgm, QuiescingExecutors executors, OptionsProvider options, @@ -2946,7 +2946,6 @@ public WorkspaceInfoFromDiff sync( getActionEnvFromOptions(options.getOptions(CoreOptions.class)); var platformOptions = options.getOptions(PlatformOptions.class); platformMappingKey = platformOptions != null ? platformOptions.platformMappingKey : null; - PrecomputedValue.REPO_ENV.set(injectable(), new LinkedHashMap<>(repoEnvOption)); RemoteOptions remoteOptions = options.getOptions(RemoteOptions.class); setRemoteExecutionEnabled(remoteOptions != null && remoteOptions.isRemoteExecutionEnabled()); cpuBoundSemaphore.set(getUpdatedSkyFunctionsSemaphore(options)); @@ -3790,7 +3789,7 @@ private void handleClientEnvironmentChanges() { for (Map.Entry entry : clientEnv.get().entrySet()) { newValuesBuilder.put( ClientEnvironmentFunction.key(entry.getKey()), - Delta.justNew(new ClientEnvironmentValue(entry.getValue()))); + Delta.justNew(new EnvironmentVariableValue(entry.getValue()))); } recordingDiffer.inject(newValuesBuilder.buildOrThrow()); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD index 332d7d1accc074..bf6c09b76e8f00 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD @@ -104,7 +104,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", - "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:repo_environment_function", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/vfs", 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 7a487089c26b28..210a42b086df81 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 @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.RepoEnvironmentFunction; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; @@ -183,6 +184,7 @@ public BazelPackageLoader buildImpl() { SkyFunctions.DIRECTORY_LISTING_STATE, new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)) .put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction()) + .put(SkyFunctions.REPOSITORY_ENVIRONMENT_VARIABLE, new RepoEnvironmentFunction()) .put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()) .put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction()) .put(SkyFunctions.REPOSITORY_DIRECTORY, repositoryFetchFunction) diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java index 5d43d7bb649c3f..bc46451215860c 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java @@ -357,7 +357,6 @@ private void initTargetPatternEvaluator(ConfiguredRuleClassProvider ruleClassPro packageLocator, UUID.randomUUID(), ImmutableMap.of(), - ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance()), QuiescingExecutorsImpl.forTesting(), FakeOptions.builder().put(packageOptions).put(buildLanguageOptions).build(), diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java index 262dba90c75b97..c841acab79f893 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java @@ -340,7 +340,6 @@ private void initEvaluator() pathPackageLocator, UUID.randomUUID(), /* clientEnv= */ ImmutableMap.of(), - /* repoEnvOption= */ ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance()), QuiescingExecutorsImpl.forTesting(), FakeOptions.builder().put(packageOptions).put(options).build(), diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java index 3d2189acb3f39e..ebf8d05b5cf690 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java @@ -2715,7 +2715,6 @@ private void syncSkyframeExecutor() throws InterruptedException, AbruptExitExcep createPackageLocator(), UUID.randomUUID(), /* clientEnv= */ ImmutableMap.of(), - /* repoEnvOption= */ ImmutableMap.of(), tsgm, QuiescingExecutorsImpl.forTesting(), options, diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh index 6647dcac9504b1..fad8c558af69b7 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh @@ -2891,6 +2891,135 @@ EOF expect_log "LAZYEVAL_KEY=xal3" } +function test_environ_incrementally_without_client_env_without_action_env() { + cat > repo.bzl < BUILD.dummy < $(setup_module_dot_bazel) <$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=None" + expect_log "MY_VAR=None" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_not_log "PREDECLARED_KEY" + expect_not_log "MY_VAR" + + bazel query --action_env=PREDECLARED_KEY=val3 --action_env=MY_VAR=val4 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_not_log "PREDECLARED_KEY" + expect_not_log "MY_VAR" +} + +function test_environ_incrementally_with_client_env_without_action_env() { + cat > repo.bzl < BUILD.dummy < $(setup_module_dot_bazel) <$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=None" + expect_log "MY_VAR=None" + + PREDECLARED_KEY=val1 bazel query @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=val1" + expect_log "MY_VAR=None" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=val1" + expect_log "MY_VAR=val2" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query --action_env=PREDECLARED_KEY=val3 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_not_log "PREDECLARED_KEY" + expect_not_log "MY_VAR" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query --action_env=PREDECLARED_KEY=val3 --action_env=MY_VAR=val4 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_not_log "PREDECLARED_KEY" + expect_not_log "MY_VAR" + + PREDECLARED_KEY=val5 MY_VAR=val6 bazel query --action_env=PREDECLARED_KEY=val3 --action_env=MY_VAR=val4 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=val5" + expect_log "MY_VAR=val6" +} + +function test_environ_incrementally_without_client_env_with_action_env() { + cat > repo.bzl < BUILD.dummy < $(setup_module_dot_bazel) <$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=None" + expect_log "MY_VAR=None" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_not_log "PREDECLARED_KEY" + expect_not_log "MY_VAR" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query --action_env=PREDECLARED_KEY=val3 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=val3" + expect_log "MY_VAR=None" + + PREDECLARED_KEY=val1 MY_VAR=val2 bazel query --action_env=PREDECLARED_KEY=val3 --action_env=MY_VAR=val4 @foo//:BUILD 2>$TEST_log || fail 'Expected query to succeed' + expect_log "PREDECLARED_KEY=val3" + expect_log "MY_VAR=val4" +} + function test_environ_build_query_build() { # Set up workspace with a repository rule that depends on env vars. # Assert that the repo rule doesn't rerun when performing a sequence of