From 64554a32d0836bf8c5eaa5d5f2bebb7163ac73bc Mon Sep 17 00:00:00 2001 From: salma-samy Date: Tue, 18 Jul 2023 04:42:43 -0700 Subject: [PATCH] Add 'environ' attribute to module extensions and update lockfile - Add 'environ' to module extensions - Store the variables and their values in the lockfile - Compare the variables and values with the lockfile and re-evaluate or error if they differ PiperOrigin-RevId: 548964193 Change-Id: Ic2d52fe3332e93095c414d8bca4c9b4312bca8c2 --- .../lib/bazel/bzlmod/BazelLockFileValue.java | 22 ++- .../bazel/bzlmod/LockFileModuleExtension.java | 9 +- .../lib/bazel/bzlmod/ModuleExtension.java | 5 + .../bzlmod/SingleExtensionEvalFunction.java | 26 +++- .../starlark/StarlarkRepositoryModule.java | 2 + .../rules/repository/RepositoryFunction.java | 52 +++---- .../skyframe/ActionEnvironmentFunction.java | 19 +-- .../skyframe/ClientEnvironmentFunction.java | 5 +- .../repository/RepositoryModuleApi.java | 17 ++- .../repository/FakeRepositoryModule.java | 9 +- .../bazel/bzlmod/StarlarkBazelModuleTest.java | 4 +- .../py/bazel/bzlmod/bazel_lockfile_test.py | 129 ++++++++++++++---- 12 files changed, 219 insertions(+), 80 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index c0d0c06aed41e4..415c31cb62af2f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -114,20 +114,28 @@ public ImmutableList getModuleExtensionDiff( ImmutableMap lockedExtensionUsages, ModuleExtensionId extensionId, byte[] transitiveDigest, + ImmutableMap envVariables, ImmutableMap extensionUsages) { ImmutableList.Builder extDiff = new ImmutableList.Builder<>(); if (lockedExtension == null) { - extDiff.add("The module extension '" + extensionId + "' does not exist in the lockfile"); - } else { - if (!Arrays.equals(transitiveDigest, lockedExtension.getBzlTransitiveDigest())) { + return extDiff + .add("The module extension '" + extensionId + "' does not exist in the lockfile") + .build(); + } + if (!Arrays.equals(transitiveDigest, lockedExtension.getBzlTransitiveDigest())) { extDiff.add( "The implementation of the extension '" + extensionId + "' or one of its transitive .bzl files has changed"); - } - if (!extensionUsages.equals(lockedExtensionUsages)) { - extDiff.add("The usages of the extension named '" + extensionId + "' has changed"); - } + } + if (!extensionUsages.equals(lockedExtensionUsages)) { + extDiff.add("The usages of the extension '" + extensionId + "' has changed"); + } + if (!envVariables.equals(lockedExtension.getEnvVariables())) { + extDiff.add( + "The environment variables the extension '" + + extensionId + + "' depends on (or their values) have changed"); } return extDiff.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java index 5431d6cf0fabe6..84b9581d74ebf2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java @@ -31,10 +31,15 @@ public abstract class LockFileModuleExtension implements Postable { @SuppressWarnings("mutable") public abstract byte[] getBzlTransitiveDigest(); + public abstract ImmutableMap getEnvVariables(); + public abstract ImmutableMap getGeneratedRepoSpecs(); public static LockFileModuleExtension create( - byte[] transitiveDigest, ImmutableMap generatedRepoSpecs) { - return new AutoValue_LockFileModuleExtension(transitiveDigest, generatedRepoSpecs); + byte[] transitiveDigest, + ImmutableMap envVariables, + ImmutableMap generatedRepoSpecs) { + return new AutoValue_LockFileModuleExtension( + transitiveDigest, envVariables, generatedRepoSpecs); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java index 414ea7dcae63c0..2de6b0db91d4e5 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.bazel.bzlmod; import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.Label; import java.util.Optional; @@ -44,6 +45,8 @@ public abstract class ModuleExtension implements StarlarkValue { public abstract Location getLocation(); + public abstract ImmutableList getEnvVariables(); + public static Builder builder() { return new AutoValue_ModuleExtension.Builder(); } @@ -61,6 +64,8 @@ public abstract static class Builder { public abstract Builder setTagClasses(ImmutableMap value); + public abstract Builder setEnvVariables(ImmutableList value); + public abstract ModuleExtension build(); } } 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 e272c0bdfd4fe9..3de892ef50f68d 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 @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps; @@ -144,9 +145,16 @@ public SkyValue compute(SkyKey skyKey, Environment env) Transience.PERSISTENT); } - // Check the lockfile first for that module extension + ModuleExtension extension = (ModuleExtension) exported; + ImmutableMap extensionEnvVars = + RepositoryFunction.getEnvVarValues(env, extension.getEnvVariables()); + if (extensionEnvVars == null) { + return null; + } byte[] bzlTransitiveDigest = BazelModuleContext.of(bzlLoadValue.getModule()).bzlTransitiveDigest(); + + // Check the lockfile first for that module extension LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); if (!lockfileMode.equals(LockfileMode.OFF)) { BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); @@ -155,14 +163,18 @@ public SkyValue compute(SkyKey skyKey, Environment env) } SingleExtensionEvalValue singleExtensionEvalValue = tryGettingValueFromLockFile( - extensionId, usagesValue, bzlTransitiveDigest, lockfileMode, lockfile); + extensionId, + extensionEnvVars, + usagesValue, + bzlTransitiveDigest, + lockfileMode, + lockfile); if (singleExtensionEvalValue != null) { return singleExtensionEvalValue; } } // Run that extension! - ModuleExtension extension = (ModuleExtension) exported; ImmutableMap generatedRepoSpecs = runModuleExtension( extensionId, extension, usagesValue, bzlLoadValue.getModule(), starlarkSemantics, env); @@ -177,7 +189,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) .post( ModuleExtensionResolutionEvent.create( extensionId, - LockFileModuleExtension.create(bzlTransitiveDigest, generatedRepoSpecs))); + LockFileModuleExtension.create( + bzlTransitiveDigest, extensionEnvVars, generatedRepoSpecs))); } return createSingleExtentionValue(generatedRepoSpecs, usagesValue); } @@ -185,6 +198,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) @Nullable private SingleExtensionEvalValue tryGettingValueFromLockFile( ModuleExtensionId extensionId, + ImmutableMap envVariables, SingleExtensionUsagesValue usagesValue, byte[] bzlTransitiveDigest, LockfileMode lockfileMode, @@ -205,7 +219,8 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( // If we have the extension, check if the implementation and usage haven't changed if (lockedExtension != null && Arrays.equals(bzlTransitiveDigest, lockedExtension.getBzlTransitiveDigest()) - && usagesValue.getExtensionUsages().equals(lockedExtensionUsages)) { + && usagesValue.getExtensionUsages().equals(lockedExtensionUsages) + && envVariables.equals(lockedExtension.getEnvVariables())) { return createSingleExtentionValue(lockedExtension.getGeneratedRepoSpecs(), usagesValue); } else if (lockfileMode.equals(LockfileMode.ERROR)) { ImmutableList extDiff = @@ -214,6 +229,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( lockedExtensionUsages, extensionId, bzlTransitiveDigest, + envVariables, usagesValue.getExtensionUsages()); throw new SingleExtensionEvalFunctionException( ExternalDepsException.withMessage( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 849ce81b357b96..91e2ecd56ae145 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -291,6 +291,7 @@ public Object moduleExtension( StarlarkCallable implementation, Dict tagClasses, // Dict Object doc, // or Starlark.NONE + Sequence environ, // StarlarkThread thread) throws EvalException { return ModuleExtension.builder() @@ -300,6 +301,7 @@ public Object moduleExtension( .setDoc(Starlark.toJavaOptional(doc, String.class)) .setDefiningBzlFileLabel( BzlInitThreadContext.fromOrFailFunction(thread, "module_extension").getBzlFile()) + .setEnvVariables(ImmutableList.copyOf(Sequence.cast(environ, String.class, "environ"))) .setLocation(thread.getCallerLocation()) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index 1854bc12159c9d..1a24d9b28b03f9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -20,6 +20,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -51,6 +52,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import java.io.IOException; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; @@ -188,9 +190,9 @@ public abstract RepositoryDirectoryValue.Builder fetch( throws InterruptedException, RepositoryFunctionException; @SuppressWarnings("unchecked") - private static Iterable getEnviron(Rule rule) { + private static Collection getEnviron(Rule rule) { if (rule.isAttrDefined("$environ", Type.STRING_LIST)) { - return (Iterable) rule.getAttr("$environ"); + return (Collection) rule.getAttr("$environ"); } return ImmutableList.of(); } @@ -201,7 +203,7 @@ private static Iterable getEnviron(Rule rule) { * is needed. */ public boolean verifyMarkerData(Rule rule, Map markerData, Environment env) - throws InterruptedException, RepositoryFunctionException { + throws InterruptedException { return verifyEnvironMarkerData(markerData, env, getEnviron(rule)) && verifyMarkerDataForFiles(rule, markerData, env) && verifySemanticsMarkerData(markerData, env); @@ -306,33 +308,37 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env) protected Map declareEnvironmentDependencies( Map markerData, Environment env, Iterable keys) throws InterruptedException { - Map environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); + ImmutableMap envDep = getEnvVarValues(env, keys); + if (envDep == null) { + return null; + } + // Add the dependencies to the marker file + keys.forEach(key -> markerData.put("ENV:" + key, envDep.get(key))); + return envDep; + } - // Returns true if there is a null value and we need to wait for some dependencies. + @Nullable + public static ImmutableMap getEnvVarValues(Environment env, Iterable keys) + throws InterruptedException { + ImmutableMap 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 the "environ" attribute. - Map repoEnv = new LinkedHashMap(environ); + ImmutableMap.Builder repoEnv = ImmutableMap.builder(); + repoEnv.putAll(environ); for (String key : keys) { String value = repoEnvOverride.get(key); if (value != null) { repoEnv.put(key, value); } } - - // Add the dependencies to the marker file - for (Map.Entry value : repoEnv.entrySet()) { - markerData.put("ENV:" + value.getKey(), value.getValue()); - } - - return repoEnv; + return repoEnv.buildKeepingLast(); } /** @@ -341,9 +347,9 @@ protected Map declareEnvironmentDependencies( * Environment)} function to verify the values for environment variables. */ protected boolean verifyEnvironMarkerData( - Map markerData, Environment env, Iterable keys) + Map markerData, Environment env, Collection keys) throws InterruptedException { - Map environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); + ImmutableMap environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); if (env.valuesMissing()) { return false; // Returns false so caller knows to return immediately } @@ -364,17 +370,18 @@ protected boolean verifyEnvironMarkerData( // Verify that all environment variable in the marker file are also in keys for (String key : markerData.keySet()) { - if (key.startsWith("ENV:") && !repoEnv.containsKey(key.substring(4))) { + if (key.startsWith("ENV:") && !keys.contains(key.substring(4))) { return false; } } + // Now verify the values of the marker data - for (Map.Entry value : repoEnv.entrySet()) { - if (!markerData.containsKey("ENV:" + value.getKey())) { + for (String key : keys) { + if (!markerData.containsKey("ENV:" + key)) { return false; } - String markerValue = markerData.get("ENV:" + value.getKey()); - if (!Objects.equals(markerValue, value.getValue())) { + String markerValue = markerData.get("ENV:" + key); + if (!Objects.equals(markerValue, repoEnv.get(key))) { return false; } } @@ -457,8 +464,7 @@ protected static String getPathAttr(Rule rule) throws RepositoryFunctionExceptio } @VisibleForTesting - protected static PathFragment getTargetPath(String userDefinedPath, Path workspace) - throws RepositoryFunctionException { + protected static PathFragment getTargetPath(String userDefinedPath, Path workspace) { PathFragment pathFragment = PathFragment.create(userDefinedPath); return workspace.getRelative(pathFragment).asFragment(); } 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 894718366bbf3c..2fdcfd5db03de6 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 @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.AbstractSkyKey; @@ -23,8 +24,6 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.SkyframeLookupResult; -import java.util.Collections; -import java.util.LinkedHashMap; import java.util.Map; import javax.annotation.Nullable; @@ -45,7 +44,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept return env.getValue(ClientEnvironmentFunction.key(key)); } - /** @return the SkyKey to invoke this function for the environment variable {@code variable}. */ + /** Returns the SkyKey to invoke this function for the environment variable {@code variable}. */ public static Key key(String variable) { return Key.create(variable); } @@ -81,8 +80,8 @@ public SkyKeyInterner getSkyKeyInterner() { * if and only if some dependencies from Skyframe still need to be resolved. */ @Nullable - public static Map getEnvironmentView(Environment env, Iterable keys) - throws InterruptedException { + public static ImmutableMap getEnvironmentView( + Environment env, Iterable keys) throws InterruptedException { ImmutableList.Builder skyframeKeysBuilder = ImmutableList.builder(); for (String key : keys) { skyframeKeysBuilder.add(key(key)); @@ -92,8 +91,8 @@ public static Map getEnvironmentView(Environment env, Iterable result = new LinkedHashMap<>(); + + ImmutableMap.Builder result = ImmutableMap.builder(); for (SkyKey key : skyframeKeys) { ClientEnvironmentValue value = (ClientEnvironmentValue) values.get(key); if (value == null) { @@ -102,8 +101,10 @@ public static Map getEnvironmentView(Environment env, Iterable { + public static class Key extends AbstractSkyKey { private static final SkyKeyInterner interner = SkyKey.newInterner(); private Key(String arg) { @@ -64,7 +65,7 @@ public ClientEnvironmentFunction(AtomicReference> clientEnv) @Nullable @Override - public SkyValue compute(SkyKey key, Environment env) throws InterruptedException { + public SkyValue compute(SkyKey key, Environment env) { return new ClientEnvironmentValue(clientEnv.get().get((String) key.argument())); } } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java index 8b443aea695e9e..9463aab6d63e70 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java @@ -160,13 +160,26 @@ StarlarkCallable repositoryRule( "A description of the module extension that can be extracted by documentation" + " generating tools.", named = true, - positional = false) + positional = false), + @Param( + name = "environ", + allowedTypes = { + @ParamType(type = Sequence.class, generic1 = String.class), + }, + defaultValue = "[]", + doc = + "Provides a list of environment variable that this module extension depends on. If " + + "an environment variable in that list changes, the extension will be " + + "re-evaluated.", + named = true, + positional = false), }, useStarlarkThread = true) Object moduleExtension( StarlarkCallable implementation, Dict tagClasses, // Dict - Object doc, + Object doc, // or Starlark.NONE + Sequence environ, // StarlarkThread thread) throws EvalException; diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java index 274340755d35c3..668d9adfb14c67 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java @@ -78,7 +78,6 @@ public StarlarkCallable repositoryRule( Object doc, StarlarkThread thread) throws EvalException { - List attrInfos; ImmutableMap.Builder attrsMapBuilder = ImmutableMap.builder(); if (attrs != null && attrs != Starlark.NONE) { attrsMapBuilder.putAll(Dict.cast(attrs, String.class, FakeDescriptor.class, "attrs")); @@ -86,7 +85,7 @@ public StarlarkCallable repositoryRule( attrsMapBuilder.put("name", IMPLICIT_NAME_ATTRIBUTE_DESCRIPTOR); attrsMapBuilder.put("repo_mapping", IMPLICIT_REPO_MAPPING_ATTRIBUTE_DESCRIPTOR); - attrInfos = + List attrInfos = attrsMapBuilder.build().entrySet().stream() .filter(entry -> !entry.getKey().startsWith("_")) .map(entry -> entry.getValue().asAttributeInfo(entry.getKey())) @@ -124,7 +123,11 @@ public String getName() { @Override public Object moduleExtension( - StarlarkCallable implementation, Dict tagClasses, Object doc, StarlarkThread thread) + StarlarkCallable implementation, + Dict tagClasses, + Object doc, + Sequence environ, + StarlarkThread thread) throws EvalException { return new Object(); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java index 8784d8a21acc87..e6fc3b3d3862d8 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableBiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; @@ -61,7 +62,8 @@ private static ModuleExtension.Builder getBaseExtensionBuilder() { .setDoc(Optional.empty()) .setDefiningBzlFileLabel(Label.parseCanonicalUnchecked("//:rje.bzl")) .setLocation(Location.BUILTIN) - .setImplementation(() -> "maven"); + .setImplementation(() -> "maven") + .setEnvVariables(ImmutableList.of()); } @Test diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 4f99b84bb1df0d..ecbfe8c439b54f 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -276,23 +276,27 @@ def testModuleExtension(self): ' for dep in mod.tags.dep:', ' print("Name:", dep.name, ", Versions:", dep.versions)', '', - ( - '_dep = tag_class(attrs={"name": attr.string(), "versions":' - ' attr.string_list()})' - ), + '_dep = tag_class(attrs={"name": attr.string(), "versions":', + ' attr.string_list()})', 'lockfile_ext = module_extension(', ' implementation=_module_ext_impl,', ' tag_classes={"dep": _dep},', + ' environ=["GREEN_TREES", "NOT_SET"],', ')', ], ) - _, _, stderr = self.RunBazel(['build', '@hello//:all']) + # Only set one env var, to make sure null variables don't crash + _, _, stderr = self.RunBazel( + ['build', '@hello//:all'], env_add={'GREEN_TREES': 'In the city'} + ) self.assertIn('Hello from the other side!', ''.join(stderr)) self.assertIn('Name: bmbm , Versions: ["v1", "v2"]', ''.join(stderr)) self.RunBazel(['shutdown']) - _, _, stderr = self.RunBazel(['build', '@hello//:all']) + _, _, stderr = self.RunBazel( + ['build', '@hello//:all'], env_add={'GREEN_TREES': 'In the city'} + ) self.assertNotIn('Hello from the other side!', ''.join(stderr)) def testModuleExtensionsInDifferentBuilds(self): @@ -351,13 +355,10 @@ def testUpdateModuleExtension(self): ' ctx.file("WORKSPACE")', ' ctx.file("BUILD", "filegroup(name=\\"lala\\")")', 'repo_rule = repository_rule(implementation = _repo_rule_impl)', - 'def _module_ext_impl(ctx):', + 'def _mod_ext_impl(ctx):', ' print("Hello from the other side!")', ' repo_rule(name= "hello")', - ( - 'lockfile_ext = module_extension(implementation =' - ' _module_ext_impl)' - ), + 'lockfile_ext = module_extension(implementation = _mod_ext_impl)', ], ) _, _, stderr = self.RunBazel(['build', '@hello//:all']) @@ -382,13 +383,10 @@ def testUpdateModuleExtension(self): ' ctx.file("WORKSPACE")', ' ctx.file("BUILD", "filegroup(name=\\"lala\\")")', 'repo_rule = repository_rule(implementation = _repo_rule_impl)', - 'def _module_ext_impl(ctx):', + 'def _mod_ext_impl(ctx):', ' print("Hello from the other town!")', ' repo_rule(name= "hello")', - ( - 'lockfile_ext = module_extension(implementation =' - ' _module_ext_impl)' - ), + 'lockfile_ext = module_extension(implementation = _mod_ext_impl)', ], ) _, _, stderr = self.RunBazel(['build', '@hello//:all']) @@ -416,13 +414,10 @@ def testUpdateModuleExtensionErrorMode(self): ' ctx.file("WORKSPACE")', ' ctx.file("BUILD", "filegroup(name=\\"lala\\")")', 'repo_rule = repository_rule(implementation = _repo_rule_impl)', - 'def _module_ext_impl(ctx):', + 'def _mod_ext_impl(ctx):', ' print("Hello from the other side!")', ' repo_rule(name= "hello")', - ( - 'lockfile_ext = module_extension(implementation =' - ' _module_ext_impl)' - ), + 'lockfile_ext = module_extension(implementation = _mod_ext_impl)', ], ) _, _, stderr = self.RunBazel(['build', '@hello//:all']) @@ -437,13 +432,10 @@ def testUpdateModuleExtensionErrorMode(self): ' ctx.file("WORKSPACE")', ' ctx.file("BUILD", "filegroup(name=\\"lalo\\")")', 'repo_rule = repository_rule(implementation = _repo_rule_impl)', - 'def _module_ext_impl(ctx):', + 'def _mod_ext_impl(ctx):', ' print("Hello from the other town!")', ' repo_rule(name= "hello")', - ( - 'lockfile_ext = module_extension(implementation =' - ' _module_ext_impl)' - ), + 'lockfile_ext = module_extension(implementation = _mod_ext_impl)', ], ) @@ -500,6 +492,91 @@ def testRemoveModuleExtensionsNotUsed(self): lockfile = json.loads(f.read().strip()) self.assertEqual(len(lockfile['moduleExtensions']), 0) + def testModuleExtensionEnvVariable(self): + self.ScratchFile( + 'MODULE.bazel', + [ + 'lockfile_ext = use_extension("extension.bzl", "lockfile_ext")', + 'use_repo(lockfile_ext, "hello")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("WORKSPACE")', + ' ctx.file("BUILD", "filegroup(name=\\"lala\\")")', + 'repo_rule = repository_rule(implementation = _repo_rule_impl)', + 'def _module_ext_impl(ctx):', + ' print("Hello from the other side!")', + ' repo_rule(name= "hello")', + 'lockfile_ext = module_extension(', + ' implementation = _module_ext_impl,', + ' environ = ["SET_ME"]', + ')', + ], + ) + _, _, stderr = self.RunBazel( + ['build', '@hello//:all'], env_add={'SET_ME': 'High in sky'} + ) + self.assertIn('Hello from the other side!', ''.join(stderr)) + # Run with same value, no evaluated + _, _, stderr = self.RunBazel( + ['build', '@hello//:all'], env_add={'SET_ME': 'High in sky'} + ) + self.assertNotIn('Hello from the other side!', ''.join(stderr)) + # Run with different value, will be re-evaluated + _, _, stderr = self.RunBazel( + ['build', '@hello//:all'], env_add={'SET_ME': 'Down to earth'} + ) + self.assertIn('Hello from the other side!', ''.join(stderr)) + + def testChangeEnvVariableInErrorMode(self): + # If environ is set up in module extension, it should be re-evaluated if its + # value changed + self.ScratchFile( + 'MODULE.bazel', + [ + 'lockfile_ext = use_extension("extension.bzl", "lockfile_ext")', + 'use_repo(lockfile_ext, "hello")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("WORKSPACE")', + ' ctx.file("BUILD", "filegroup(name=\\"lala\\")")', + 'repo_rule = repository_rule(implementation = _repo_rule_impl)', + 'def _module_ext_impl(ctx):', + ' print("Hello from the other side!")', + ' repo_rule(name= "hello")', + 'lockfile_ext = module_extension(', + ' implementation = _module_ext_impl,', + ' environ = ["SET_ME"]', + ')', + ], + ) + self.RunBazel(['build', '@hello//:all'], env_add={'SET_ME': 'High in sky'}) + exit_code, _, stderr = self.RunBazel( + ['build', '--lockfile_mode=error', '@hello//:all'], + env_add={'SET_ME': 'Down to earth'}, + allow_failure=True, + ) + self.AssertExitCode(exit_code, 48, stderr) + self.assertIn( + ( + 'ERROR: Lock file is no longer up-to-date because: The environment' + ' variables the extension' + " 'ModuleExtensionId{bzlFileLabel=//:extension.bzl," + " extensionName=lockfile_ext, isolationKey=Optional.empty}' depends" + ' on (or their values) have changed' + ), + stderr, + ) + if __name__ == '__main__': unittest.main()