Sort execution requirements for Starlark rules.

Ensures that action keys are deterministic regardless of hash iteration order.

PiperOrigin-RevId: 409157205

src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java

@@ -21,6 +21,7 @@
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -277,29 +278,27 @@ public static ImmutableMap<String, String> getExecutionInfo(
    *     actions' execution requirements, for more details {@see
    *     StarlarkSematicOptions#experimentalAllowTagsPropagation}
    */
-  public static ImmutableMap<String, String> getFilteredExecutionInfo(
+  public static ImmutableSortedMap<String, String> getFilteredExecutionInfo(
       @Nullable Object executionRequirementsUnchecked, Rule rule, boolean allowTagsPropagation)
       throws EvalException {
-    Map<String, String> checkedExecutionRequirements =
-        TargetUtils.filter(
-            executionRequirementsUnchecked == null
-                ? ImmutableMap.of()
-                : Dict.noneableCast(
+    Map<String, String> executionInfo =
+        executionRequirementsUnchecked == null
+            ? ImmutableMap.of()
+            : TargetUtils.filter(
+                Dict.noneableCast(
                     executionRequirementsUnchecked,
                     String.class,
                     String.class,
                     "execution_requirements"));
 
-    // adding filtered execution requirements to the execution info map
-    Map<String, String> executionInfoBuilder = new HashMap<>(checkedExecutionRequirements);
-
     if (allowTagsPropagation) {
+      executionInfo = new HashMap<>(executionInfo); // Make mutable.
       Map<String, String> checkedTags = getExecutionInfo(rule);
       // merging filtered tags to the execution info map avoiding duplicates
-      checkedTags.forEach(executionInfoBuilder::putIfAbsent);
+      checkedTags.forEach(executionInfo::putIfAbsent);
     }
 
-    return ImmutableMap.copyOf(executionInfoBuilder);
+    return ImmutableSortedMap.copyOf(executionInfo);
   }
 
   /**

src/test/shell/integration/aquery_test.sh

@@ -1489,6 +1489,36 @@ EOF
   assert_contains "\"value\": \"123456\"" output
 }
 
+# Regression test for b/205753626.
+function test_starlark_action_with_reqs_has_deterministic_action_key() {
+  local -r pkg="${FUNCNAME[0]}"
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+
+  cat >$pkg/BUILD <<'EOF'
+load(":defs.bzl", "lots_of_reqs")
+lots_of_reqs(name = "reqs")
+EOF
+  cat >$pkg/defs.bzl <<'EOF'
+def _lots_of_reqs_impl(ctx):
+    f = ctx.actions.declare_file(ctx.attr.name + ".txt")
+    ctx.actions.run_shell(
+      outputs = [f],
+      command = "touch " + f.path,
+      execution_requirements = {"requires-" + str(i): "1" for i in range(100)},
+    )
+    return DefaultInfo(files = depset([f]))
+
+lots_of_reqs = rule(implementation = _lots_of_reqs_impl)
+EOF
+
+  bazel aquery $pkg:reqs > output1 2> "$TEST_log" || fail "Expected success"
+  bazel shutdown || fail "Couldn't shutdown"
+  bazel aquery $pkg:reqs > output2 2> "$TEST_log" || fail "Expected success"
+
+  diff <(grep ActionKey output1) <(grep ActionKey output2) \
+    || fail "Nondeterministic action key"
+}
+
 # Usage: assert_matches expected_pattern actual
 function assert_matches() {
   [[ "$2" =~ $1 ]] || fail "Expected to match '$1', was: $2"