Remove RuleClass.Configurator and obsolete LIPO_ON_DEMAND uses.
Dynamic configs use RuleTransitionFactory instead.
Part of the static config cleanup effort.
PiperOrigin-RevId: 165590679
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 7dcb00c..6f97280 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
@@ -60,7 +60,6 @@
import com.google.devtools.build.lib.packages.InputFile;
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;
@@ -1785,15 +1784,6 @@
currentConfig.getTransitions().configurationHook(fromRule, attribute, toTarget, delegate);
currentConfig = Iterables.getOnlyElement(delegate.toConfigurations);
- // Allow rule classes to override their own configurations.
- Rule associatedRule = toTarget.getAssociatedRule();
- if (associatedRule != null) {
- @SuppressWarnings("unchecked")
- RuleClass.Configurator<BuildConfiguration, Rule> func =
- associatedRule.getRuleClassObject().<BuildConfiguration, Rule>getConfigurator();
- currentConfig = func.apply(associatedRule, currentConfig);
- }
-
toConfigs.add(currentConfig);
}
toConfigurations = toConfigs.build();
@@ -1953,22 +1943,6 @@
}
}
}
-
- /**
- * Dynamic configurations don't support rule class configurators (which may need intermediate
- * configurations to apply). The only current use of that is LIPO, which dynamic
- * configurations have a different code path for:
- * {@link com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LIPO_ON_DEMAND}.
- *
- * So just check that if there is a configurator, it's for LIPO, in which case we can ignore
- * it.
- */
- if (associatedRule != null) {
- @SuppressWarnings("unchecked")
- RuleClass.Configurator<?, ?> func =
- associatedRule.getRuleClassObject().getConfigurator();
- Verify.verify(func == RuleClass.NO_CHANGE || func.getCategory().equals("lipo"));
- }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java
index 26dbe4c..df577ad 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java
@@ -73,8 +73,7 @@
attr("linkshared", BOOLEAN)
.value(false)
.nonconfigurable("used to *determine* the rule's configuration"))
- .cfg(BazelCppRuleClasses.LIPO_ON_DEMAND) // static configuration version
- .cfg(CppRuleClasses.LIPO_ON_DEMAND) // dynamic configuration version
+ .cfg(CppRuleClasses.LIPO_ON_DEMAND)
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
index 206e439..7df9e4e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
@@ -37,7 +37,6 @@
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.Preconditions;
import com.google.common.base.Predicates;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
@@ -72,29 +71,6 @@
static final SafeImplicitOutputsFunction CC_BINARY_IMPLICIT_OUTPUTS =
fromFunctions(CppRuleClasses.CC_BINARY_STRIPPED, CppRuleClasses.CC_BINARY_DEBUG_PACKAGE);
- static final RuleClass.Configurator<BuildConfiguration, Rule> LIPO_ON_DEMAND =
- new RuleClass.Configurator<BuildConfiguration, Rule>() {
- @Override
- public BuildConfiguration apply(Rule rule, BuildConfiguration configuration) {
- Preconditions.checkState(!configuration.useDynamicConfigurations(),
- "Dynamic configurations don't use rule class configurators for LIPO");
- BuildConfiguration toplevelConfig =
- configuration.getConfiguration(LipoTransition.TARGET_CONFIG_FOR_LIPO);
- CppConfiguration cppConfig = configuration.getFragment(CppConfiguration.class);
- if (toplevelConfig != null
- && cppConfig.isDataConfigurationForLipoOptimization()
- && rule.getLabel().equals(cppConfig.getLipoContextForBuild())) {
- return toplevelConfig;
- }
- return configuration;
- }
-
- @Override
- public String getCategory() {
- return "lipo";
- }
- };
-
public static final LateBoundLabel<BuildConfiguration> STL =
new LateBoundLabel<BuildConfiguration>() {
@Override
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 efc9d3c..7cbb25f 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
@@ -170,20 +170,6 @@
}
/**
- * Using this callback function, rules can override their own configuration during the
- * analysis phase.
- */
- public interface Configurator<TConfig, TRule> {
- TConfig apply(TRule rule, TConfig configuration);
-
- /**
- * Describes the Bazel feature this configurator is used for. Used for checking that dynamic
- * configuration transitions are only applied to expected configurator types.
- */
- String getCategory();
- }
-
- /**
* A factory or builder class for rule implementations.
*/
public interface ConfiguredTargetFactory<TConfiguredTarget, TContext> {
@@ -204,22 +190,6 @@
}
/**
- * Default rule configurator, it doesn't change the assigned configuration.
- */
- public static final RuleClass.Configurator<Object, Object> NO_CHANGE =
- new RuleClass.Configurator<Object, Object>() {
- @Override
- public Object apply(Object rule, Object configuration) {
- return configuration;
- }
-
- @Override
- public String getCategory() {
- return "core";
- }
- };
-
- /**
* For Bazel's constraint system: the attribute that declares the set of environments a rule
* supports, overriding the defaults for their respective groups.
*/
@@ -493,7 +463,6 @@
private boolean outputsDefaultExecutable = false;
private boolean isConfigMatcher = false;
private ImplicitOutputsFunction implicitOutputsFunction = ImplicitOutputsFunction.NONE;
- private Configurator<?, ?> configurator = NO_CHANGE;
private RuleTransitionFactory transitionFactory;
private RuleTransitionFactory outgoingTransitionFactory;
private ConfiguredTargetFactory<?, ?> configuredTargetFactory = null;
@@ -614,7 +583,6 @@
outputsDefaultExecutable,
implicitOutputsFunction,
isConfigMatcher,
- configurator,
transitionFactory,
outgoingTransitionFactory,
configuredTargetFactory,
@@ -749,16 +717,6 @@
return this;
}
- public Builder cfg(Configurator<?, ?> configurator) {
- 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(configurator);
- this.configurator = configurator;
- return this;
- }
-
/**
* Applies the given transition to all incoming edges for this rule class.
*
@@ -1093,12 +1051,6 @@
private final ImplicitOutputsFunction implicitOutputsFunction;
/**
- * The set of implicit outputs generated by a rule, expressed as a function
- * of that rule.
- */
- private final Configurator<?, ?> configurator;
-
- /**
* 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.
*/
@@ -1198,7 +1150,6 @@
boolean outputsDefaultExecutable,
ImplicitOutputsFunction implicitOutputsFunction,
boolean isConfigMatcher,
- Configurator<?, ?> configurator,
RuleTransitionFactory transitionFactory,
RuleTransitionFactory outgoingRuleTransitionFactory,
ConfiguredTargetFactory<?, ?> configuredTargetFactory,
@@ -1224,7 +1175,6 @@
this.binaryOutput = binaryOutput;
this.implicitOutputsFunction = implicitOutputsFunction;
this.isConfigMatcher = isConfigMatcher;
- this.configurator = Preconditions.checkNotNull(configurator);
this.transitionFactory = transitionFactory;
this.outgoingTransitionFactory = outgoingRuleTransitionFactory;
this.configuredTargetFactory = configuredTargetFactory;
@@ -1293,11 +1243,6 @@
return implicitOutputsFunction;
}
- @SuppressWarnings("unchecked")
- public <C, R> Configurator<C, R> getConfigurator() {
- return (Configurator<C, R>) configurator;
- }
-
public RuleTransitionFactory getTransitionFactory() {
return transitionFactory;
}
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 56dc190..d82c27b 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
@@ -48,7 +48,6 @@
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;
-import com.google.devtools.build.lib.packages.RuleClass.Configurator;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
@@ -101,7 +100,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -139,7 +137,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -268,7 +265,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -316,7 +312,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -416,7 +411,6 @@
false,
ImplicitOutputsFunction.fromTemplates(
"foo-%{name}.bar", "lib%{name}-wazoo-%{name}.mumble", "stuff-%{outs}-bar"),
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -458,7 +452,6 @@
false,
false,
ImplicitOutputsFunction.fromTemplates("%{dirname}lib%{basename}.bar"),
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -496,7 +489,6 @@
false,
false,
ImplicitOutputsFunction.fromTemplates("empty"),
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -659,7 +651,6 @@
false,
false,
ImplicitOutputsFunction.fromTemplates("first-%{name}", "second-%{name}", "out-%{outs}"),
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -703,7 +694,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
@@ -854,7 +844,6 @@
boolean workspaceOnly,
boolean outputsDefaultExecutable,
ImplicitOutputsFunction implicitOutputsFunction,
- Configurator<?, ?> configurator,
RuleTransitionFactory transitionFactory,
ConfiguredTargetFactory<?, ?> configuredTargetFactory,
PredicateWithMessage<Rule> validityPredicate,
@@ -883,7 +872,6 @@
outputsDefaultExecutable,
implicitOutputsFunction,
/*isConfigMatcher=*/ false,
- configurator,
transitionFactory,
null,
configuredTargetFactory,
@@ -914,7 +902,6 @@
false,
false,
ImplicitOutputsFunction.NONE,
- RuleClass.NO_CHANGE,
null,
DUMMY_CONFIGURED_TARGET_FACTORY,
PredicatesWithMessage.<Rule>alwaysTrue(),
