Plumbs transition key data through to ConfiguredTargetAndData and makes getSplitPrerequisiteConfiguredTargetAndTargets use transition keys instead of reading CPU information from BuildOptions.
RELNOTES: None
PiperOrigin-RevId: 299425217
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
index 5ed5883..7dc403c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
@@ -29,19 +29,20 @@
* <p>The dep's configuration can be specified in one of two ways:
*
* <p>Explicit configurations: includes the target and the configuration of the dependency
- * configured target and any aspects that may be required, as well as the configurations for
- * these aspects.
+ * configured target and any aspects that may be required, as well as the configurations for these
+ * aspects and an optional transition key. {@link Dependency#getTransitionKey} provides some more
+ * context on transition keys.
*
- * <p>Configuration transitions: includes the target and the desired configuration transitions
- * that should be applied to produce the dependency's configuration. It's the caller's
- * responsibility to construct an actual configuration out of that. A set of aspects is also
- * included; the caller must also construct configurations for each of these.
+ * <p>Configuration transitions: includes the target and the desired configuration transitions that
+ * should be applied to produce the dependency's configuration. It's the caller's responsibility to
+ * construct an actual configuration out of that. A set of aspects is also included; the caller must
+ * also construct configurations for each of these.
*
* <p>Note that the presence of an aspect here does not necessarily mean that it will be created.
* They will be filtered based on the {@link TransitiveInfoProvider} instances their associated
- * configured targets have (we cannot do that here because the configured targets are not
- * available yet). No error or warning is reported in this case, because it is expected that rules
- * sometimes over-approximate the providers they supply in their definitions.
+ * configured targets have (we cannot do that here because the configured targets are not available
+ * yet). No error or warning is reported in this case, because it is expected that rules sometimes
+ * over-approximate the providers they supply in their definitions.
*/
public abstract class Dependency {
@@ -62,9 +63,11 @@
*/
public static Dependency withConfiguration(Label label, BuildConfiguration configuration) {
return new ExplicitConfigurationDependency(
- label, configuration,
+ label,
+ configuration,
AspectCollection.EMPTY,
- ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
+ ImmutableMap.<AspectDescriptor, BuildConfiguration>of(),
+ null);
}
/**
@@ -80,8 +83,30 @@
for (AspectDescriptor aspect : aspects.getAllAspects()) {
aspectBuilder.put(aspect, configuration);
}
- return new ExplicitConfigurationDependency(label, configuration, aspects,
- aspectBuilder.build());
+ return new ExplicitConfigurationDependency(
+ label, configuration, aspects, aspectBuilder.build(), null);
+ }
+
+ /**
+ * Creates a new {@link Dependency} with the given configuration, aspects, and transition key. The
+ * configuration is also applied to all aspects. This should be preferred over {@link
+ * Dependency#withConfigurationAndAspects} if the {@code configuration} was derived from a {@link
+ * ConfigurationTransition}, and so there is a corresponding transition key.
+ *
+ * <p>The configuration and aspects must not be {@code null}.
+ */
+ public static Dependency withConfigurationAspectsAndTransitionKey(
+ Label label,
+ BuildConfiguration configuration,
+ AspectCollection aspects,
+ @Nullable String transitionKey) {
+ ImmutableMap.Builder<AspectDescriptor, BuildConfiguration> aspectBuilder =
+ new ImmutableMap.Builder<>();
+ for (AspectDescriptor aspect : aspects.getAllAspects()) {
+ aspectBuilder.put(aspect, configuration);
+ }
+ return new ExplicitConfigurationDependency(
+ label, configuration, aspects, aspectBuilder.build(), transitionKey);
}
/**
@@ -96,7 +121,7 @@
AspectCollection aspects,
Map<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
return new ExplicitConfigurationDependency(
- label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations));
+ label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations), null);
}
/**
@@ -159,6 +184,14 @@
public abstract BuildConfiguration getAspectConfiguration(AspectDescriptor aspect);
/**
+ * Returns the key of a configuration transition, if exists, associated with this dependency. See
+ * {@link ConfigurationTransition#apply} for more details.
+ *
+ * @throws IllegalStateException if {@link #hasExplicitConfiguration()} returns false.
+ */
+ public abstract String getTransitionKey();
+
+ /**
* Implementation of a dependency with no configuration, suitable for, e.g., source files or
* visibility.
*/
@@ -194,6 +227,12 @@
return null;
}
+ @Nullable
+ @Override
+ public String getTransitionKey() {
+ return null;
+ }
+
@Override
public int hashCode() {
return label.hashCode();
@@ -221,15 +260,19 @@
private final BuildConfiguration configuration;
private final AspectCollection aspects;
private final ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations;
+ @Nullable private final String transitionKey;
public ExplicitConfigurationDependency(
- Label label, BuildConfiguration configuration,
+ Label label,
+ BuildConfiguration configuration,
AspectCollection aspects,
- ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
+ ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations,
+ @Nullable String transitionKey) {
super(label);
this.configuration = Preconditions.checkNotNull(configuration);
this.aspects = Preconditions.checkNotNull(aspects);
this.aspectConfigurations = Preconditions.checkNotNull(aspectConfigurations);
+ this.transitionKey = transitionKey;
}
@Override
@@ -258,9 +301,15 @@
return aspectConfigurations.get(aspect);
}
+ @Nullable
+ @Override
+ public String getTransitionKey() {
+ return transitionKey;
+ }
+
@Override
public int hashCode() {
- return Objects.hash(label, configuration, aspectConfigurations);
+ return Objects.hash(label, configuration, aspectConfigurations, transitionKey);
}
@Override
@@ -325,6 +374,12 @@
}
@Override
+ public String getTransitionKey() {
+ throw new IllegalStateException(
+ "This dependency has a transition, not an explicit configuration.");
+ }
+
+ @Override
public int hashCode() {
return Objects.hash(label, transition, aspects);
}
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 b1b13d0..65f99ac 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
@@ -49,7 +49,6 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
-import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentCollection;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
@@ -856,7 +855,7 @@
}
/**
- * Returns the prerequisites keyed by the CPU of their configurations. If the split transition
+ * Returns the prerequisites keyed by their configuration transition keys. If the split transition
* is not active (e.g. split() returned an empty list), the key is an empty Optional.
*/
public Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>>
@@ -891,8 +890,8 @@
}
/**
- * Returns the prerequisites keyed by the CPU of their configurations. If the split transition is
- * not active (e.g. split() returned an empty list), the key is an empty Optional.
+ * Returns the prerequisites keyed by their transition keys. If the split transition is not active
+ * (e.g. split() returned an empty list), the key is an empty Optional.
*/
public Map<Optional<String>, List<ConfiguredTargetAndData>>
getSplitPrerequisiteConfiguredTargetAndTargets(String attributeName) {
@@ -913,28 +912,23 @@
List<ConfiguredTargetAndData> deps = getConfiguredTargetAndTargetDeps(attributeName);
if (SplitTransition.equals(fromOptions, splitOptions.values())) {
- // The split transition is not active. Defer the decision on which CPU to use.
+ // The split transition is not active.
return ImmutableMap.of(Optional.<String>absent(), deps);
}
- Set<String> cpus = new HashSet<>();
- for (BuildOptions options : splitOptions.values()) {
- // This method should only be called when the split config is enabled on the command line, in
- // which case this cpu can't be null.
- cpus.add(options.get(CoreOptions.class).cpu);
- }
-
// Use an ImmutableListMultimap.Builder here to preserve ordering.
ImmutableListMultimap.Builder<Optional<String>, ConfiguredTargetAndData> result =
ImmutableListMultimap.builder();
for (ConfiguredTargetAndData t : deps) {
- if (t.getConfiguration() != null) {
- result.put(Optional.of(t.getConfiguration().getCpu()), t);
- } else {
- // Source files don't have a configuration, so we add them to all architecture entries.
- for (String cpu : cpus) {
- result.put(Optional.of(cpu), t);
+ if (t.getTransitionKey() == null
+ || t.getTransitionKey().equals(ConfigurationTransition.PATCH_TRANSITION_KEY)) {
+ // The target doesn't have a specific transition key associated. This likely means it's a
+ // non-configurable target, e.g. files, package groups. Pan out to all available keys.
+ for (String key : splitOptions.keySet()) {
+ result.put(Optional.of(key), t);
}
+ } else {
+ result.put(Optional.of(t.getTransitionKey()), t);
}
}
return Multimaps.asMap(result.build());
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index b8732d9..b1303d9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
@@ -53,6 +53,7 @@
import com.google.devtools.build.lib.skyframe.TransitiveTargetKey;
import com.google.devtools.build.lib.skyframe.TransitiveTargetValue;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
@@ -121,9 +122,10 @@
throws ConfiguredTargetFunction.DependencyEvaluationException, InterruptedException {
// Maps each Skyframe-evaluated BuildConfiguration to the dependencies that need that
- // configuration. For cases where Skyframe isn't needed to get the configuration (e.g. when
- // we just re-used the original rule's configuration), we should skip this outright.
- Multimap<SkyKey, Map.Entry<DependencyKind, Dependency>> keysToEntries =
+ // configuration paired with a transition key corresponding to the BuildConfiguration. For cases
+ // where Skyframe isn't needed to get the configuration (e.g. when we just re-used the original
+ // rule's configuration), we should skip this outright.
+ Multimap<SkyKey, Pair<Map.Entry<DependencyKind, Dependency>, String>> keysToEntries =
LinkedListMultimap.create();
// Stores the result of applying a transition to the current configuration using a
@@ -282,8 +284,8 @@
putOnlyEntry(
resolvedDeps,
dependencyEdge,
- Dependency.withConfigurationAndAspects(
- dep.getLabel(), ctgValue.getConfiguration(), dep.getAspects()));
+ Dependency.withConfigurationAspectsAndTransitionKey(
+ dep.getLabel(), ctgValue.getConfiguration(), dep.getAspects(), null));
continue;
}
@@ -297,15 +299,16 @@
}
try {
- for (BuildOptions options : toOptions.values()) {
+ for (Map.Entry<String, BuildOptions> optionsEntry : toOptions.entrySet()) {
if (sameFragments) {
keysToEntries.put(
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
currentConfiguration.fragmentClasses(),
- BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
- depsEntry);
+ BuildOptions.diffForReconstruction(
+ defaultBuildOptions, optionsEntry.getValue())),
+ new Pair<>(depsEntry, optionsEntry.getKey()));
} else {
keysToEntries.put(
@@ -313,8 +316,9 @@
platformMappingValue,
defaultBuildOptions,
depFragments,
- BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
- depsEntry);
+ BuildOptions.diffForReconstruction(
+ defaultBuildOptions, optionsEntry.getValue())),
+ new Pair<>(depsEntry, optionsEntry.getKey()));
}
}
} catch (OptionsParsingException e) {
@@ -355,8 +359,8 @@
}
BuildConfiguration trimmedConfig =
((BuildConfigurationValue) valueOrException.get()).getConfiguration();
- for (Map.Entry<DependencyKind, Dependency> info : keysToEntries.get(key)) {
- Dependency originalDep = info.getValue();
+ for (Pair<Map.Entry<DependencyKind, Dependency>, String> info : keysToEntries.get(key)) {
+ Dependency originalDep = info.first.getValue();
if (trimmedConfig.trimConfigurationsRetroactively()
&& !originalDep.getAspects().isEmpty()) {
String message =
@@ -368,10 +372,10 @@
throw new ConfiguredTargetFunction.DependencyEvaluationException(
new InvalidConfigurationException(message));
}
- DependencyEdge attr = new DependencyEdge(info.getKey(), originalDep.getLabel());
+ DependencyEdge attr = new DependencyEdge(info.first.getKey(), originalDep.getLabel());
Dependency resolvedDep =
- Dependency.withConfigurationAndAspects(
- originalDep.getLabel(), trimmedConfig, originalDep.getAspects());
+ Dependency.withConfigurationAspectsAndTransitionKey(
+ originalDep.getLabel(), trimmedConfig, originalDep.getAspects(), info.second);
Attribute attribute = attr.dependencyKind.getAttribute();
if (attribute != null && attribute.getTransitionFactory().isSplit()) {
resolvedDeps.put(attr, resolvedDep);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
index fc0e372..b566386 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.rules.objc;
import static com.google.devtools.build.lib.packages.Type.STRING;
+import static com.google.devtools.build.lib.rules.apple.AppleConfiguration.IOS_CPU_PREFIX;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
@@ -303,7 +304,7 @@
}
appleCommandLineOptions.configurationDistinguisher = configurationDistinguisher;
- splitBuildOptions.put(cpu, splitOptions);
+ splitBuildOptions.put(IOS_CPU_PREFIX + cpu, splitOptions);
}
return splitBuildOptions.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 0a81c5f..aa6078d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -329,7 +329,8 @@
new ConfiguredTargetAndData(
associatedTarget,
targetPkg.getTarget(associatedTarget.getLabel().getName()),
- configuration);
+ configuration,
+ null);
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Name already verified", e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
index d583220..764aa97 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
@@ -29,10 +29,11 @@
import javax.annotation.Nullable;
/**
- * A container class for a {@link ConfiguredTarget} and associated data, {@link Target} and {@link
- * BuildConfiguration}. In the future, {@link ConfiguredTarget} objects will no longer contain their
- * associated {@link BuildConfiguration}. Consumers that need the {@link Target} or {@link
- * BuildConfiguration} must therefore have access to one of these objects.
+ * A container class for a {@link ConfiguredTarget} and associated data, {@link Target}, {@link
+ * BuildConfiguration}, and an optional transition key. In the future, {@link ConfiguredTarget}
+ * objects will no longer contain their associated {@link BuildConfiguration}. Consumers that need
+ * the {@link Target} or {@link BuildConfiguration} must therefore have access to one of these
+ * objects.
*
* <p>These objects are intended to be short-lived, never stored in Skyframe, since they pair three
* heavyweight objects, a {@link ConfiguredTarget}, a {@link Target} (which holds a {@link
@@ -42,13 +43,18 @@
private final ConfiguredTarget configuredTarget;
private final Target target;
private final BuildConfiguration configuration;
+ @Nullable private final String transitionKey;
@VisibleForTesting
public ConfiguredTargetAndData(
- ConfiguredTarget configuredTarget, Target target, BuildConfiguration configuration) {
+ ConfiguredTarget configuredTarget,
+ Target target,
+ BuildConfiguration configuration,
+ String transitionKey) {
this.configuredTarget = configuredTarget;
this.target = target;
this.configuration = configuration;
+ this.transitionKey = transitionKey;
Preconditions.checkState(
configuredTarget.getLabel().equals(target.getLabel()),
"Unable to construct ConfiguredTargetAndData:"
@@ -104,7 +110,7 @@
}
try {
return new ConfiguredTargetAndData(
- ct, packageValue.getPackage().getTarget(ct.getLabel().getName()), configuration);
+ ct, packageValue.getPackage().getTarget(ct.getLabel().getName()), configuration, null);
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Failed to retrieve target for " + ct, e);
}
@@ -118,7 +124,7 @@
if (configuredTarget.equals(maybeNew)) {
return this;
}
- return new ConfiguredTargetAndData(maybeNew, this.target, configuration);
+ return new ConfiguredTargetAndData(maybeNew, target, configuration, transitionKey);
}
public Target getTarget() {
@@ -132,4 +138,9 @@
public ConfiguredTarget getConfiguredTarget() {
return configuredTarget;
}
+
+ @Nullable
+ public String getTransitionKey() {
+ return transitionKey;
+ }
}
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 04bab2c..446d2a2 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
@@ -831,7 +831,8 @@
new ConfiguredTargetAndData(
depValue.getConfiguredTarget(),
pkgValue.getPackage().getTarget(depLabel.getName()),
- depConfiguration));
+ depConfiguration,
+ dep.getTransitionKey()));
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Target already verified for " + dep, e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 234763b..2c165e3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1844,7 +1844,8 @@
new ConfiguredTargetAndData(
mergedTarget,
packageValue.getPackage().getTarget(configuredTarget.getLabel().getName()),
- resolvedConfig));
+ resolvedConfig,
+ null));
} catch (DuplicateException | NoSuchTargetException e) {
throw new IllegalStateException(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java b/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java
index a2b39f1..8f39f0b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java
@@ -41,7 +41,7 @@
ConfiguredTarget ct = Iterables.getOnlyElement(result.getTargetsToBuild());
TargetAndConfiguration tac = Iterables.getOnlyElement(result.getTopLevelTargetsWithConfigs());
ConfiguredTargetAndData ctAndData =
- new ConfiguredTargetAndData(ct, tac.getTarget(), tac.getConfiguration());
+ new ConfiguredTargetAndData(ct, tac.getTarget(), tac.getConfiguration(), null);
TopLevelArtifactContext context =
new TopLevelArtifactContext(false, false, OutputGroupInfo.DEFAULT_GROUPS);
ArtifactsToBuild artifactsToBuild =
