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
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 07f5483..0869785 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 @@
/*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 @@
*/
@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 @@
.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 92687d6..c5d7a67 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 @@
@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 @@
@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 @@
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 @@
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 932bc2b..b81ea28 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 @@
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 @@
ConfiguredAttributeMapper.of(builder.rule, configConditions);
this.features = getEnabledFeatures();
this.attributeMap = attributeMap;
+ this.hostConfiguration = builder.hostConfiguration;
}
private ImmutableSet<String> getEnabledFeatures() {
@@ -211,9 +213,7 @@
* 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 @@
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 e36a681..41c1693 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 @@
@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 @@
}
}
- /**
- * 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 @@
}
/**
+ * 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 b00f2c8..7d2efe8 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.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 @@
}
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 68fb4b8..dff91d6 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 @@
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 @@
@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 @@
}
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 d094eba..fbae0e5 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 @@
*/
@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
