Enable rules to transition based on their Rule objects.
This begins to allow for cases where a rule sets configuration
based on its attributes, such as where a rule attribute names
flags and their values - sort of a reverse select.
There are no such cases yet, but they're coming!
--
PiperOrigin-RevId: 150648357
MOS_MIGRATED_REVID=150648357
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index df50202..6043cda 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -60,6 +60,7 @@
import com.google.devtools.build.lib.packages.PackageSpecification;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.pkgcache.LoadingResult;
@@ -855,11 +856,12 @@
Attribute.Transition ruleclassTransition = null;
if (targetAndConfig.getTarget().getAssociatedRule() != null) {
- ruleclassTransition = targetAndConfig
- .getTarget()
- .getAssociatedRule()
- .getRuleClassObject()
- .getTransition();
+ Rule associatedRule = targetAndConfig.getTarget().getAssociatedRule();
+ RuleTransitionFactory transitionFactory =
+ associatedRule.getRuleClassObject().getTransitionFactory();
+ if (transitionFactory != null) {
+ ruleclassTransition = transitionFactory.buildTransitionFor(associatedRule);
+ }
}
if (targetAndConfig.getConfiguration() != null) {
asDeps.put(targetAndConfig.getConfiguration(),
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 e7df822..1fc05d8 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
@@ -56,6 +56,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.test.TestActionBuilder;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
@@ -1849,15 +1850,18 @@
transitionsManager.configurationHook(fromRule, attribute, toTarget, this);
Rule associatedRule = toTarget.getAssociatedRule();
- PatchTransition ruleClassTransition = (PatchTransition)
- associatedRule.getRuleClassObject().getTransition();
-
- if (ruleClassTransition != null) {
- if (currentTransition == ConfigurationTransition.NONE) {
- currentTransition = ruleClassTransition;
- } else {
- currentTransition = new ComposingSplitTransition(ruleClassTransition,
- currentTransition);
+ RuleTransitionFactory transitionFactory =
+ associatedRule.getRuleClassObject().getTransitionFactory();
+ if (transitionFactory != null) {
+ PatchTransition ruleClassTransition = (PatchTransition)
+ transitionFactory.buildTransitionFor(associatedRule);
+ if (ruleClassTransition != null) {
+ if (currentTransition == ConfigurationTransition.NONE) {
+ currentTransition = ruleClassTransition;
+ } else {
+ currentTransition = new ComposingSplitTransition(ruleClassTransition,
+ currentTransition);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index eeacba5..1a2dbb2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -445,6 +445,22 @@
}
}
+ /**
+ * A RuleTransitionFactory which always returns the same transition.
+ */
+ private static final class FixedTransitionFactory implements RuleTransitionFactory {
+ private final Transition transition;
+
+ private FixedTransitionFactory(Transition transition) {
+ this.transition = transition;
+ }
+
+ @Override
+ public Transition buildTransitionFor(Rule rule) {
+ return transition;
+ }
+ }
+
/** List of required attributes for normal rules, name and type. */
public static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_NORMAL_RULES =
ImmutableList.of(attr("tags", Type.STRING_LIST).build());
@@ -470,7 +486,7 @@
private boolean outputsDefaultExecutable = false;
private ImplicitOutputsFunction implicitOutputsFunction = ImplicitOutputsFunction.NONE;
private Configurator<?, ?> configurator = NO_CHANGE;
- private Transition transition;
+ private RuleTransitionFactory transitionFactory;
private ConfiguredTargetFactory<?, ?> configuredTargetFactory = null;
private PredicateWithMessage<Rule> validityPredicate =
PredicatesWithMessage.<Rule>alwaysTrue();
@@ -584,7 +600,7 @@
outputsDefaultExecutable,
implicitOutputsFunction,
configurator,
- transition,
+ transitionFactory,
configuredTargetFactory,
validityPredicate,
preferredDependencyPredicate,
@@ -718,8 +734,9 @@
public Builder cfg(Configurator<?, ?> configurator) {
Preconditions.checkState(type != RuleClassType.ABSTRACT,
"Setting not inherited property (cfg) of abstract rule class '%s'", name);
- Preconditions.checkState(transition == null,
- "Property cfg cannot be set to both a configurator and a transition");
+ Preconditions.checkState(this.transitionFactory == null && this.configurator == NO_CHANGE,
+ "Property cfg has already been set");
+ Preconditions.checkNotNull(configurator);
this.configurator = configurator;
return this;
}
@@ -734,9 +751,20 @@
public Builder cfg(Transition transition) {
Preconditions.checkState(type != RuleClassType.ABSTRACT,
"Setting not inherited property (cfg) of abstract rule class '%s'", name);
- Preconditions.checkState(configurator == NO_CHANGE,
- "Property cfg cannot be set to both a configurator and a transition");
- this.transition = transition;
+ Preconditions.checkState(this.transitionFactory == null && this.configurator == NO_CHANGE,
+ "Property cfg has already been set");
+ Preconditions.checkNotNull(transition);
+ this.transitionFactory = new FixedTransitionFactory(transition);
+ return this;
+ }
+
+ public Builder cfg(RuleTransitionFactory transitionFactory) {
+ Preconditions.checkState(type != RuleClassType.ABSTRACT,
+ "Setting not inherited property (cfg) of abstract rule class '%s'", name);
+ Preconditions.checkState(this.transitionFactory == null && this.configurator == NO_CHANGE,
+ "Property cfg has already been set");
+ Preconditions.checkNotNull(transitionFactory);
+ this.transitionFactory = transitionFactory;
return this;
}
@@ -1000,12 +1028,10 @@
private final Configurator<?, ?> configurator;
/**
- * A configuration transition that should be applied on any edge of the configured target graph
- * that leads into a target of this rule class.
- *
- * <p>This transition must be a PatchTransition, but that class is not accessible in this package.
+ * A factory which will produce a configuration transition that should be applied on any edge of
+ * the configured target graph that leads into a target of this rule class.
*/
- private final Transition transition;
+ private final RuleTransitionFactory transitionFactory;
/**
* The factory that creates configured targets from this rule.
@@ -1090,7 +1116,7 @@
boolean outputsDefaultExecutable,
ImplicitOutputsFunction implicitOutputsFunction,
Configurator<?, ?> configurator,
- Transition transition,
+ RuleTransitionFactory transitionFactory,
ConfiguredTargetFactory<?, ?> configuredTargetFactory,
PredicateWithMessage<Rule> validityPredicate,
Predicate<String> preferredDependencyPredicate,
@@ -1112,7 +1138,7 @@
this.binaryOutput = binaryOutput;
this.implicitOutputsFunction = implicitOutputsFunction;
this.configurator = Preconditions.checkNotNull(configurator);
- this.transition = transition;
+ this.transitionFactory = transitionFactory;
this.configuredTargetFactory = configuredTargetFactory;
this.validityPredicate = validityPredicate;
this.preferredDependencyPredicate = preferredDependencyPredicate;
@@ -1182,8 +1208,8 @@
return (Configurator<C, R>) configurator;
}
- public Transition getTransition() {
- return transition;
+ public RuleTransitionFactory getTransitionFactory() {
+ return transitionFactory;
}
@SuppressWarnings("unchecked")
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleTransitionFactory.java
new file mode 100644
index 0000000..215461f
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleTransitionFactory.java
@@ -0,0 +1,32 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.packages;
+
+import com.google.devtools.build.lib.packages.Attribute.Transition;
+import javax.annotation.Nullable;
+
+/**
+ * Customizable transition which accepts the rule's nonconfigurable attributes.
+ */
+public interface RuleTransitionFactory {
+ /**
+ * Generates a transition to be used when entering the given rule.
+ *
+ * <p>This transition must be a PatchTransition, but that class is not accessible in this package.
+ * If this class determines that no transition should be performed, it should return {@code null}.
+ */
+ @Nullable
+ Transition buildTransitionFor(Rule rule);
+}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index e8bad99..696027c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -23,6 +23,7 @@
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
import com.google.common.base.Function;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -56,12 +57,15 @@
import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.NativeAspectClass;
+import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
+import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.syntax.Type;
@@ -1040,6 +1044,65 @@
}
}
+ private static class SetsTestFilterFromAttributePatchTransition implements PatchTransition {
+ private final String value;
+
+ public SetsTestFilterFromAttributePatchTransition(String value) {
+ this.value = value;
+ }
+
+ @Override
+ public BuildOptions apply(BuildOptions options) {
+ BuildOptions result = options.clone();
+ result.get(Options.class).testFilter = "SET BY PATCH FACTORY: " + value;
+ return result;
+ }
+
+ @Override
+ public boolean defaultsToSelf() {
+ return true;
+ }
+ }
+
+ private static class SetsTestFilterFromAttributeTransitionFactory
+ implements RuleTransitionFactory {
+ @Override
+ public Transition buildTransitionFor(Rule rule) {
+ NonconfigurableAttributeMapper attributes = NonconfigurableAttributeMapper.of(rule);
+ String value = attributes.get("sets_test_filter_to", STRING);
+ if (Strings.isNullOrEmpty(value)) {
+ return null;
+ } else {
+ return new SetsTestFilterFromAttributePatchTransition(value);
+ }
+ }
+ }
+
+ /**
+ * Rule with a RuleTransitionFactory which sets the --test_filter flag according to its attribute.
+ */
+ public static class UsesRuleTransitionFactoryRule implements RuleDefinition {
+ @Override
+ public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
+ return builder
+ .cfg(new SetsTestFilterFromAttributeTransitionFactory())
+ .add(
+ attr("sets_test_filter_to", STRING)
+ .nonconfigurable("used in RuleTransitionFactory")
+ .value(""))
+ .build();
+ }
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("uses_rule_transition_factory")
+ .factoryClass(DummyRuleFactory.class)
+ .ancestors(BaseRule.class)
+ .build();
+ }
+ }
+
/** A rule with an empty split transition on an attribute. */
public static class EmptySplitRule implements RuleDefinition {
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 39e9523..08c86ee 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -49,7 +49,6 @@
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.events.Location.LineAndColumn;
import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
-import com.google.devtools.build.lib.packages.Attribute.Transition;
import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
@@ -861,7 +860,7 @@
boolean outputsDefaultExecutable,
ImplicitOutputsFunction implicitOutputsFunction,
Configurator<?, ?> configurator,
- Transition transition,
+ RuleTransitionFactory transitionFactory,
ConfiguredTargetFactory<?, ?> configuredTargetFactory,
PredicateWithMessage<Rule> validityPredicate,
Predicate<String> preferredDependencyPredicate,
@@ -889,7 +888,7 @@
outputsDefaultExecutable,
implicitOutputsFunction,
configurator,
- transition,
+ transitionFactory,
configuredTargetFactory,
validityPredicate,
preferredDependencyPredicate,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
index 43a4b43..a587eba 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java
@@ -148,6 +148,67 @@
assertThat(target.getConfiguration().getCpu()).isNotEqualTo("SET BY PATCH");
}
+ @Test
+ public void ruleTransitionFactoryUsesNonconfigurableAttributesToGenerateTransition()
+ throws Exception {
+ setRulesAvailableInTests(
+ new TestAspects.BaseRule(),
+ new TestAspects.AttributeTransitionRule(),
+ new TestAspects.UsesRuleTransitionFactoryRule());
+ useConfiguration("--test_filter=SET ON COMMAND LINE: original and best");
+ scratch.file(
+ "a/BUILD",
+ "attribute_transition(",
+ " name='top',",
+ " without_transition=':factory',",
+ ")",
+ "uses_rule_transition_factory(",
+ " name='factory',",
+ " sets_test_filter_to='funkiest',",
+ ")");
+ List<ConfiguredTarget> deps = getConfiguredDeps("//a:top", "without_transition");
+ BuildConfiguration config = Iterables.getOnlyElement(deps).getConfiguration();
+ assertThat(config.getTestFilter()).isEqualTo("SET BY PATCH FACTORY: funkiest");
+ }
+
+ @Test
+ public void ruleTransitionFactoryCanReturnNullToCauseNoTransition() throws Exception {
+ setRulesAvailableInTests(
+ new TestAspects.BaseRule(),
+ new TestAspects.AttributeTransitionRule(),
+ new TestAspects.UsesRuleTransitionFactoryRule());
+ useConfiguration("--test_filter=SET ON COMMAND LINE: original and best");
+ scratch.file(
+ "a/BUILD",
+ "attribute_transition(",
+ " name='top',",
+ " without_transition=':factory',",
+ ")",
+ "uses_rule_transition_factory(",
+ " name='factory',",
+ " sets_test_filter_to='',",
+ ")");
+ List<ConfiguredTarget> deps = getConfiguredDeps("//a:top", "without_transition");
+ BuildConfiguration config = Iterables.getOnlyElement(deps).getConfiguration();
+ assertThat(config.getTestFilter()).isEqualTo("SET ON COMMAND LINE: original and best");
+ }
+
+ @Test
+ public void topLevelRuleTransitionFactoryUsesNonconfigurableAttributes() throws Exception {
+ setRulesAvailableInTests(
+ new TestAspects.BaseRule(), new TestAspects.UsesRuleTransitionFactoryRule());
+ useConfiguration("--test_filter=SET ON COMMAND LINE: original and best");
+ scratch.file(
+ "a/BUILD",
+ "uses_rule_transition_factory(",
+ " name='factory',",
+ " sets_test_filter_to='Maximum Dance',",
+ ")");
+ ConfiguredTarget target = Iterables.getOnlyElement(update("//a:factory").getTargetsToBuild());
+ assertThat(target.getConfiguration().getTestFilter())
+ .isEqualTo("SET BY PATCH FACTORY: Maximum Dance");
+ }
+
/**
* Returns a custom {@link PatchTransition} with the given value added to
* {@link BuildConfiguration.Options#testFilter}.
