Remove outdated Attribute.Configurator interface.

With dynamic configurations we no longer need a special
class to apply transitions: a simple patch transition
bound to an attribute works just as well.

PiperOrigin-RevId: 168442602

4 files changed, 13 insertions, 101 deletions

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 f5829ed2e9..4997a895f8 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
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -376,16 +375,12 @@ public abstract class DependencyResolver {
           getSplitOptions(depResolver.rule, attribute, ruleConfig);
       if (!splitOptions.isEmpty() && !ruleConfig.isHostConfiguration()) {
         // Late-bound attribute with a split transition:
-        // Since we want to get the same results as BuildConfiguration.evaluateTransition (but
+        // Since we want to get the same results as ConfigurationResolver.evaluateTransition (but
         // skip it since we've already applied the split), we want to make sure this logic
-        // doesn't do anything differently. evaluateTransition has additional logic
-        // for host configs and attributes with configurators. To keep the fork as simple as
-        // possible, we don't support the configurator case. And when we're in the host
-        // configuration, we fall back to the non-split branch, which calls
-        // BuildConfiguration.evaluateTransition, which returns its "host mode" result without
-        // ever looking at the split.
-        Verify.verify(attribute.getConfigurator() == null);
-
+        // doesn't do anything differently. ConfigurationResolver.evaluateTransition has additional
+        // logic for host configs. So when we're in the host configuration we fall back to the
+        // non-split branch, which calls ConfigurationResolver.evaluateTransition, which returns its
+        // "host mode" result without ever looking at the split.
         Iterable<BuildConfiguration> splitConfigs =
             getConfigurations(ruleConfig.fragmentClasses(), splitOptions);
         if (splitConfigs == null) {
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 3f28614cb7..162b9defe7 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
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.FragmentCollection;
+import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.analysis.fileset.FilesetProvider;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
@@ -1232,20 +1233,21 @@ public final class RuleContext extends TargetContext
       throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
         + " is not a label type attribute");
     }
+    Attribute.Transition transition = attributeDefinition.getConfigurationTransition();
     if (mode == Mode.HOST) {
-      if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.HOST) {
+      if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.HOST) {
         throw new IllegalStateException(getRule().getLocation() + ": "
             + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the host configuration");
       }
     } else if (mode == Mode.TARGET) {
-      if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.NONE) {
+      if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.NONE) {
         throw new IllegalStateException(getRule().getLocation() + ": "
             + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the target configuration");
       }
     } else if (mode == Mode.DATA) {
-      if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.DATA) {
+      if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.DATA) {
         throw new IllegalStateException(getRule().getLocation() + ": "
             + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the data configuration");
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index 3f6e710430..2da77b0129 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
@@ -15,10 +15,8 @@
 package com.google.devtools.build.lib.analysis.config;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import com.google.devtools.build.lib.packages.Attribute.Configurator;
 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.InputFile;
@@ -109,23 +107,13 @@ public final class ConfigurationResolver {
     // TODO(gregce): make the below transitions composable (i.e. take away the "else" clauses).
     // The "else" is a legacy restriction from static configurations.
     if (attribute.hasSplitConfigurationTransition()) {
-      Preconditions.checkState(attribute.getConfigurator() == null);
       currentTransition = split(currentTransition,
           (SplitTransition<BuildOptions>) attribute.getSplitTransition(fromRule));
     } else {
       // III. Attributes determine configurations. The configuration of a prerequisite is determined
       // by the attribute.
-      @SuppressWarnings("unchecked")
-      Configurator<BuildOptions> configurator =
-          (Configurator<BuildOptions>) attribute.getConfigurator();
-      if (configurator != null) {
-        // TODO(gregce): attribute configurators are a holdover from static configurations. Remove
-        // them and remove this branch.
-        currentTransition = applyAttributeConfigurator(currentTransition, configurator);
-      } else {
-        currentTransition = composeTransitions(currentTransition,
-            attribute.getConfigurationTransition());
-      }
+      currentTransition = composeTransitions(currentTransition,
+          attribute.getConfigurationTransition());
     }
 
     return applyConfigurationHook(currentTransition, toTarget);
@@ -190,37 +178,6 @@ public final class ConfigurationResolver {
   }
 
   /**
-   * Applies the given attribute configurator and composes it after an existing transition.
-   */
-  @VisibleForTesting
-  public Transition applyAttributeConfigurator(Transition currentTransition,
-      Configurator<BuildOptions> configurator) {
-    if (isFinal(currentTransition)) {
-      return currentTransition;
-    }
-    return composeTransitions(currentTransition, new AttributeConfiguratorTransition(configurator));
-  }
-
-  /**
-   * A {@link PatchTransition} that applies an attribute configurator over some input options
-   * to determine which transition to use, then applies that transition over those options
-   * for the final output.
-   */
-  private static final class AttributeConfiguratorTransition implements PatchTransition {
-    private final Configurator<BuildOptions> configurator;
-
-    AttributeConfiguratorTransition(Configurator<BuildOptions> configurator) {
-      this.configurator = configurator;
-    }
-
-    @Override
-    public BuildOptions apply(BuildOptions options) {
-      return Iterables.getOnlyElement(
-          ComposingSplitTransition.apply(options, configurator.apply(options)));
-    }
-  }
-
-  /**
    * Applies any configuration hooks associated with the dep target, composes their transitions
    * after an existing transition, and returns the composed result.
    */
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 85a0f25b99..5a020be02f 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
@@ -311,22 +311,6 @@ public final class Attribute implements Comparable<Attribute> {
       };
 
   /**
-   * Using this callback function, rules can change the configuration of their dependencies during
-   * the analysis phase.
-   *
-   * <p>If dynamic configurations are enabled, the returned transition must be a
-   * {@link com.google.devtools.build.lib.analysis.config.PatchTransition}.
-   *
-   * @deprecated this is only needed for statically configured builds. Dynamically configured builds
-   *     should just use {@link Attribute.Builder#cfg(Transition)}} with a directly provided
-   *     {@link com.google.devtools.build.lib.analysis.config.PatchTransition}.
-   */
-  @Deprecated
-  public interface Configurator<TBuildOptions> {
-    Transition apply(TBuildOptions fromOptions);
-  }
-
-  /**
    * 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
@@ -424,7 +408,6 @@ public final class Attribute implements Comparable<Attribute> {
     private Transition configTransition = ConfigurationTransition.NONE;
     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;
@@ -576,17 +559,6 @@ public final class Attribute implements Comparable<Attribute> {
     }
 
     /**
-     * @deprecated Use {@link #cfg(Transition)}} with a
-     * {@link com.google.devtools.build.lib.analysis.config.PatchTransition} instead. This method
-     * only provides legacy support for statically configured builds.
-     */
-    @Deprecated
-    public Builder<TYPE> cfg(Configurator<?> configurator) {
-      this.configurator = configurator;
-      return this;
-    }
-
-    /**
      * Requires the attribute target to be executable; only for label or label
      * list attributes. Defaults to {@code false}.
      */
@@ -1143,7 +1115,6 @@ public final class Attribute implements Comparable<Attribute> {
           Sets.immutableEnumSet(propertyFlags),
           valueSet ? value : type.getDefaultValue(),
           configTransition,
-          configurator,
           splitTransitionProvider,
           allowedRuleClassesForLabels,
           allowedRuleClassesForLabelsWarning,
@@ -1731,7 +1702,6 @@ public final class Attribute implements Comparable<Attribute> {
 
   private final Transition configTransition;
 
-  private final Configurator<?> configurator;
   private final SplitTransitionProvider splitTransitionProvider;
 
   /**
@@ -1786,7 +1756,6 @@ public final class Attribute implements Comparable<Attribute> {
       Set<PropertyFlag> propertyFlags,
       Object defaultValue,
       Transition configTransition,
-      Configurator<?> configurator,
       SplitTransitionProvider splitTransitionProvider,
       Predicate<RuleClass> allowedRuleClassesForLabels,
       Predicate<RuleClass> allowedRuleClassesForLabelsWarning,
@@ -1798,7 +1767,7 @@ public final class Attribute implements Comparable<Attribute> {
       ImmutableList<RuleAspect<?>> aspects) {
     Preconditions.checkNotNull(configTransition);
     Preconditions.checkArgument(
-        (configTransition == ConfigurationTransition.NONE && configurator == null)
+        (configTransition == ConfigurationTransition.NONE)
         || type.getLabelClass() == LabelClass.DEPENDENCY
         || type.getLabelClass() == LabelClass.NONDEP_REFERENCE,
         "Configuration transitions can only be specified for label or label list attributes");
@@ -1808,8 +1777,6 @@ public final class Attribute implements Comparable<Attribute> {
         name);
     if (isLateBound(name)) {
       LateBoundDefault<?> lateBoundDefault = (LateBoundDefault<?>) defaultValue;
-      Preconditions.checkArgument((configurator == null),
-          "a late bound attribute cannot specify a configurator");
       Preconditions.checkArgument(!lateBoundDefault.useHostConfiguration()
           || (configTransition == ConfigurationTransition.HOST),
           "a late bound default value using the host configuration must use the host transition");
@@ -1820,7 +1787,6 @@ public final class Attribute implements Comparable<Attribute> {
     this.propertyFlags = propertyFlags;
     this.defaultValue = defaultValue;
     this.configTransition = configTransition;
-    this.configurator = configurator;
     this.splitTransitionProvider = splitTransitionProvider;
     this.allowedRuleClassesForLabels = allowedRuleClassesForLabels;
     this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning;
@@ -1920,14 +1886,6 @@ public final class Attribute implements Comparable<Attribute> {
   }
 
   /**
-   * Returns the configurator instance for this attribute for label or label list attributes.
-   * For other attributes it will always return {@code null}.
-   */
-  public Configurator<?> getConfigurator() {
-    return configurator;
-  }
-
-  /**
    * Returns the split configuration transition for this attribute.
    *
    * @param rule the originating {@link Rule} which owns this attribute