[6.4.0] Fix crash when environ contains duplicate entries

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<String, String> extensionEnvVars =
-        RepositoryFunction.getEnvVarValues(env, extension.getEnvVariables());
+        RepositoryFunction.getEnvVarValues(env, ImmutableSet.copyOf(extension.getEnvVariables()));
     if (extensionEnvVars == null) {
       return null;
     }

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<String> getEnviron(Rule rule) {
-    return (Iterable<String>) rule.getAttr("$environ");
+  private static ImmutableSet<String> getEnviron(Rule rule) {
+    return ImmutableSet.copyOf((Iterable<String>) rule.getAttr("$environ"));
   }
 
   @Override

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<String> PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR);
+  private static final ImmutableSet<String> 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<String, String> 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<String, String> environ =
-        declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_LIST);
+        declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET);
     if (environ == null) {
       return null;
     }

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<String> PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR);
+  private static final ImmutableSet<String> PATH_ENV_VAR_AS_SET = ImmutableSet.of(PATH_ENV_VAR);
   private static final ImmutableList<String> LOCAL_MAVEN_REPOSITORIES =
       ImmutableList.of(
           "extras/android/m2repository", "extras/google/m2repository", "extras/m2repository");
@@ -180,7 +181,7 @@ public boolean verifyMarkerData(Rule rule, Map<String, String> 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<String, String> environ =
-        declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_LIST);
+        declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET);
     if (environ == null) {
       return null;
     }

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<String> getEnviron(Rule rule) {
+  private static ImmutableSet<String> getEnviron(Rule rule) {
     if (rule.isAttrDefined("$environ", Type.STRING_LIST)) {
-      return (Collection<String>) rule.getAttr("$environ");
+      return ImmutableSet.copyOf((Collection<String>) rule.getAttr("$environ"));
     }
-    return ImmutableList.of();
+    return ImmutableSet.of();
   }
 
   /**
@@ -288,7 +289,7 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env)
    */
   @Nullable
   protected Map<String, String> declareEnvironmentDependencies(
-      Map<String, String> markerData, Environment env, Iterable<String> keys)
+      Map<String, String> markerData, Environment env, Set<String> keys)
       throws InterruptedException {
     ImmutableMap<String, String> envDep = getEnvVarValues(env, keys);
     if (envDep == null) {
@@ -300,7 +301,7 @@ protected Map<String, String> declareEnvironmentDependencies(
   }
 
   @Nullable
-  public static ImmutableMap<String, String> getEnvVarValues(Environment env, Iterable<String> keys)
+  public static ImmutableMap<String, String> getEnvVarValues(Environment env, Set<String> keys)
       throws InterruptedException {
     ImmutableMap<String, String> environ = ActionEnvironmentFunction.getEnvironmentView(env, keys);
     if (environ == null) {
@@ -329,7 +330,7 @@ public static ImmutableMap<String, String> getEnvVarValues(Environment env, Iter
    * Environment)} function to verify the values for environment variables.
    */
   protected boolean verifyEnvironMarkerData(
-      Map<String, String> markerData, Environment env, Collection<String> keys)
+      Map<String, String> markerData, Environment env, Set<String> keys)
       throws InterruptedException {
     ImmutableMap<String, String> environ = ActionEnvironmentFunction.getEnvironmentView(env, keys);
     if (env.valuesMissing()) {

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<String, String> getEnvironmentView(
-      Environment env, Iterable<String> keys) throws InterruptedException {
-    ImmutableList.Builder<SkyKey> skyframeKeysBuilder = ImmutableList.builder();
-    for (String key : keys) {
-      skyframeKeysBuilder.add(key(key));
-    }
-    ImmutableList<SkyKey> skyframeKeys = skyframeKeysBuilder.build();
+  public static ImmutableMap<String, String> getEnvironmentView(Environment env, Set<String> keys)
+      throws InterruptedException {
+    var skyframeKeys = keys.stream().map(ActionEnvironmentFunction::key).collect(toImmutableSet());
     SkyframeLookupResult values = env.getValuesAndExceptions(skyframeKeys);
     if (env.valuesMissing()) {
       return null;

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 <<EOF
+load('//:def.bzl', 'repo')
+repo(name='foo')
+EOF
+
+  touch BUILD
+  cat > 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"