diff options
author | Michael Staib <mstaib@google.com> | 2017-03-20 18:06:48 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-03-21 12:51:14 +0000 |
commit | 2a6752078f66cf14d1481486e836571e0df6974c (patch) | |
tree | 95571ee9c2eafe56bdadaec8af7a1d03a3c16cf0 /src | |
parent | 4f472d2b44630fa38d82707f7e33f4aa5005d444 (diff) |
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
Diffstat (limited to 'src')
7 files changed, 220 insertions, 33 deletions
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 df50202141..6043cda299 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.NoSuchThingException; 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 @@ public class BuildView { 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 e7df822128..1fc05d8034 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.PackageGroup; 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 @@ public final class BuildConfiguration { 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 eeacba5ad7..1a2dbb2127 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 @@ public class RuleClass { } } + /** + * 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 @@ public class RuleClass { 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 @@ public class RuleClass { outputsDefaultExecutable, implicitOutputsFunction, configurator, - transition, + transitionFactory, configuredTargetFactory, validityPredicate, preferredDependencyPredicate, @@ -718,8 +734,9 @@ public class RuleClass { 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 class RuleClass { 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 @@ public class RuleClass { 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 @@ public class RuleClass { boolean outputsDefaultExecutable, ImplicitOutputsFunction implicitOutputsFunction, Configurator<?, ?> configurator, - Transition transition, + RuleTransitionFactory transitionFactory, ConfiguredTargetFactory<?, ?> configuredTargetFactory, PredicateWithMessage<Rule> validityPredicate, Predicate<String> preferredDependencyPredicate, @@ -1112,7 +1138,7 @@ public class RuleClass { 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 @@ public class RuleClass { 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 0000000000..215461fa3d --- /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 e8bad99a60..696027c439 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; 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.AspectParameters; 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 @@ public class TestAspects { } } + 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 39e9523cf4..08c86ee6b7 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.EventKind; 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 @@ public class RuleClassTest extends PackageLoadingTestCase { boolean outputsDefaultExecutable, ImplicitOutputsFunction implicitOutputsFunction, Configurator<?, ?> configurator, - Transition transition, + RuleTransitionFactory transitionFactory, ConfiguredTargetFactory<?, ?> configuredTargetFactory, PredicateWithMessage<Rule> validityPredicate, Predicate<String> preferredDependencyPredicate, @@ -889,7 +888,7 @@ public class RuleClassTest extends PackageLoadingTestCase { 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 43a4b435b8..a587ebaf01 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 @@ public class ConfigurationsForTargetsWithDynamicConfigurationsTest 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}. |