Introduce SplitTransitionProvider, to determine the split transition on an attribute based on the Rule itself (the transition may thus be determined based on the values of other attributes of the rule)
--
MOS_MIGRATED_REVID=120275649
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index a74094e..fe8bddc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -28,7 +28,6 @@
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
-import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.EnvironmentGroup;
@@ -356,13 +355,13 @@
}
List<BuildConfiguration> actualConfigurations = ImmutableList.of(configuration);
- if (attribute.getConfigurationTransition() instanceof SplitTransition<?>) {
+ if (attribute.hasSplitConfigurationTransition()) {
Preconditions.checkState(attribute.getConfigurator() == null);
// TODO(bazel-team): This ends up applying the split transition twice, both here and in the
// visitRule method below - this is not currently a problem, because the configuration graph
// never contains nested split transitions, so the second application is idempotent.
- actualConfigurations = configuration.getSplitConfigurations(
- (SplitTransition<?>) attribute.getConfigurationTransition());
+ actualConfigurations =
+ configuration.getSplitConfigurations(attribute.getSplitTransition(rule));
}
for (BuildConfiguration actualConfig : actualConfigurations) {
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 0b1248c..8d930a2 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
@@ -584,8 +584,7 @@
public List<? extends TransitiveInfoCollection> getPrerequisites(String attributeName,
Mode mode) {
Attribute attributeDefinition = getAttribute(attributeName);
- if ((mode == Mode.TARGET)
- && (attributeDefinition.getConfigurationTransition() instanceof SplitTransition)) {
+ if ((mode == Mode.TARGET) && (attributeDefinition.hasSplitConfigurationTransition())) {
// TODO(bazel-team): If you request a split-configured attribute in the target configuration,
// we return only the list of configured targets for the first architecture; this is for
// backwards compatibility with existing code in cases where the call to getPrerequisites is
@@ -617,8 +616,7 @@
checkAttribute(attributeName, Mode.SPLIT);
Attribute attributeDefinition = getAttribute(attributeName);
- SplitTransition<?> transition =
- (SplitTransition<?>) attributeDefinition.getConfigurationTransition();
+ SplitTransition<?> transition = attributeDefinition.getSplitTransition(rule);
List<BuildConfiguration> configurations =
getConfiguration().getTransitions().getSplitConfigurations(transition);
if (configurations.size() == 1) {
@@ -969,7 +967,7 @@
+ " is not configured for the data configuration");
}
} else if (mode == Mode.SPLIT) {
- if (!(attributeDefinition.getConfigurationTransition() instanceof SplitTransition)) {
+ if (!(attributeDefinition.hasSplitConfigurationTransition())) {
throw new IllegalStateException(getRule().getLocation() + ": "
+ getRuleClassNameForLogging() + " attribute " + attributeName
+ " is not configured for a split transition");
@@ -998,7 +996,7 @@
return Mode.TARGET;
} else if (attributeDefinition.getConfigurationTransition() == ConfigurationTransition.DATA) {
return Mode.DATA;
- } else if (attributeDefinition.getConfigurationTransition() instanceof SplitTransition) {
+ } else if (attributeDefinition.hasSplitConfigurationTransition()) {
return Mode.SPLIT;
}
throw new IllegalStateException(getRule().getLocation() + ": "
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 67e34ab..3f8c6be 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -1825,9 +1825,9 @@
return;
}
- if (attribute.getConfigurationTransition() instanceof SplitTransition) {
+ if (attribute.hasSplitConfigurationTransition()) {
Preconditions.checkState(attribute.getConfigurator() == null);
- transitionApplier.split((SplitTransition<?>) attribute.getConfigurationTransition());
+ transitionApplier.split(attribute.getSplitTransition(fromRule));
} else {
// III. Attributes determine configurations. The configuration of a prerequisite is determined
// by the attribute.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index cf4d210..aeac774 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -149,11 +149,27 @@
/** Transition from the target configuration to the data configuration. */
// TODO(bazel-team): Move this elsewhere.
- DATA;
+ DATA,
+
+ /**
+ * Transition to one or more configurations. To obtain the actual child configurations,
+ * invoke {@link Attribute#getSplitTransition(Rule)}. See {@link SplitTransition}.
+ **/
+ SPLIT(true);
+
+ private boolean defaultsToSelf;
+
+ ConfigurationTransition() {
+ this(false);
+ }
+
+ ConfigurationTransition(boolean defaultsToSelf) {
+ this.defaultsToSelf = defaultsToSelf;
+ }
@Override
public boolean defaultsToSelf() {
- return false;
+ return defaultsToSelf;
}
}
@@ -270,6 +286,37 @@
}
/**
+ * Provides a {@link SplitTransition} given the originating target {@link Rule}. The split
+ * transition may be constant for all instances of the originating rule, or it may differ
+ * based on attributes of that rule. For instance, a split transition on a rule's deps may differ
+ * depending on the 'platform' attribute of the rule.
+ */
+ public interface SplitTransitionProvider {
+ /**
+ * Returns the {@link SplitTransition} given the originating rule.
+ */
+ SplitTransition<?> apply(Rule fromRule);
+ }
+
+ /**
+ * Implementation of {@link SplitTransitionProvider} that returns a single {@link SplitTransition}
+ * regardless of the originating rule.
+ */
+ private static class BasicSplitTransitionProvider implements SplitTransitionProvider {
+
+ private final SplitTransition<?> splitTransition;
+
+ BasicSplitTransitionProvider(SplitTransition<?> splitTransition) {
+ this.splitTransition = splitTransition;
+ }
+
+ @Override
+ public SplitTransition<?> apply(Rule fromRule) {
+ return splitTransition;
+ }
+ }
+
+ /**
* A predicate class to check if the value of the attribute comes from a predefined set.
*/
public static class AllowedValueSet implements PredicateWithMessage<Object> {
@@ -340,6 +387,7 @@
private Predicate<RuleClass> allowedRuleClassesForLabels = Predicates.alwaysTrue();
private Predicate<RuleClass> allowedRuleClassesForLabelsWarning = Predicates.alwaysFalse();
private Configurator<?, ?> configurator;
+ private SplitTransitionProvider splitTransitionProvider;
private FileTypeSet allowedFileTypesForLabels;
private ValidityPredicate validityPredicate = ANY_EDGE;
private Object value;
@@ -444,14 +492,40 @@
}
/**
+ * Defines the configuration transition for this attribute.
+ */
+ public Builder<TYPE> cfg(SplitTransitionProvider splitTransitionProvider) {
+ Preconditions.checkState(this.configTransition == ConfigurationTransition.NONE,
+ "the configuration transition is already set");
+
+ this.splitTransitionProvider = Preconditions.checkNotNull(splitTransitionProvider);
+ this.configTransition = ConfigurationTransition.SPLIT;
+ return this;
+ }
+
+ /**
+ * Defines the configuration transition for this attribute. Defaults to
+ * {@code NONE}.
+ */
+ public Builder<TYPE> cfg(SplitTransition<?> configTransition) {
+ return cfg(new BasicSplitTransitionProvider(Preconditions.checkNotNull(configTransition)));
+ }
+
+ /**
* Defines the configuration transition for this attribute. Defaults to
* {@code NONE}.
*/
public Builder<TYPE> cfg(Transition configTransition) {
Preconditions.checkState(this.configTransition == ConfigurationTransition.NONE,
"the configuration transition is already set");
- this.configTransition = configTransition;
- return this;
+ Preconditions.checkArgument(configTransition != ConfigurationTransition.SPLIT,
+ "split transitions must be defined using the SplitTransition object");
+ if (configTransition instanceof SplitTransition) {
+ return cfg((SplitTransition<?>) configTransition);
+ } else {
+ this.configTransition = configTransition;
+ return this;
+ }
}
public Builder<TYPE> cfg(Configurator<?, ?> configurator) {
@@ -877,6 +951,7 @@
valueSet ? value : type.getDefaultValue(),
configTransition,
configurator,
+ splitTransitionProvider,
allowedRuleClassesForLabels,
allowedRuleClassesForLabelsWarning,
allowedFileTypesForLabels,
@@ -1135,6 +1210,7 @@
private final Transition configTransition;
private final Configurator<?, ?> configurator;
+ private final SplitTransitionProvider splitTransitionProvider;
/**
* For label or label-list attributes, this predicate returns which rule
@@ -1192,6 +1268,7 @@
Object defaultValue,
Transition configTransition,
Configurator<?, ?> configurator,
+ SplitTransitionProvider splitTransitionProvider,
Predicate<RuleClass> allowedRuleClassesForLabels,
Predicate<RuleClass> allowedRuleClassesForLabelsWarning,
FileTypeSet allowedFileTypesForLabels,
@@ -1225,6 +1302,7 @@
this.defaultValue = defaultValue;
this.configTransition = configTransition;
this.configurator = configurator;
+ this.splitTransitionProvider = splitTransitionProvider;
this.allowedRuleClassesForLabels = allowedRuleClassesForLabels;
this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning;
this.allowedFileTypesForLabels = allowedFileTypesForLabels;
@@ -1329,6 +1407,25 @@
}
/**
+ * Returns the split configuration transition for this attribute.
+ *
+ * @param rule the originating {@link Rule} which owns this attribute
+ * @throws IllegalStateException if {@link #hasSplitConfigurationTransition} is not true
+ */
+ public SplitTransition<?> getSplitTransition(Rule rule) {
+ Preconditions.checkState(hasSplitConfigurationTransition());
+ return splitTransitionProvider.apply(rule);
+ }
+
+ /**
+ * Returns true if this attribute transitions on a split transition.
+ * See {@link SplitTransition}.
+ */
+ public boolean hasSplitConfigurationTransition() {
+ return (splitTransitionProvider != null);
+ }
+
+ /**
* Returns whether the target is required to be executable for label or label
* list attributes. For other attributes it always returns {@code false}.
*/
@@ -1539,6 +1636,7 @@
builder.validityPredicate = validityPredicate;
builder.condition = condition;
builder.configTransition = configTransition;
+ builder.splitTransitionProvider = splitTransitionProvider;
builder.propertyFlags = propertyFlags.isEmpty() ?
EnumSet.noneOf(PropertyFlag.class) : EnumSet.copyOf(propertyFlags);
builder.value = defaultValue;
