Refactor HOST configuration transitions to be dynamic-configuration friendly.

Dynamic configuration transitions require access to Skyframe (since they instantiate BuildConfigurations as Skyframe nodes). There are various places in Bazel where static transitions are done with no convenient Skyframe access. This cl shuffles host transitions, in particular, to places that are more amenable.

This change also assumes one host configuration per invocation. While this isn't strictly true (each target configuration can have its own host, and multiple target configurations are possible per build), we don't leverage that functionality in any meaningful way today. So until we have a proper interface for multiple host configurations, let's not block dynamic config progress on it.

--
MOS_MIGRATED_REVID=97008479

7 files changed, 68 insertions, 27 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 07f54837b6..0869785195 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
@@ -897,7 +897,8 @@ public class BuildView {
             /*isSystemEnv=*/false, config.extendedSanityChecks(), eventHandler,
             /*skyframeEnv=*/null, config.isActionsEnabled(), binTools);
     return new RuleContext.Builder(analysisEnvironment,
-        (Rule) target.getTarget(), config, ruleClassProvider.getPrerequisiteValidator())
+        (Rule) target.getTarget(), config, getHostConfigurationForTesting(config),
+        ruleClassProvider.getPrerequisiteValidator())
             .setVisibility(NestedSetBuilder.<PackageSpecification>create(
                 Order.STABLE_ORDER, PackageSpecification.EVERYTHING))
             .setPrerequisites(getPrerequisiteMapForTesting(target))
