Support setting properties under exec groups on platforms.

Previously, exec groups were rejected on platforms. This change allows setting
arbitrary exec groups on platforms, and applying only the relevant ones to
actions after platform resolution.

Also fixes a bug where the default execution platform for the "test" execution
group did not take into account execution constraints  set on targets.

Builds upon a change by @ulfjack:
ulfjack@342543f

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -475,14 +475,21 @@ public ActionOwner getActionOwner() {
    * rather than the host platform. Note that the value is not cached (on the assumption that this
    * method is only called once).
    */
-  public ActionOwner getTestActionOwner() {
+  public ActionOwner getTestActionOwner(boolean useTargetPlatform) {
+    PlatformInfo executionPlatform = null;
+    if (toolchainContexts != null) {
+      executionPlatform = useTargetPlatform
+          ? toolchainContexts.getTargetPlatform()
+          : getExecutionPlatform();
+    }
     ActionOwner actionOwner =
         createActionOwner(
             rule,
             aspectDescriptors,
             getConfiguration(),
             getExecProperties(TEST_RUNNER_EXEC_GROUP, execProperties),
-            toolchainContexts == null ? null : toolchainContexts.getTargetPlatform());
+            executionPlatform,
+            ImmutableSet.of(TEST_RUNNER_EXEC_GROUP));
     return actionOwner;
   }
 
@@ -501,7 +508,8 @@ public ActionOwner getActionOwner(String execGroup) {
             aspectDescriptors,
             getConfiguration(),
             getExecProperties(execGroup, execProperties),
-            getExecutionPlatform(execGroup));
+            getExecutionPlatform(execGroup),
+            ImmutableSet.of(execGroup));
     actionOwners.put(execGroup, actionOwner);
     return actionOwner;
   }
@@ -608,12 +616,24 @@ public ImmutableList<Artifact> getBuildInfo(BuildInfoKey key) throws Interrupted
             AnalysisUtils.isStampingEnabled(this, getConfiguration()), key, getConfiguration());
   }
 
+  /**
+   * Computes a map of exec properties given the execution platform, taking only properties in exec
+   * groups that are applicable to this action.
+   */
   private static ImmutableMap<String, String> computeExecProperties(
-      Map<String, String> targetExecProperties, @Nullable PlatformInfo executionPlatform) {
+      Map<String, String> targetExecProperties,
+      @Nullable PlatformInfo executionPlatform,
+      Set<String> execGroups) {
     Map<String, String> execProperties = new HashMap<>();
 
     if (executionPlatform != null) {
-      execProperties.putAll(executionPlatform.execProperties());
+      for (Map.Entry<String, Map<String, String>> execGroup :
+          parseExecGroups(executionPlatform.execProperties()).entrySet()) {
+        if (execGroup.getKey().equals(DEFAULT_EXEC_GROUP_NAME)
+                || execGroups.contains(execGroup.getKey())) {
+          execProperties.putAll(execGroup.getValue());
+        }
+      }
     }
 
     // If the same key occurs both in the platform and in target-specific properties, the
@@ -629,7 +649,8 @@ public static ActionOwner createActionOwner(
       ImmutableList<AspectDescriptor> aspectDescriptors,
       BuildConfiguration configuration,
       Map<String, String> targetExecProperties,
-      @Nullable PlatformInfo executionPlatform) {
+      @Nullable PlatformInfo executionPlatform,
+      Set<String> execGroups) {
     return ActionOwner.create(
         rule.getLabel(),
         aspectDescriptors,
@@ -639,7 +660,7 @@ public static ActionOwner createActionOwner(
         configuration.checksum(),
         configuration.toBuildEvent(),
         configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null,
-        computeExecProperties(targetExecProperties, executionPlatform),
+        computeExecProperties(targetExecProperties, executionPlatform, execGroups),
         executionPlatform);
   }
 
@@ -1277,19 +1298,18 @@ private ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
     } else {
       return parseExecProperties(
           execProperties,
-          toolchainContexts == null ? ImmutableSet.of() : toolchainContexts.getExecGroups());
+          toolchainContexts == null ? null : toolchainContexts.getExecGroups());
     }
   }
 
   /**
    * Parse raw exec properties attribute value into a map of exec group names to their properties.
    * The raw map can have keys of two forms: (1) 'property' and (2) 'exec_group_name.property'. The
-   * former get parsed into the target's default exec group, the latter get parsed into their
-   * relevant exec groups.
+   * former get parsed into the default exec group, the latter get parsed into their relevant exec
+   * groups.
    */
-  private static ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
-      Map<String, String> rawExecProperties, Set<String> execGroups)
-      throws InvalidExecGroupException {
+  private static Map<String, Map<String, String>> parseExecGroups(
+      Map<String, String> rawExecProperties) {
     Map<String, Map<String, String>> consolidatedProperties = new HashMap<>();
     consolidatedProperties.put(DEFAULT_EXEC_GROUP_NAME, new HashMap<>());
     for (Map.Entry<String, String> execProperty : rawExecProperties.entrySet()) {
@@ -1302,14 +1322,30 @@ private static ImmutableMap<String, ImmutableMap<String, String>> parseExecPrope
       } else {
         String execGroup = rawProperty.substring(0, delimiterIndex);
         String property = rawProperty.substring(delimiterIndex + 1);
-        if (!execGroups.contains(execGroup)) {
+        consolidatedProperties.putIfAbsent(execGroup, new HashMap<>());
+        consolidatedProperties.get(execGroup).put(property, execProperty.getValue());
+      }
+    }
+    return consolidatedProperties;
+  }
+
+  /**
+   * Parse raw exec properties attribute value into a map of exec group names to their properties.
+   * If given a set of exec groups, validates all the exec groups in the map are applicable to the
+   * action.
+   */
+  private static ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
+      Map<String, String> rawExecProperties, @Nullable Set<String> execGroups)
+      throws InvalidExecGroupException {
+    Map<String, Map<String, String>> consolidatedProperties = parseExecGroups(rawExecProperties);
+    if (execGroups != null) {
+      for (Map.Entry<String, Map<String, String>> execGroup : consolidatedProperties.entrySet()) {
+        String execGroupName = execGroup.getKey();
+        if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) {
           throw new InvalidExecGroupException(
               String.format(
-                  "Tried to set exec property '%s' for non-existent exec group '%s'.",
-                  property, execGroup));
+                  "Tried to set properties for non-existent exec group '%s'.", execGroupName));
         }
-        consolidatedProperties.putIfAbsent(execGroup, new HashMap<>());
-        consolidatedProperties.get(execGroup).put(property, execProperty.getValue());
       }
     }
 

src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java

@@ -331,12 +331,9 @@ private TestParams createTestAction(int shards) throws InterruptedException {
     ImmutableList.Builder<Artifact> coverageArtifacts = ImmutableList.builder();
     ImmutableList.Builder<ActionInput> testOutputs = ImmutableList.builder();
 
-    ActionOwner actionOwner = testConfiguration.useTargetPlatformForTests()
-        ? ruleContext.getTestActionOwner()
-        : ruleContext.getActionOwner(TEST_RUNNER_EXEC_GROUP);
-    if (actionOwner == null) {
-      actionOwner = ruleContext.getActionOwner();
-    }
+    ActionOwner actionOwner = ruleContext.getTestActionOwner(
+        testConfiguration.useTargetPlatformForTests());
+
     // Use 1-based indices for user friendliness.
     for (int shard = 0; shard < shardRuns; shard++) {
       String shardDir = shardRuns > 1 ? String.format("shard_%d_of_%d", shard + 1, shards) : null;