Create a new ExecGroupCollection container to manage exec group inheritance and exec property parsing.
Fixes #13459.
PiperOrigin-RevId: 373388266
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 7942efc..ea2c124 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -29,7 +29,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -45,6 +44,7 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.analysis.AliasProvider.TargetMode;
+import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -98,12 +98,7 @@
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.LabelClass;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
-import com.google.devtools.build.lib.server.FailureDetails.Analysis;
-import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
-import com.google.devtools.build.lib.skyframe.SaneAnalysisException;
-import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -217,10 +212,10 @@
private final FragmentClassSet universalFragments;
private final RuleErrorConsumer reporter;
@Nullable private final ToolchainCollection<ResolvedToolchainContext> toolchainContexts;
+ private final ExecGroupCollection execGroupCollection;
private final ConstraintSemantics<RuleContext> constraintSemantics;
private final ImmutableSet<String> requiredConfigFragments;
private final List<Expander> makeVariableExpanders = new ArrayList<>();
- private final ImmutableTable<String, String, String> execProperties;
/** Map of exec group names to ActionOwners. */
private final Map<String, ActionOwner> actionOwners = new HashMap<>();
@@ -264,6 +259,7 @@
ActionLookupKey actionLookupKey,
ImmutableMap<String, Attribute> aspectAttributes,
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
+ ExecGroupCollection execGroupCollection,
ConstraintSemantics<RuleContext> constraintSemantics,
ImmutableSet<String> requiredConfigFragments,
String toolsRepository,
@@ -300,7 +296,7 @@
this.actionOwnerSymbolGenerator = new SymbolGenerator<>(actionLookupKey);
reporter = builder.reporter;
this.toolchainContexts = toolchainContexts;
- this.execProperties = parseExecProperties(builder.rawExecProperties);
+ this.execGroupCollection = execGroupCollection;
this.constraintSemantics = constraintSemantics;
this.requiredConfigFragments = requiredConfigFragments;
this.starlarkSemantics = starlarkSemantics;
@@ -517,9 +513,8 @@
rule,
aspectDescriptors,
getConfiguration(),
- getExecProperties(execGroup, execProperties),
- getExecutionPlatform(execGroup),
- ImmutableSet.of(execGroup));
+ getExecGroups().getExecProperties(execGroup),
+ getExecutionPlatform(execGroup));
actionOwners.put(execGroup, actionOwner);
return actionOwner;
}
@@ -626,47 +621,13 @@
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. Properties for specific exec groups take precedence
- * over properties that don't specify an exec group.
- */
- private static ImmutableMap<String, String> computeExecProperties(
- Map<String, String> targetExecProperties,
- @Nullable PlatformInfo executionPlatform,
- Set<String> execGroups) {
- Map<String, String> execProperties = new HashMap<>();
-
- if (executionPlatform != null) {
- ImmutableTable<String, String, String> execPropertiesPerGroup =
- parseExecGroups(executionPlatform.execProperties());
-
- if (execPropertiesPerGroup.containsRow(DEFAULT_EXEC_GROUP_NAME)) {
- execProperties.putAll(execPropertiesPerGroup.row(DEFAULT_EXEC_GROUP_NAME));
- }
-
- for (String execGroup : execPropertiesPerGroup.rowKeySet()) {
- if (execGroups.contains(execGroup)) {
- execProperties.putAll(execPropertiesPerGroup.row(execGroup));
- }
- }
- }
-
- // If the same key occurs both in the platform and in target-specific properties, the
- // value is taken from target-specific properties (effectively overriding the platform
- // properties).
- execProperties.putAll(targetExecProperties);
- return ImmutableMap.copyOf(execProperties);
- }
-
@VisibleForTesting
public static ActionOwner createActionOwner(
Rule rule,
ImmutableList<AspectDescriptor> aspectDescriptors,
BuildConfiguration configuration,
- Map<String, String> targetExecProperties,
- @Nullable PlatformInfo executionPlatform,
- Set<String> execGroups) {
+ ImmutableMap<String, String> execProperties,
+ @Nullable PlatformInfo executionPlatform) {
return ActionOwner.create(
rule.getLabel(),
aspectDescriptors,
@@ -676,7 +637,7 @@
configuration.checksum(),
configuration.toBuildEvent(),
configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null,
- computeExecProperties(targetExecProperties, executionPlatform, execGroups),
+ execProperties,
executionPlatform);
}
@@ -1374,6 +1335,10 @@
return toolchainContexts;
}
+ public ExecGroupCollection getExecGroups() {
+ return execGroupCollection;
+ }
+
public boolean targetPlatformHasConstraint(ConstraintValueInfo constraintValue) {
if (toolchainContexts == null || toolchainContexts.getTargetPlatform() == null) {
return false;
@@ -1408,101 +1373,6 @@
return ans.build();
}
- private ImmutableTable<String, String, String> parseExecProperties(
- Map<String, String> execProperties) throws InvalidExecGroupException {
- if (execProperties.isEmpty()) {
- return ImmutableTable.of();
- } else {
- return parseExecProperties(
- execProperties, toolchainContexts == null ? null : toolchainContexts.getExecGroupNames());
- }
- }
-
- /**
- * 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 default exec group, the latter get parsed into their relevant exec
- * groups.
- */
- private static ImmutableTable<String, String, String> parseExecGroups(
- Map<String, String> rawExecProperties) {
- ImmutableTable.Builder<String, String, String> execProperties = ImmutableTable.builder();
- for (Map.Entry<String, String> execProperty : rawExecProperties.entrySet()) {
- String rawProperty = execProperty.getKey();
- int delimiterIndex = rawProperty.indexOf('.');
- if (delimiterIndex == -1) {
- execProperties.put(DEFAULT_EXEC_GROUP_NAME, rawProperty, execProperty.getValue());
- } else {
- String execGroup = rawProperty.substring(0, delimiterIndex);
- String property = rawProperty.substring(delimiterIndex + 1);
- execProperties.put(execGroup, property, execProperty.getValue());
- }
- }
- return execProperties.build();
- }
-
- /**
- * 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 ImmutableTable<String, String, String> parseExecProperties(
- Map<String, String> rawExecProperties, @Nullable Set<String> execGroups)
- throws InvalidExecGroupException {
- ImmutableTable<String, String, String> consolidatedProperties =
- parseExecGroups(rawExecProperties);
- if (execGroups != null) {
- for (String execGroupName : consolidatedProperties.rowKeySet()) {
- if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) {
- throw new InvalidExecGroupException(
- String.format(
- "Tried to set properties for non-existent exec group '%s'.", execGroupName));
- }
- }
- }
-
- return consolidatedProperties;
- }
-
- /**
- * Gets the combined exec properties of the given exec group and the target's exec properties. If
- * a property is set in both, the exec group properties take precedence. If a non-existent exec
- * group is passed in, just returns the target's exec properties.
- *
- * @param execGroup group whose properties to retrieve
- * @param execProperties Map of exec group name to map of properties and values
- */
- private static ImmutableMap<String, String> getExecProperties(
- String execGroup, ImmutableTable<String, String, String> execProperties) {
- if (!execProperties.containsRow(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) {
- return execProperties.row(DEFAULT_EXEC_GROUP_NAME);
- }
-
- // Use a HashMap to build here because we expect duplicate keys to happen
- // (and rewrite previous entries).
- Map<String, String> targetAndGroupProperties =
- new HashMap<>(execProperties.row(DEFAULT_EXEC_GROUP_NAME));
- targetAndGroupProperties.putAll(execProperties.row(execGroup));
- return ImmutableMap.copyOf(targetAndGroupProperties);
- }
-
- /** An error for when the user tries to access an non-existent exec group */
- public static final class InvalidExecGroupException extends Exception
- implements SaneAnalysisException {
- InvalidExecGroupException(String message) {
- super(message);
- }
-
- @Override
- public DetailedExitCode getDetailedExitCode() {
- return DetailedExitCode.of(
- FailureDetail.newBuilder()
- .setMessage(getMessage())
- .setAnalysis(Analysis.newBuilder().setCode(Code.EXEC_GROUP_MISSING))
- .build());
- }
- }
-
private void checkAttributeIsDependency(String attributeName) {
Attribute attributeDefinition = attributes.getAttributeDefinition(attributeName);
if (attributeDefinition == null) {
@@ -1876,6 +1746,7 @@
private ImmutableMap<String, Attribute> aspectAttributes;
private ImmutableList<Aspect> aspects;
private ToolchainCollection<ResolvedToolchainContext> toolchainContexts;
+ private ExecGroupCollection.Builder execGroupCollectionBuilder;
private ImmutableMap<String, String> rawExecProperties;
private ConstraintSemantics<RuleContext> constraintSemantics;
private ImmutableSet<String> requiredConfigFragments = ImmutableSet.of();
@@ -1933,14 +1804,7 @@
}
ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
createFilesetEntryMap(target.getAssociatedRule(), configConditions.asProviders());
- if (rawExecProperties == null) {
- if (!attributes.has(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT)) {
- rawExecProperties = ImmutableMap.of();
- } else {
- rawExecProperties =
- ImmutableMap.copyOf(attributes.get(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT));
- }
- }
+
return new RuleContext(
this,
attributes,
@@ -1952,6 +1816,7 @@
actionOwnerSymbol,
firstNonNull(aspectAttributes, ImmutableMap.of()),
toolchainContexts,
+ createExecGroupCollection(execGroupCollectionBuilder, attributes),
constraintSemantics,
requiredConfigFragments,
toolsRepository,
@@ -1959,6 +1824,21 @@
mutability);
}
+ private ExecGroupCollection createExecGroupCollection(
+ ExecGroupCollection.Builder execGroupCollectionBuilder, AttributeMap attributes)
+ throws InvalidExecGroupException {
+ if (rawExecProperties == null) {
+ if (!attributes.has(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT)) {
+ rawExecProperties = ImmutableMap.of();
+ } else {
+ rawExecProperties =
+ ImmutableMap.copyOf(attributes.get(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT));
+ }
+ }
+
+ return execGroupCollectionBuilder.build(toolchainContexts, rawExecProperties);
+ }
+
private void checkAttributesNonEmpty(AttributeMap attributes) {
for (String attributeName : attributes.getAttributeNames()) {
Attribute attr = attributes.getAttributeDefinition(attributeName);
@@ -2062,6 +1942,12 @@
return this;
}
+ public Builder setExecGroupCollectionBuilder(
+ ExecGroupCollection.Builder execGroupCollectionBuilder) {
+ this.execGroupCollectionBuilder = execGroupCollectionBuilder;
+ return this;
+ }
+
/**
* Warning: if you set the exec properties using this method any exec_properties attribute value
* will be ignored in favor of this value.