@@ -911,8 +912,9 @@ public class BuildView {
    */
   @VisibleForTesting
   public RuleContext getRuleContextForTesting(ConfiguredTarget target, AnalysisEnvironment env) {
+    BuildConfiguration targetConfig = target.getConfiguration();
     return new RuleContext.Builder(
-        env, (Rule) target.getTarget(), target.getConfiguration(),
+        env, (Rule) target.getTarget(), targetConfig, getHostConfigurationForTesting(targetConfig),
         ruleClassProvider.getPrerequisiteValidator())
             .setVisibility(NestedSetBuilder.<PackageSpecification>create(
                 Order.STABLE_ORDER, PackageSpecification.EVERYTHING))
@@ -921,6 +923,11 @@ public class BuildView {
             .build();
   }
 
+  private BuildConfiguration getHostConfigurationForTesting(BuildConfiguration config) {
+    // TODO(bazel-team): support dynamic transitions in tests.
+    return config == null ? null : config.getConfiguration(Attribute.ConfigurationTransition.HOST);
+  }
+
   /**
    * For a configured target dependentTarget, returns the desired configured target
    * that is depended upon. Useful for obtaining the a target with aspects
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 92687d6e65..c5d7a671f2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -156,12 +156,12 @@ public final class ConfiguredTargetFactory {
   @Nullable
   public final ConfiguredTarget createConfiguredTarget(AnalysisEnvironment analysisEnvironment,
       ArtifactFactory artifactFactory, Target target, BuildConfiguration config,
-      ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
+      BuildConfiguration hostConfig, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions)
       throws InterruptedException {
     if (target instanceof Rule) {
-      return createRule(
-          analysisEnvironment, (Rule) target, config, prerequisiteMap, configConditions);
+      return createRule(analysisEnvironment, (Rule) target, config, hostConfig,
+          prerequisiteMap, configConditions);
     }
 
     // Visibility, like all package groups, doesn't have a configuration
@@ -203,10 +203,11 @@ public final class ConfiguredTargetFactory {
   @Nullable
   private ConfiguredTarget createRule(
       AnalysisEnvironment env, Rule rule, BuildConfiguration configuration,
+      BuildConfiguration hostConfiguration,
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions) throws InterruptedException {
     // Visibility computation and checking is done for every rule.
-    RuleContext ruleContext = new RuleContext.Builder(env, rule, configuration,
+    RuleContext ruleContext = new RuleContext.Builder(env, rule, configuration, hostConfiguration,
         ruleClassProvider.getPrerequisiteValidator())
         .setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule, null))
         .setPrerequisites(prerequisiteMap)
@@ -272,6 +273,7 @@ public final class ConfiguredTargetFactory {
     RuleContext.Builder builder = new RuleContext.Builder(env,
         associatedTarget.getTarget(),
         associatedTarget.getConfiguration(),
+        getHostConfiguration(associatedTarget.getConfiguration()),
         ruleClassProvider.getPrerequisiteValidator());
     RuleContext ruleContext = builder
         .setVisibility(convertVisibility(
@@ -286,6 +288,11 @@ public final class ConfiguredTargetFactory {
     return aspectFactory.create(associatedTarget, ruleContext);
   }
 
+  private static BuildConfiguration getHostConfiguration(BuildConfiguration config) {
+    // TODO(bazel-team): support dynamic transitions.
+    return config == null ? null : config.getConfiguration(Attribute.ConfigurationTransition.HOST);
+  }
+
   /**
    * A pseudo-implementation for configured targets that creates fail actions for all declared
    * outputs, both implicit and explicit.
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 932bc2b672..b81ea289b5 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
@@ -133,6 +133,7 @@ public final class RuleContext extends TargetContext
   private final AttributeMap attributes;
   private final ImmutableSet<String> features;
   private final Map<String, Attribute> attributeMap;
+  private final BuildConfiguration hostConfiguration;
 
   private ActionOwner actionOwner;
 
@@ -152,6 +153,7 @@ public final class RuleContext extends TargetContext
         ConfiguredAttributeMapper.of(builder.rule, configConditions);
     this.features = getEnabledFeatures();
     this.attributeMap = attributeMap;
+    this.hostConfiguration = builder.hostConfiguration;
   }
 
   private ImmutableSet<String> getEnabledFeatures() {
@@ -211,9 +213,7 @@ public final class RuleContext extends TargetContext
    * host configurations, even during a single build.
    */
   public BuildConfiguration getHostConfiguration() {
-    BuildConfiguration configuration = getConfiguration();
-    // Note: the Builder checks that the configuration is non-null.
-    return configuration.getConfiguration(ConfigurationTransition.HOST);
+    return hostConfiguration;
   }
 
   /**
@@ -1131,16 +1131,19 @@ public final class RuleContext extends TargetContext
     private final AnalysisEnvironment env;
     private final Rule rule;
     private final BuildConfiguration configuration;
+    private final BuildConfiguration hostConfiguration;
     private final PrerequisiteValidator prerequisiteValidator;
     private ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
     private Set<ConfigMatchingProvider> configConditions;
     private NestedSet<PackageSpecification> visibility;
 
     Builder(AnalysisEnvironment env, Rule rule, BuildConfiguration configuration,
+        BuildConfiguration hostConfiguration,
         PrerequisiteValidator prerequisiteValidator) {
       this.env = Preconditions.checkNotNull(env);
       this.rule = Preconditions.checkNotNull(rule);
       this.configuration = Preconditions.checkNotNull(configuration);
+      this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
       this.prerequisiteValidator = prerequisiteValidator;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
index e36a6817bc..41c169337d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
@@ -54,10 +54,13 @@ import java.util.Set;
 @ThreadSafe
 public final class BuildConfigurationCollection implements Serializable {
   private final ImmutableList<BuildConfiguration> targetConfigurations;
+  private final BuildConfiguration hostConfiguration;
 
-  public BuildConfigurationCollection(List<BuildConfiguration> targetConfigurations)
+  public BuildConfigurationCollection(List<BuildConfiguration> targetConfigurations,
+      BuildConfiguration hostConfiguration)
       throws InvalidConfigurationException {
     this.targetConfigurations = ImmutableList.copyOf(targetConfigurations);
+    this.hostConfiguration = hostConfiguration;
 
     // Except for the host configuration (which may be identical across target configs), the other
     // configurations must all have different cache keys or we will end up with problems.
@@ -71,13 +74,6 @@ public final class BuildConfigurationCollection implements Serializable {
     }
   }
 
-  /**
-   * Creates an empty configuration collection which will return null for everything.
-   */
-  public BuildConfigurationCollection() {
-    this.targetConfigurations = ImmutableList.of();
-  }
-
   public static BuildConfiguration configureTopLevelTarget(BuildConfiguration topLevelConfiguration,
       Target toTarget) {
     if (toTarget instanceof InputFile || toTarget instanceof PackageGroup) {
@@ -91,6 +87,18 @@ public final class BuildConfigurationCollection implements Serializable {
   }
 
   /**
+   * Returns the host configuration for this collection.
+   *
+   * <p>Don't use this method. It's limited in that it assumes a single host configuration for
+   * the entire collection. This may not be true in the future and more flexible interfaces based
+   * on dynamic configurations will likely supplant this interface anyway. Its main utility is
+   * to keep Bazel working while dynamic configuration progress is under way.
+   */
+  public BuildConfiguration getHostConfiguration() {
+    return hostConfiguration;
+  }
+
+  /**
    * Returns all configurations that can be reached from the target configuration through any kind
    * of configuration transition.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
index b00f2c8cb8..7d2efe8214 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Supplier;
+import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
@@ -23,6 +24,7 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti
 import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.skyframe.ConfigurationCollectionValue.ConfigurationCollectionKey;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -105,7 +107,18 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       }
       targetConfigurations.add(targetConfiguration);
     }
-    return new BuildConfigurationCollection(targetConfigurations);
+
+    // Sanity check that all host configs are the same. This may not be true once we have
+    // better support for multi-host builds.
+    BuildConfiguration hostConfiguration =
+        targetConfigurations.get(0).getConfiguration(ConfigurationTransition.HOST);
+    for (BuildConfiguration targetConfig :
+        targetConfigurations.subList(1, targetConfigurations.size())) {
+      Verify.verify(
+          targetConfig.getConfiguration(ConfigurationTransition.HOST).equals(hostConfiguration));
+    }
+
+    return new BuildConfigurationCollection(targetConfigurations, hostConfiguration);
   }
 
   @Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 68fb4b8638..dff91d6976 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -170,8 +170,13 @@ final class ConfiguredTargetFunction implements SkyFunction {
 
       ListMultimap<Attribute, ConfiguredTarget> depValueMap =
           computeDependencies(env, resolver, ctgValue, null, configConditions);
+
+      // TODO(bazel-team): Support dynamically created host configurations.
+      BuildConfiguration hostConfiguration = configuration == null
+          ? null : configuration.getConfiguration(Attribute.ConfigurationTransition.HOST);
+
       return createConfiguredTarget(
-          view, env, target, configuration, depValueMap, configConditions);
+          view, env, target, configuration, hostConfiguration, depValueMap, configConditions);
     } catch (DependencyEvaluationException e) {
       throw new ConfiguredTargetFunctionException(e.getRootCauseSkyKey(), e.getCause());
     }
@@ -465,10 +470,9 @@ final class ConfiguredTargetFunction implements SkyFunction {
   @Nullable
   private ConfiguredTargetValue createConfiguredTarget(SkyframeBuildView view,
       Environment env, Target target, BuildConfiguration configuration,
-      ListMultimap<Attribute, ConfiguredTarget> depValueMap,
+      BuildConfiguration hostConfiguration, ListMultimap<Attribute, ConfiguredTarget> depValueMap,
       Set<ConfigMatchingProvider> configConditions)
-      throws ConfiguredTargetFunctionException,
-      InterruptedException {
+      throws ConfiguredTargetFunctionException, InterruptedException {
     StoredEventHandler events = new StoredEventHandler();
     BuildConfiguration ownerConfig = (configuration == null)
         ? null : configuration.getArtifactOwnerConfiguration();
@@ -480,7 +484,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
     }
 
     ConfiguredTarget configuredTarget = view.createConfiguredTarget(target, configuration,
-        analysisEnvironment, depValueMap, configConditions);
+        hostConfiguration, analysisEnvironment, depValueMap, configConditions);
 
     events.replayOn(env.getListener());
     if (events.hasErrors()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index d094eba092..fbae0e5438 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -369,14 +369,13 @@ public final class SkyframeBuildView {
    */
   @Nullable
   ConfiguredTarget createConfiguredTarget(Target target, BuildConfiguration configuration,
-      CachingAnalysisEnvironment analysisEnvironment,
+      BuildConfiguration hostConfiguration, CachingAnalysisEnvironment analysisEnvironment,
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
-      Set<ConfigMatchingProvider> configConditions)
-      throws InterruptedException {
+      Set<ConfigMatchingProvider> configConditions) throws InterruptedException {
     Preconditions.checkState(enableAnalysis,
         "Already in execution phase %s %s", target, configuration);
     return factory.createConfiguredTarget(analysisEnvironment, artifactFactory, target,
-        configuration, prerequisiteMap, configConditions);
+        configuration, hostConfiguration, prerequisiteMap, configConditions);
   }
 
   @Nullable