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 748735a88288e2..4d00ea10c18a3e 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 @@ -159,7 +159,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) ModuleExtension extension = ((ModuleExtension) exported); ImmutableMap extensionEnvVars = - RepositoryFunction.getEnvVarValues(env, extension.getEnvVariables()); + RepositoryFunction.getEnvVarValues(env, ImmutableSet.copyOf(extension.getEnvVariables())); if (extensionEnvVars == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java index 140697445739bb..fc2c43d1a1b276 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java @@ -285,8 +285,8 @@ public RepositoryDirectoryValue.Builder fetch( } @SuppressWarnings("unchecked") - private static Iterable getEnviron(Rule rule) { - return (Iterable) rule.getAttr("$environ"); + private static ImmutableSet getEnviron(Rule rule) { + return ImmutableSet.copyOf((Iterable) rule.getAttr("$environ")); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java index d6bfd2d4d0f420..934fa82e1b4ccd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java @@ -18,6 +18,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -71,7 +72,7 @@ public class AndroidNdkRepositoryFunction extends AndroidRepositoryFunction { private static final String PATH_ENV_VAR = "ANDROID_NDK_HOME"; private static final PathFragment PLATFORMS_DIR = PathFragment.create("platforms"); - private static final ImmutableList PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR); + private static final ImmutableSet PATH_ENV_VAR_AS_SET = ImmutableSet.of(PATH_ENV_VAR); private static String getDefaultCrosstool(Integer majorRevision) { // From NDK 17, libc++ replaces gnu-libstdc++ as the default STL. @@ -262,7 +263,7 @@ public boolean verifyMarkerData(Rule rule, Map markerData, Envir if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_LIST); + return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_SET); } @Override @@ -276,7 +277,7 @@ public RepositoryDirectoryValue.Builder fetch( SkyKey key) throws InterruptedException, RepositoryFunctionException { Map environ = - declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_LIST); + declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET); if (environ == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java index 5875814a248c8e..e52dbb6303f011 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java @@ -19,6 +19,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -163,7 +164,7 @@ public String toString() { private static final PathFragment SYSTEM_IMAGES_DIR = PathFragment.create("system-images"); private static final AndroidRevision MIN_BUILD_TOOLS_REVISION = AndroidRevision.parse("30.0.0"); private static final String PATH_ENV_VAR = "ANDROID_HOME"; - private static final ImmutableList PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR); + private static final ImmutableSet PATH_ENV_VAR_AS_SET = ImmutableSet.of(PATH_ENV_VAR); private static final ImmutableList LOCAL_MAVEN_REPOSITORIES = ImmutableList.of( "extras/android/m2repository", "extras/google/m2repository", "extras/m2repository"); @@ -180,7 +181,7 @@ public boolean verifyMarkerData(Rule rule, Map markerData, Envir if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_LIST); + return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_SET); } @Override @@ -194,7 +195,7 @@ public RepositoryDirectoryValue.Builder fetch( SkyKey key) throws RepositoryFunctionException, InterruptedException { Map environ = - declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_LIST); + declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET); if (environ == null) { return null; } 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 26d04ae28c2288..2ac13825d39ce8 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 @@ -19,8 +19,8 @@ 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.collect.ImmutableSet; import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -55,6 +55,7 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; +import java.util.Set; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Starlark; @@ -172,11 +173,11 @@ public abstract RepositoryDirectoryValue.Builder fetch( throws InterruptedException, RepositoryFunctionException; @SuppressWarnings("unchecked") - private static Collection getEnviron(Rule rule) { + private static ImmutableSet getEnviron(Rule rule) { if (rule.isAttrDefined("$environ", Type.STRING_LIST)) { - return (Collection) rule.getAttr("$environ"); + return ImmutableSet.copyOf((Collection) rule.getAttr("$environ")); } - return ImmutableList.of(); + return ImmutableSet.of(); } /** @@ -288,7 +289,7 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env) */ @Nullable protected Map declareEnvironmentDependencies( - Map markerData, Environment env, Iterable keys) + Map markerData, Environment env, Set keys) throws InterruptedException { ImmutableMap envDep = getEnvVarValues(env, keys); if (envDep == null) { @@ -300,7 +301,7 @@ protected Map declareEnvironmentDependencies( } @Nullable - public static ImmutableMap getEnvVarValues(Environment env, Iterable keys) + public static ImmutableMap getEnvVarValues(Environment env, Set keys) throws InterruptedException { ImmutableMap environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); if (environ == null) { @@ -329,7 +330,7 @@ public static ImmutableMap getEnvVarValues(Environment env, Iter * Environment)} function to verify the values for environment variables. */ protected boolean verifyEnvironMarkerData( - Map markerData, Environment env, Collection keys) + Map markerData, Environment env, Set keys) throws InterruptedException { ImmutableMap environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); if (env.valuesMissing()) { 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 4edf0fb1b31078..2a8cd3f8c80963 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 @@ -14,7 +14,8 @@ package com.google.devtools.build.lib.skyframe; -import com.google.common.collect.ImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + import com.google.common.collect.ImmutableMap; import com.google.common.collect.Interner; import com.google.devtools.build.lib.bugreport.BugReport; @@ -27,6 +28,7 @@ import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.SkyframeLookupResult; import java.util.Map; +import java.util.Set; import javax.annotation.Nullable; /** @@ -77,13 +79,9 @@ public SkyFunctionName functionName() { * if and only if some dependencies from Skyframe still need to be resolved. */ @Nullable - public static ImmutableMap getEnvironmentView( - Environment env, Iterable keys) throws InterruptedException { - ImmutableList.Builder skyframeKeysBuilder = ImmutableList.builder(); - for (String key : keys) { - skyframeKeysBuilder.add(key(key)); - } - ImmutableList skyframeKeys = skyframeKeysBuilder.build(); + public static ImmutableMap getEnvironmentView(Environment env, Set keys) + throws InterruptedException { + var skyframeKeys = keys.stream().map(ActionEnvironmentFunction::key).collect(toImmutableSet()); SkyframeLookupResult values = env.getValuesAndExceptions(skyframeKeys); if (env.valuesMissing()) { return null; diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 5569735b30d612..4ded3acfc5bb0d 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2356,4 +2356,26 @@ EOF bazel build --experimental_repository_disable_download //:it || fail "Failed to build" } +function test_duplicate_value_in_environ() { + cat >> WORKSPACE < def.bzl <<'EOF' +def _impl(repository_ctx): + repository_ctx.file("WORKSPACE") + repository_ctx.file("BUILD", """filegroup(name="bar",srcs=[])""") + +repo = repository_rule( + implementation=_impl, + environ=["FOO", "FOO"], +) +EOF + + FOO=bar bazel build @foo//:bar >& $TEST_log \ + || fail "Expected build to succeed" +} + run_suite "local repository tests"