Simplify Attribute.Configurator, add dynamic configs support.

Specifically:
 1) Read BuildOptions instead of BuildConfiguration
 2) Remove unused extra parameters

1) is especially useful for dynamic configs. Before this change, dynamic configs just didn't support attribute configurators. This is because support would require Skyframe-instantiating temporary intermediate configurations, which is horribly awkward and would massively complicate Bazel's dependency evaluation logic. Using BuildOptions instead of BuildConfiguration completely eliminates the problem.

As a bonus, dynamic configs can compose attribute configurators with any other transitions (including splits). This actually makes them more powerful than static configs. Whether anyone wants to use that composition is a different story, but that's now a policy decision vs. a technical limitation. This should also come in handy for RuleClass configurators, which will likely also leverage this.

--
PiperOrigin-RevId: 150080977
MOS_MIGRATED_REVID=150080977

1 files changed, 26 insertions, 11 deletions

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 9077e0fb4e..a92335518c 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
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.packages;
 
+import static com.google.common.collect.Sets.newEnumSet;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
@@ -327,11 +329,19 @@ public final class Attribute implements Comparable<Attribute> {
       };
 
   /**
-   * Using this callback function, rules can set the configuration of their dependencies during the
-   * analysis phase.
+   * 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}.
    */
-  public interface Configurator<TConfig, TRule> {
-    TConfig apply(TRule fromRule, TConfig fromConfiguration, Attribute attribute, Target toTarget);
+  @Deprecated
+  public interface Configurator<TBuildOptions> {
+    Transition apply(TBuildOptions fromOptions);
   }
 
   /**
@@ -433,7 +443,7 @@ 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 Configurator<?> configurator;
     private SplitTransitionProvider splitTransitionProvider;
     private FileTypeSet allowedFileTypesForLabels;
     private ValidityPredicate validityPredicate = ANY_EDGE;
@@ -586,7 +596,13 @@ public final class Attribute implements Comparable<Attribute> {
       }
     }
 
-    public Builder<TYPE> cfg(Configurator<?, ?> configurator) {
+    /**
+     * @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;
     }
@@ -1694,7 +1710,7 @@ public final class Attribute implements Comparable<Attribute> {
 
   private final Transition configTransition;
 
-  private final Configurator<?, ?> configurator;
+  private final Configurator<?> configurator;
   private final SplitTransitionProvider splitTransitionProvider;
 
   /**
@@ -1755,7 +1771,7 @@ public final class Attribute implements Comparable<Attribute> {
       Set<PropertyFlag> propertyFlags,
       Object defaultValue,
       Transition configTransition,
-      Configurator<?, ?> configurator,
+      Configurator<?> configurator,
       SplitTransitionProvider splitTransitionProvider,
       Predicate<RuleClass> allowedRuleClassesForLabels,
       Predicate<RuleClass> allowedRuleClassesForLabelsWarning,
@@ -1895,7 +1911,7 @@ 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() {
+  public Configurator<?> getConfigurator() {
     return configurator;
   }
 
@@ -2171,8 +2187,7 @@ public final class Attribute implements Comparable<Attribute> {
     builder.condition = condition;
     builder.configTransition = configTransition;
     builder.splitTransitionProvider = splitTransitionProvider;
-    builder.propertyFlags = propertyFlags.isEmpty() ?
-        EnumSet.noneOf(PropertyFlag.class) : EnumSet.copyOf(propertyFlags);
+    builder.propertyFlags = newEnumSet(propertyFlags, PropertyFlag.class);
     builder.value = defaultValue;
     builder.valueSet = false;
     builder.allowedValues = allowedValues;