From df22958d81b78ae9edd0a0874e5595cfa189877c Mon Sep 17 00:00:00 2001 From: mstaib Date: Fri, 31 Mar 2017 21:50:46 +0000 Subject: Add isConfigMatcher property to rule classes. BuildConfiguration currently depends directly on the name of a rule class to see if it is config_setting. This creates a circular dependency between ConfigSetting and BuildConfiguration. As ConfigSetting is being moved to rules/config, this circular dependency will no longer work. This replaces the name special case for a special property of the rule class, which only config_setting should set. RELNOTES: None. PiperOrigin-RevId: 151871084 --- .../lib/analysis/config/BuildConfiguration.java | 3 +-- .../lib/analysis/config/ConfigRuleClasses.java | 1 + .../devtools/build/lib/packages/RuleClass.java | 24 ++++++++++++++++++++++ .../devtools/build/lib/packages/RuleClassTest.java | 1 + 4 files changed, 27 insertions(+), 2 deletions(-) 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 5c479ee495..60c0e207da 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 @@ -1952,8 +1952,7 @@ public final class BuildConfiguration { // from these transitions, since their *purpose* is to do computation on the owning // rule's configuration. // TODO(bazel-team): don't require special casing here. This is far too hackish. - if (toTarget instanceof Rule - && ((Rule) toTarget).getRuleClass().equals(ConfigRuleClasses.ConfigSettingRule.RULE_NAME)) { + if (toTarget instanceof Rule && ((Rule) toTarget).getRuleClassObject().isConfigMatcher()) { transitionApplier.applyTransition(Attribute.ConfigurationTransition.NONE); // Unnecessary. return; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java index c6f5cd450c..7df805c4d5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java @@ -147,6 +147,7 @@ public class ConfigRuleClasses { */ .add(attr(SETTINGS_ATTRIBUTE, STRING_DICT).mandatory() .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON)) + .setIsConfigMatcherForConfigSettingOnly() .build(); } 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 1a2dbb2127..37bc246916 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 @@ -484,6 +484,7 @@ public class RuleClass { private boolean binaryOutput = true; private boolean workspaceOnly = false; private boolean outputsDefaultExecutable = false; + private boolean isConfigMatcher = false; private ImplicitOutputsFunction implicitOutputsFunction = ImplicitOutputsFunction.NONE; private Configurator configurator = NO_CHANGE; private RuleTransitionFactory transitionFactory; @@ -599,6 +600,7 @@ public class RuleClass { workspaceOnly, outputsDefaultExecutable, implicitOutputsFunction, + isConfigMatcher, configurator, transitionFactory, configuredTargetFactory, @@ -968,6 +970,17 @@ public class RuleClass { return this; } + /** + * Causes rules of this type to be evaluated with the parent's configuration, always, so that + * rules which match against parts of the configuration will behave as expected. + * + *

This is only intended for use by {@code config_setting} - other rules should not use this! + */ + public Builder setIsConfigMatcherForConfigSettingOnly() { + this.isConfigMatcher = true; + return this; + } + /** * Returns an Attribute.Builder object which contains a replica of the * same attribute in the parent rule if exists. @@ -999,6 +1012,7 @@ public class RuleClass { private final boolean binaryOutput; private final boolean workspaceOnly; private final boolean outputsDefaultExecutable; + private final boolean isConfigMatcher; /** * A (unordered) mapping from attribute names to small integers indexing into @@ -1115,6 +1129,7 @@ public class RuleClass { boolean workspaceOnly, boolean outputsDefaultExecutable, ImplicitOutputsFunction implicitOutputsFunction, + boolean isConfigMatcher, Configurator configurator, RuleTransitionFactory transitionFactory, ConfiguredTargetFactory configuredTargetFactory, @@ -1137,6 +1152,7 @@ public class RuleClass { this.publicByDefault = publicByDefault; this.binaryOutput = binaryOutput; this.implicitOutputsFunction = implicitOutputsFunction; + this.isConfigMatcher = isConfigMatcher; this.configurator = Preconditions.checkNotNull(configurator); this.transitionFactory = transitionFactory; this.configuredTargetFactory = configuredTargetFactory; @@ -1342,6 +1358,14 @@ public class RuleClass { return supportsConstraintChecking; } + /** + * Returns true if rules of this type should be evaluated with the parent's configuration so that + * they can match on aspects of it. + */ + public boolean isConfigMatcher() { + return isConfigMatcher; + } + /** * Creates a new {@link Rule} {@code r} where {@code r.getPackage()} is the {@link Package} * associated with {@code pkgBuilder}. 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 08c86ee6b7..38a8e0ab3c 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 @@ -887,6 +887,7 @@ public class RuleClassTest extends PackageLoadingTestCase { workspaceOnly, outputsDefaultExecutable, implicitOutputsFunction, + /*isConfigMatcher=*/ false, configurator, transitionFactory, configuredTargetFactory, -- cgit v1.2.3