apply configuration transitions when following alias labels
PiperOrigin-RevId: 393033365
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 81ae07d..6cd2826 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
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -26,8 +28,11 @@
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment.MissingDepException;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
+import com.google.devtools.build.lib.analysis.Dependency;
+import com.google.devtools.build.lib.analysis.DependencyKey;
import com.google.devtools.build.lib.analysis.DependencyKind;
import com.google.devtools.build.lib.analysis.DuplicateException;
import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
@@ -38,8 +43,12 @@
import com.google.devtools.build.lib.analysis.ToolchainCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.ConfigConditions;
+import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.analysis.config.TransitionResolver;
+import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.causes.Cause;
@@ -52,8 +61,10 @@
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NativeAspectClass;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.OutputFile;
@@ -281,11 +292,10 @@
ConfiguredTarget associatedTarget = baseConfiguredTargetValue.getConfiguredTarget();
- ConfiguredTargetAndData associatedConfiguredTargetAndData;
Package targetPkg;
BuildConfiguration configuration = null;
PackageValue.Key packageKey =
- PackageValue.key(associatedTarget.getLabel().getPackageIdentifier());
+ PackageValue.key(associatedTarget.getOriginalLabel().getPackageIdentifier());
if (associatedTarget.getConfigurationKey() == null) {
PackageValue val = ((PackageValue) env.getValue(packageKey));
if (val == null) {
@@ -307,25 +317,34 @@
((BuildConfigurationValue) result.get(associatedTarget.getConfigurationKey()))
.getConfiguration();
}
+
+ Target target;
try {
- associatedConfiguredTargetAndData =
- new ConfiguredTargetAndData(
- associatedTarget,
- targetPkg.getTarget(associatedTarget.getLabel().getName()),
- configuration,
- null);
+ target = targetPkg.getTarget(associatedTarget.getOriginalLabel().getName());
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Name already verified", e);
}
- if (baseConfiguredTargetValue.getConfiguredTarget().getProvider(AliasProvider.class) != null) {
+ if (AliasProvider.isAlias(associatedTarget)) {
return createAliasAspect(
env,
- associatedConfiguredTargetAndData.getTarget(),
+ view.getHostConfiguration(aspectConfiguration),
+ new TargetAndConfiguration(target, configuration),
aspect,
key,
- baseConfiguredTargetValue.getConfiguredTarget());
+ aspectConfiguration,
+ associatedTarget);
}
+ // If we get here, label should match original label, and therefore the target we looked up
+ // above indeed corresponds to associatedTarget.getLabel().
+ Preconditions.checkState(
+ associatedTarget.getOriginalLabel().equals(associatedTarget.getLabel()),
+ "Non-alias %s should have matching label but found %s",
+ associatedTarget.getOriginalLabel(),
+ associatedTarget.getLabel());
+
+ ConfiguredTargetAndData associatedConfiguredTargetAndData =
+ new ConfiguredTargetAndData(associatedTarget, target, configuration, null);
// If the incompatible flag is set, the top-level aspect should not be applied on top-level
// targets whose rules do not advertise the aspect's required providers. The aspect should not
@@ -338,7 +357,6 @@
starlarkSemantics.getBool(
BuildLanguageOptions.INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS);
if (checkRuleAdvertisedProviders) {
- Target target = associatedConfiguredTargetAndData.getTarget();
if (target instanceof Rule) {
if (!aspect
.getDefinition()
@@ -406,27 +424,8 @@
associatedConfiguredTargetAndData.getTarget(), aspectConfiguration);
ImmutableList<Aspect> aspectPath = aspectPathBuilder.build();
try {
- // Determine what toolchains are needed by this target.
- UnloadedToolchainContext unloadedToolchainContext = null;
- if (configuration != null) {
- // Configuration can be null in the case of aspects applied to input files. In this case,
- // there are no chances of toolchains being used, so skip it.
- try {
- ImmutableSet<Label> requiredToolchains = aspect.getDefinition().getRequiredToolchains();
- unloadedToolchainContext =
- (UnloadedToolchainContext)
- env.getValueOrThrow(
- ToolchainContextKey.key()
- .configurationKey(BuildConfigurationValue.key(configuration))
- .requiredToolchainTypeLabels(requiredToolchains)
- .build(),
- ToolchainException.class);
- } catch (ToolchainException e) {
- // TODO(katre): better error handling
- throw new AspectCreationException(
- e.getMessage(), new LabelCause(key.getLabel(), e.getDetailedExitCode()));
- }
- }
+ UnloadedToolchainContext unloadedToolchainContext =
+ getUnloadedToolchainContext(env, key, aspect, configuration);
if (env.valuesMissing()) {
return null;
}
@@ -550,6 +549,37 @@
}
}
+ @Nullable
+ private static UnloadedToolchainContext getUnloadedToolchainContext(
+ Environment env, AspectKey key, Aspect aspect, @Nullable BuildConfiguration configuration)
+ throws InterruptedException, AspectCreationException {
+ // Determine what toolchains are needed by this target.
+ UnloadedToolchainContext unloadedToolchainContext = null;
+ if (configuration != null) {
+ // Configuration can be null in the case of aspects applied to input files. In this case,
+ // there are no chances of toolchains being used, so skip it.
+ try {
+ ImmutableSet<Label> requiredToolchains = aspect.getDefinition().getRequiredToolchains();
+ unloadedToolchainContext =
+ (UnloadedToolchainContext)
+ env.getValueOrThrow(
+ ToolchainContextKey.key()
+ .configurationKey(BuildConfigurationValue.key(configuration))
+ .requiredToolchainTypeLabels(requiredToolchains)
+ .build(),
+ ToolchainException.class);
+ } catch (ToolchainException e) {
+ // TODO(katre): better error handling
+ throw new AspectCreationException(
+ e.getMessage(), new LabelCause(key.getLabel(), e.getDetailedExitCode()));
+ }
+ }
+ if (env.valuesMissing()) {
+ return null;
+ }
+ return unloadedToolchainContext;
+ }
+
/**
* Returns whether or not to use the new toolchain transition. Checks the global incompatible
* change flag and the aspect's toolchain transition readiness attribute.
@@ -620,31 +650,125 @@
aspectPathBuilder.add(key);
}
- private SkyValue createAliasAspect(
+ /**
+ * Computes the given aspectKey of an alias-like target, by depending on the corresponding key of
+ * the next target in the alias chain (if there are more), or the "real" configured target.
+ */
+ @Nullable
+ private AspectValue createAliasAspect(
Environment env,
- Target originalTarget,
+ BuildConfiguration hostConfiguration,
+ TargetAndConfiguration originalTarget,
Aspect aspect,
AspectKey originalKey,
+ BuildConfiguration aspectConfiguration,
ConfiguredTarget configuredTarget)
- throws InterruptedException {
- ImmutableList<Label> aliasChain = configuredTarget.getProvider(AliasProvider.class)
- .getAliasChain();
+ throws AspectFunctionException, InterruptedException {
+ ImmutableList<Label> aliasChain =
+ configuredTarget.getProvider(AliasProvider.class).getAliasChain();
// Find the next alias in the chain: either the next alias (if there are two) or the name of
// the real configured target.
- Label aliasLabel = aliasChain.size() > 1 ? aliasChain.get(1) : configuredTarget.getLabel();
+ Label aliasedLabel = aliasChain.size() > 1 ? aliasChain.get(1) : configuredTarget.getLabel();
- return createAliasAspect(env, originalTarget, aspect, originalKey, aliasLabel);
+ NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
+ storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
+ NestedSetBuilder<Cause> transitiveRootCauses = NestedSetBuilder.stableOrder();
+
+ // Compute the Dependency from originalTarget to aliasedLabel
+ Dependency dep;
+ try {
+ UnloadedToolchainContext unloadedToolchainContext =
+ getUnloadedToolchainContext(env, originalKey, aspect, originalTarget.getConfiguration());
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ // See comment in compute() above for why we pair target with aspectConfiguration here
+ TargetAndConfiguration originalTargetAndAspectConfiguration =
+ new TargetAndConfiguration(originalTarget.getTarget(), aspectConfiguration);
+
+ // Get the configuration targets that trigger this rule's configurable attributes.
+ ConfigConditions configConditions =
+ ConfiguredTargetFunction.getConfigConditions(
+ env,
+ originalTargetAndAspectConfiguration,
+ transitivePackagesForPackageRootResolution,
+ unloadedToolchainContext == null ? null : unloadedToolchainContext.targetPlatform(),
+ transitiveRootCauses);
+ if (configConditions == null) {
+ // Those targets haven't yet been resolved.
+ return null;
+ }
+
+ Target aliasedTarget = getTargetFromLabel(env, aliasedLabel);
+ if (aliasedTarget == null) {
+ return null;
+ }
+ ConfigurationTransition transition =
+ TransitionResolver.evaluateTransition(
+ aspectConfiguration,
+ NoTransition.INSTANCE,
+ aliasedTarget,
+ ((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory());
+
+ // Use ConfigurationResolver to apply any configuration transitions on the alias edge.
+ // This is a shortened/simplified variant of ConfiguredTargetFunction.computeDependencies
+ // for just the one special attribute we care about here.
+ DependencyKey depKey =
+ DependencyKey.builder().setLabel(aliasedLabel).setTransition(transition).build();
+ DependencyKind depKind =
+ DependencyKind.AttributeDependencyKind.forRule(
+ getAttributeContainingAlias(originalTarget.getTarget()));
+ ConfigurationResolver resolver =
+ new ConfigurationResolver(
+ env,
+ originalTargetAndAspectConfiguration,
+ hostConfiguration,
+ configConditions.asProviders());
+ ImmutableList<Dependency> deps = resolver.resolveConfiguration(depKind, depKey);
+ if (deps == null) {
+ return null;
+ }
+ // Actual should resolve to exactly one dependency
+ Preconditions.checkState(
+ deps.size() == 1, "Unexpected split in alias %s: %s", originalTarget.getLabel(), deps);
+ dep = deps.get(0);
+ } catch (NoSuchPackageException | NoSuchTargetException e) {
+ throw new AspectFunctionException(e);
+ } catch (ConfiguredValueCreationException e) {
+ throw new AspectFunctionException(e);
+ } catch (AspectCreationException e) {
+ throw new AspectFunctionException(e);
+ }
+
+ if (!transitiveRootCauses.isEmpty()) {
+ NestedSet<Cause> causes = transitiveRootCauses.build();
+ throw new AspectFunctionException(
+ new AspectCreationException(
+ "Loading failed",
+ causes,
+ ConfiguredTargetFunction.getPrioritizedDetailedExitCode(causes)));
+ }
+
+ // Now that we have a Dependency, we can compute the aliased key and depend on it
+ AspectKey actualKey = buildAliasAspectKey(originalKey, aliasedLabel, dep);
+ return createAliasAspect(
+ env,
+ originalTarget.getTarget(),
+ originalKey,
+ aspect,
+ actualKey,
+ transitivePackagesForPackageRootResolution);
}
private AspectValue createAliasAspect(
Environment env,
Target originalTarget,
- Aspect aspect,
AspectKey originalKey,
- Label aliasLabel)
+ Aspect aspect,
+ AspectKey depKey,
+ @Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
throws InterruptedException {
- SkyKey depKey = originalKey.withLabel(aliasLabel);
-
// Compute the AspectValue of the target the alias refers to (which can itself be either an
// alias or a real target)
AspectValue real = (AspectValue) env.getValue(depKey);
@@ -652,20 +776,80 @@
return null;
}
- NestedSet<Package> transitivePackagesForPackageRootResolution =
- storeTransitivePackagesForPackageRootResolution
- ? NestedSetBuilder.<Package>stableOrder()
- .addTransitive(real.getTransitivePackagesForPackageRootResolution())
- .add(originalTarget.getPackage())
- .build()
- : null;
-
+ NestedSet<Package> finalTransitivePackagesForPackageRootResolution = null;
+ if (transitivePackagesForPackageRootResolution != null) {
+ finalTransitivePackagesForPackageRootResolution =
+ transitivePackagesForPackageRootResolution
+ .addTransitive(real.getTransitivePackagesForPackageRootResolution())
+ .add(originalTarget.getPackage())
+ .build();
+ }
return new AspectValue(
originalKey,
aspect,
originalTarget.getLocation(),
ConfiguredAspect.forAlias(real.getConfiguredAspect()),
- transitivePackagesForPackageRootResolution);
+ finalTransitivePackagesForPackageRootResolution);
+ }
+
+ @Nullable
+ private static Target getTargetFromLabel(Environment env, Label aliasLabel)
+ throws InterruptedException, NoSuchPackageException, NoSuchTargetException {
+ SkyValue val =
+ env.getValueOrThrow(
+ PackageValue.key(aliasLabel.getPackageIdentifier()), NoSuchPackageException.class);
+ if (val == null) {
+ return null;
+ }
+
+ Package pkg = ((PackageValue) val).getPackage();
+ return pkg.getTarget(aliasLabel.getName());
+ }
+
+ private static AspectKey buildAliasAspectKey(
+ AspectKey originalKey, Label aliasLabel, Dependency dep) {
+ ImmutableList<AspectKey> aliasedBaseKeys =
+ originalKey.getBaseKeys().stream()
+ .map(baseKey -> buildAliasAspectKey(baseKey, aliasLabel, dep))
+ .collect(toImmutableList());
+ return AspectValueKey.createAspectKey(
+ aliasLabel,
+ dep.getConfiguration(),
+ aliasedBaseKeys,
+ originalKey.getAspectDescriptor(),
+ dep.getAspectConfiguration(originalKey.getAspectDescriptor()));
+ }
+
+ /**
+ * Given an alias-like target, returns the attribute containing the "actual", by looking for
+ * attribute names used in known alias rules (Alias, Bind, LateBoundAlias, XcodeConfigAlias).
+ *
+ * <p>Alias and Bind rules use "actual", which will be by far the most common match here. It'll
+ * likely be rare that aspects need to traverse across other alias-like rules.
+ */
+ // TODO(lberki,kmb): try to avoid this, maybe by recording the attribute name in AliasProvider
+ private static Attribute getAttributeContainingAlias(Target originalTarget) {
+ Attribute aliasAttr = null;
+ for (Attribute attr : originalTarget.getAssociatedRule().getAttributes()) {
+ switch (attr.getName()) {
+ case "actual": // alias and bind rules
+ case ":alias": // LateBoundAlias-derived rules
+ case ":xcode_config": // xcode_config_alias rule
+ Preconditions.checkState(
+ aliasAttr == null,
+ "Found multiple candidate attributes %s and %s in %s",
+ aliasAttr,
+ attr,
+ originalTarget);
+ aliasAttr = attr;
+ break;
+ default:
+ break;
+ }
+ }
+ Preconditions.checkState(
+ aliasAttr != null, "Attribute containing alias not found in %s", originalTarget);
+ return aliasAttr;
}
@Nullable
@@ -701,7 +885,13 @@
&& associatedTarget.getTarget() instanceof OutputFile) {
OutputFile outputFile = (OutputFile) associatedTarget.getTarget();
Label label = outputFile.getGeneratingRule().getLabel();
- return createAliasAspect(env, associatedTarget.getTarget(), aspect, key, label);
+ return createAliasAspect(
+ env,
+ associatedTarget.getTarget(),
+ key,
+ aspect,
+ key.withLabel(label),
+ transitivePackagesForPackageRootResolution);
} else if (AspectResolver.aspectMatchesConfiguredTarget(associatedTarget, aspect)) {
try {
CurrentRuleTracker.beginConfiguredAspect(aspect.getAspectClass());
