Add dynamic config support for top-level configuration hooks.

PiperOrigin-RevId: 155223580

4 files changed, 149 insertions, 23 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 5a45dc6bca..4c22da39fb 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
@@ -40,6 +40,7 @@ import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAsp
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 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.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -840,12 +841,19 @@ public class BuildView {
     for (BuildConfiguration config : configurations.getTargetConfigurations()) {
       for (Target target : targets) {
         nodes.add(new TargetAndConfiguration(target,
-            BuildConfigurationCollection.configureTopLevelTarget(config, target)));
+            config.useDynamicConfigurations()
+                // Dynamic configurations apply top-level transitions through a different code path:
+                // BuildConfiguration#topLevelConfigurationHook. That path has the advantages of a)
+                // not requiring a global transitions table and b) making its choices outside core
+                // Bazel code.
+                ? (target.isConfigurable() ? config : null)
+                : BuildConfigurationCollection.configureTopLevelTarget(config, target)));
       }
     }
-    return configurations.useDynamicConfigurations()
-        ? getDynamicConfigurations(nodes, eventHandler)
-        : ImmutableList.copyOf(nodes);
+    return ImmutableList.copyOf(
+        configurations.useDynamicConfigurations()
+            ? getDynamicConfigurations(nodes, eventHandler)
+            : nodes);
   }
 
   /**
@@ -855,7 +863,8 @@ public class BuildView {
    *
    * <p>Else returns configurations that unconditionally include all fragments.
    *
-   * <p>Preserves the original input order. Uses original (untrimmed) configurations for targets
+   * <p>Preserves the original input order (but merges duplicate nodes that might occur due to
+   * top-level configuration transitions) . Uses original (untrimmed) configurations for targets
    * that can't be evaluated (e.g. due to loading phase errors).
    *
    * <p>This is suitable for feeding {@link ConfiguredTargetValue} keys: as general principle {@link
@@ -864,7 +873,7 @@ public class BuildView {
    */
   // TODO(bazel-team): error out early for targets that fail - untrimmed configurations should
   // never make it through analysis (and especially not seed ConfiguredTargetValues)
