Replace Transitions.configurationHook with equivalent rule class transitions.

This removes the last dynamic dependency on the static configuration
transition table.

PiperOrigin-RevId: 161162272

4 files changed, 70 insertions, 14 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 a2c9ec75a7..80876cf69d 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
@@ -42,7 +42,9 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollectio
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ComposingSplitTransition;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.analysis.config.DynamicTransitionMapper;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -893,7 +895,9 @@ public class BuildView {
       if (targetAndConfig.getConfiguration() != null) {
         asDeps.put(targetAndConfig.getConfiguration(),
             Dependency.withTransitionAndAspects(
-                targetAndConfig.getLabel(), getTopLevelTransition(targetAndConfig),
+                targetAndConfig.getLabel(),
+                getTopLevelTransition(targetAndConfig,
+                    ruleClassProvider.getDynamicTransitionMapper()),
                 // TODO(bazel-team): support top-level aspects
                 AspectCollection.EMPTY));
       }
@@ -934,8 +938,8 @@ public class BuildView {
    * Returns the transition to apply to the top-level configuration before applying it to this
    * target. This enables support for rule-triggered top-level configuration hooks.
    */
-  private static Attribute.Transition getTopLevelTransition(
-      TargetAndConfiguration targetAndConfig) {
+  private static Attribute.Transition getTopLevelTransition(TargetAndConfiguration targetAndConfig,
+      DynamicTransitionMapper dynamicTransitionMapper) {
     Target target = targetAndConfig.getTarget();
     BuildConfiguration fromConfig = targetAndConfig.getConfiguration();
     Preconditions.checkArgument(fromConfig.useDynamicConfigurations());
@@ -956,7 +960,11 @@ public class BuildView {
     if (transitionFactory == null) {
       return topLevelTransition;
     }
-    Attribute.Transition ruleClassTransition = transitionFactory.buildTransitionFor(associatedRule);
+    // dynamicTransitionMapper is only needed because of Attribute.ConfigurationTransition.DATA:
+    // this is C++-specific but non-C++ rules declare it. So they can't directly provide the
+    // C++-specific patch transition that implements it.
+    PatchTransition ruleClassTransition = (PatchTransition)
+        dynamicTransitionMapper.map(transitionFactory.buildTransitionFor(associatedRule));
     if (ruleClassTransition == null) {
       return topLevelTransition;
     } else if (topLevelTransition == ConfigurationTransition.NONE) {
@@ -1160,7 +1168,12 @@ public class BuildView {
     if (factory == null) {
       return ConfigurationTransition.NONE;
     }
-    Transition transition = factory.buildTransitionFor(rule);
+
+    // dynamicTransitionMapper is only needed because of Attribute.ConfigurationTransition.DATA:
+    // this is C++-specific but non-C++ rules declare it. So they can't directly provide the
+    // C++-specific patch transition that implements it.
+    PatchTransition transition = (PatchTransition)
+        ruleClassProvider.getDynamicTransitionMapper().map(factory.buildTransitionFor(rule));
     return (transition == null) ? ConfigurationTransition.NONE : transition;
   }
 
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 4d718b49fb..4cb3064a01 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
@@ -1912,7 +1912,6 @@ public final class BuildConfiguration implements BuildEvent {
    * transitions that the caller subsequently creates configurations from.
    */
   private static class DynamicTransitionApplier implements TransitionApplier {
-    private final Transitions transitionsManager;
     private final DynamicTransitionMapper dynamicTransitionMapper;
     private boolean splitApplied = false;
 
@@ -1921,9 +1920,7 @@ public final class BuildConfiguration implements BuildEvent {
     // so calling code doesn't need special logic to support combinations.
     private Transition currentTransition = Attribute.ConfigurationTransition.NONE;
 
-    private DynamicTransitionApplier(Transitions transitionsManager,
-        DynamicTransitionMapper dynamicTransitionMapper) {
-      this.transitionsManager = transitionsManager;
+    private DynamicTransitionApplier(DynamicTransitionMapper dynamicTransitionMapper) {
       this.dynamicTransitionMapper = dynamicTransitionMapper;
     }
 
@@ -2032,14 +2029,15 @@ public final class BuildConfiguration implements BuildEvent {
       if (isFinal(currentTransition)) {
         return;
       }
-      transitionsManager.configurationHook(fromRule, attribute, toTarget, this);
-
       Rule associatedRule = toTarget.getAssociatedRule();
       RuleTransitionFactory transitionFactory =
           associatedRule.getRuleClassObject().getTransitionFactory();
       if (transitionFactory != null) {
+        // dynamicTransitionMapper is only needed because of Attribute.ConfigurationTransition.DATA:
+        // this is C++-specific but non-C++ rules declare it. So they can't directly provide the
+        // C++-specific patch transition that implements it.
         PatchTransition ruleClassTransition = (PatchTransition)
-            transitionFactory.buildTransitionFor(associatedRule);
+            dynamicTransitionMapper.map(transitionFactory.buildTransitionFor(associatedRule));
         if (ruleClassTransition != null) {
           if (currentTransition == ConfigurationTransition.NONE) {
             currentTransition = ruleClassTransition;
@@ -2086,7 +2084,7 @@ public final class BuildConfiguration implements BuildEvent {
    */
   public TransitionApplier getTransitionApplier() {
     return useDynamicConfigurations()
-        ? new DynamicTransitionApplier(this.getTransitions(), dynamicTransitionMapper)
+        ? new DynamicTransitionApplier(dynamicTransitionMapper)
         : new StaticTransitionApplier(this);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java
new file mode 100644
index 0000000000..3cc2ff71f4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java
@@ -0,0 +1,45 @@
+// 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.analysis.config;
+
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.RuleTransitionFactory;
+
+/**
+ * A {@link RuleTransitionFactory} that composes other {@link RuleTransitionFactory}s.
+ */
+public class ComposingRuleTransitionFactory implements RuleTransitionFactory {
+
+  private final RuleTransitionFactory rtf1;
+  private final RuleTransitionFactory rtf2;
+
+  /**
+   * Creates a factory that applies the given factories in-order ({@code rtf1} first,
+   * {@code rtf2} second).
+   */
+  public ComposingRuleTransitionFactory(RuleTransitionFactory rtf1, RuleTransitionFactory rtf2) {
+    this.rtf1 = rtf1;
+    this.rtf2 = rtf2;
+  }
+
+  @Override
+  public Attribute.Transition buildTransitionFor(Rule rule) {
+    return new ComposingPatchTransition(
+        (PatchTransition) rtf1.buildTransitionFor(rule),
+        (PatchTransition) rtf2.buildTransitionFor(rule));
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java b/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java
index 1f3a78b46b..9cd4077100 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java
@@ -74,7 +74,7 @@ public final class DynamicTransitionMapper {
    * {@link IllegalArgumentException}.
    */
   public Transition map(Transition fromTransition) {
-    if (fromTransition instanceof PatchTransition) {
+    if (fromTransition instanceof PatchTransition || fromTransition == null) {
       return fromTransition;
     }
     Transition toTransition = map.get(fromTransition);