-  private List<TargetAndConfiguration> getDynamicConfigurations(
+  private LinkedHashSet<TargetAndConfiguration> getDynamicConfigurations(
       Iterable<TargetAndConfiguration> inputs, ExtendedEventHandler eventHandler)
       throws InterruptedException {
     Map<Label, Target> labelsToTargets = new LinkedHashMap<>();
@@ -876,23 +885,10 @@ public class BuildView {
 
     for (TargetAndConfiguration targetAndConfig : inputs) {
       labelsToTargets.put(targetAndConfig.getLabel(), targetAndConfig.getTarget());
-
-      Attribute.Transition ruleclassTransition = null;
-      if (targetAndConfig.getTarget().getAssociatedRule() != null) {
-        Rule associatedRule = targetAndConfig.getTarget().getAssociatedRule();
-        RuleTransitionFactory transitionFactory =
-            associatedRule.getRuleClassObject().getTransitionFactory();
-        if (transitionFactory != null) {
-          ruleclassTransition = transitionFactory.buildTransitionFor(associatedRule);
-        }
-      }
       if (targetAndConfig.getConfiguration() != null) {
         asDeps.put(targetAndConfig.getConfiguration(),
             Dependency.withTransitionAndAspects(
-                targetAndConfig.getLabel(),
-                ruleclassTransition == null
-                  ? Attribute.ConfigurationTransition.NONE
-                  : ruleclassTransition,
+                targetAndConfig.getLabel(), getTopLevelTransition(targetAndConfig),
                 // TODO(bazel-team): support top-level aspects
                 AspectCollection.EMPTY));
       }
@@ -916,8 +912,7 @@ public class BuildView {
       }
     }
 
-    ImmutableList.Builder<TargetAndConfiguration> result =
-        ImmutableList.<TargetAndConfiguration>builder();
+    LinkedHashSet<TargetAndConfiguration> result = new LinkedHashSet<>();
     for (TargetAndConfiguration originalInput : inputs) {
       if (successfullyEvaluatedTargets.containsKey(originalInput)) {
         // The configuration was successfully trimmed.
@@ -927,9 +922,46 @@ public class BuildView {
         result.add(originalInput);
       }
     }
-    return result.build();
+    return result;
+  }
+
+  /**
+   * 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) {
+    Target target = targetAndConfig.getTarget();
+    BuildConfiguration fromConfig = targetAndConfig.getConfiguration();
+    Preconditions.checkArgument(fromConfig.useDynamicConfigurations());
+
+    // Top-level transitions (chosen by configuration fragments):
+    Transition topLevelTransition = fromConfig.topLevelConfigurationHook(target);
+    if (topLevelTransition == null) {
+      topLevelTransition = ConfigurationTransition.NONE;
+    }
+
+    // Rule class transitions (chosen by rule class definitions):
+    if (target.getAssociatedRule() == null) {
+      return topLevelTransition;
+    }
+    Rule associatedRule = target.getAssociatedRule();
+    RuleTransitionFactory transitionFactory =
+        associatedRule.getRuleClassObject().getTransitionFactory();
+    if (transitionFactory == null) {
+      return topLevelTransition;
+    }
+    Attribute.Transition ruleClassTransition = transitionFactory.buildTransitionFor(associatedRule);
+    if (ruleClassTransition == null) {
+      return topLevelTransition;
+    } else if (topLevelTransition == ConfigurationTransition.NONE) {
+      return ruleClassTransition;
+    } else {
+      return new ComposingSplitTransition(topLevelTransition, ruleClassTransition);
+    }
   }
 
+
   /**
    * Gets a dynamic configuration for the given target.
    *
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 e942556160..8514cdd7cd 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
@@ -212,6 +212,20 @@ public final class BuildConfiguration {
     public PatchTransition getArtifactOwnerTransition() {
       return null;
     }
+
+    /**
+     * Returns an extra transition that should apply to top-level targets in this
+     * configuration. Returns null if no transition is needed.
+     *
+     * <p>Overriders should not change {@link FragmentOptions} not associated with their fragment.
+     *
+     * <p>If multiple fragments specify a transition, they're composed together in a
+     * deterministic but undocumented order (so don't write code expecting a specific order).
+     */
+    @Nullable
+    public PatchTransition topLevelConfigurationHook(Target toTarget) {
+      return null;
+    }
   }
 
   private static final Label convertLabel(String input) throws OptionsParsingException {
@@ -2679,4 +2693,24 @@ public final class BuildConfiguration {
   public ImmutableCollection<String> getSkylarkFragmentNames() {
     return skylarkVisibleFragments.keySet();
   }
+
+  /**
+   * Returns an extra transition that should apply to top-level targets in this
+   * configuration. Returns null if no transition is needed.
+   */
+  @Nullable
+  public PatchTransition topLevelConfigurationHook(Target toTarget) {
+    PatchTransition currentTransition = null;
+    for (Fragment fragment : fragments.values()) {
+      PatchTransition fragmentTransition = fragment.topLevelConfigurationHook(toTarget);
+      if (fragmentTransition == null) {
+        continue;
+      } else if (currentTransition == null) {
+        currentTransition = fragmentTransition;
+      } else {
+        currentTransition = new ComposingPatchTransition(currentTransition, fragmentTransition);
+      }
+    }
+    return currentTransition;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java
new file mode 100644
index 0000000000..fb4c7e5d6c
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java
@@ -0,0 +1,42 @@
+// 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.common.collect.Iterables;
+
+/**
+ * A {@link ComposingSplitTransition} that only supports {@link PatchTransition}s
+ *
+ * <p>Calling code that doesn't want to have to handle splits should prefer this version.
+ */
+public class ComposingPatchTransition implements PatchTransition {
+  private final ComposingSplitTransition delegate;
+
+  public ComposingPatchTransition(PatchTransition transition1, PatchTransition transition2) {
+    this.delegate = new ComposingSplitTransition(transition1, transition2);
+  }
+
+  @Override
+  public boolean defaultsToSelf() {
+    throw new UnsupportedOperationException(
+        "dynamic configurations don't use global transition tables");
+  }
+
+  @Override
+  public BuildOptions apply(BuildOptions options) {
+    return Iterables.getOnlyElement(delegate.split(options));
+  }
+}
+
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index b911026c94..4388571311 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -40,10 +40,13 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.OutputFile;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters;
 import com.google.devtools.build.lib.rules.cpp.CppLinkActionConfigs.CppLinkPlatform;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.transitions.ContextCollectorOwnerTransition;
+import com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -2253,4 +2256,19 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   public PatchTransition getArtifactOwnerTransition() {
     return isLipoContextCollector() ? ContextCollectorOwnerTransition.INSTANCE : null;
   }
+
+  @Nullable
+  @Override
+  public PatchTransition topLevelConfigurationHook(Target toTarget) {
+    // Top-level output files that aren't outputs of the LIPO context should be built in
+    // the data config. This is so their output path prefix doesn't have "-lipo" in it, which
+    // is a confusing and unnecessary deviation from how they would normally look.
+    if (toTarget instanceof OutputFile
+        && isLipoOptimization()
+        && !toTarget.getAssociatedRule().getLabel().equals(getLipoContextLabel())) {
+      return DisableLipoTransition.INSTANCE;
+    } else {
+      return null;
+    }
+  }
 }