Add safety checks for aspects.
As aspects are not currently supported in retroactive trimming mode, these
checks ensure that they are not silently executed incorrectly.
Progress on #6524.
PiperOrigin-RevId: 244946454
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 bc1dff2..33d20e1 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
@@ -324,6 +324,11 @@
String skylarkFunctionName = aspect.substring(delimiterPosition + 1);
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
+ if (targetSpec.getConfiguration() != null
+ && targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
+ throw new ViewCreationFailedException(
+ "Aspects were requested, but are not supported in retroactive trimming mode.");
+ }
aspectConfigurations.put(
Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration());
aspectKeys.add(
@@ -342,6 +347,11 @@
if (aspectFactoryClass != null) {
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
+ if (targetSpec.getConfiguration() != null
+ && targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
+ throw new ViewCreationFailedException(
+ "Aspects were requested, but are not supported in retroactive trimming mode.");
+ }
// For invoking top-level aspects, use the top-level configuration for both the
// aspect and the base target while the top-level configuration is untrimmed.
BuildConfiguration configuration = targetSpec.getConfiguration();
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 6adfaae6..b7bfe66 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
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
@@ -197,6 +198,17 @@
if (sameFragments) {
if (transition == NoTransition.INSTANCE) {
+ if (ctgValue.getConfiguration().trimConfigurationsRetroactively()
+ && !dep.getAspects().isEmpty()) {
+ String message =
+ ctgValue.getLabel()
+ + " has aspects attached, but these are not supported in retroactive"
+ + " trimming mode.";
+ env.getListener()
+ .handle(Event.error(TargetUtils.getLocationMaybe(ctgValue.getTarget()), message));
+ throw new ConfiguredTargetFunction.DependencyEvaluationException(
+ new InvalidConfigurationException(message));
+ }
// The dep uses the same exact configuration. Let's re-use the current configuration and
// skip adding a Skyframe dependency edge on it.
putOnlyEntry(
@@ -211,6 +223,16 @@
// uniquely frequent. It's possible, e.g., for every node in the configured target graph
// to incur multiple host transitions. So we aggressively optimize to avoid hurting
// analysis time.
+ if (hostConfiguration.trimConfigurationsRetroactively() && !dep.getAspects().isEmpty()) {
+ String message =
+ ctgValue.getLabel()
+ + " has aspects attached, but these are not supported in retroactive"
+ + " trimming mode.";
+ env.getListener()
+ .handle(Event.error(TargetUtils.getLocationMaybe(ctgValue.getTarget()), message));
+ throw new ConfiguredTargetFunction.DependencyEvaluationException(
+ new InvalidConfigurationException(message));
+ }
putOnlyEntry(
resolvedDeps,
dependencyEdge,
@@ -256,6 +278,17 @@
if (sameFragments
&& toOptions.size() == 1
&& Iterables.getOnlyElement(toOptions).equals(currentConfiguration.getOptions())) {
+ if (ctgValue.getConfiguration().trimConfigurationsRetroactively()
+ && !dep.getAspects().isEmpty()) {
+ String message =
+ ctgValue.getLabel()
+ + " has aspects attached, but these are not supported in retroactive"
+ + " trimming mode.";
+ env.getListener()
+ .handle(Event.error(TargetUtils.getLocationMaybe(ctgValue.getTarget()), message));
+ throw new ConfiguredTargetFunction.DependencyEvaluationException(
+ new InvalidConfigurationException(message));
+ }
putOnlyEntry(
resolvedDeps,
dependencyEdge,
@@ -330,12 +363,25 @@
// null out on missing values from *this specific Skyframe request*.
return null;
}
- BuildConfigurationValue trimmedConfig = (BuildConfigurationValue) valueOrException.get();
+ BuildConfiguration trimmedConfig =
+ ((BuildConfigurationValue) valueOrException.get()).getConfiguration();
for (Map.Entry<DependencyKind, Dependency> info : keysToEntries.get(key)) {
Dependency originalDep = info.getValue();
+ if (trimmedConfig.trimConfigurationsRetroactively()
+ && !originalDep.getAspects().isEmpty()) {
+ String message =
+ ctgValue.getLabel()
+ + " has aspects attached, but these are not supported in retroactive"
+ + " trimming mode.";
+ env.getListener()
+ .handle(Event.error(TargetUtils.getLocationMaybe(ctgValue.getTarget()), message));
+ throw new ConfiguredTargetFunction.DependencyEvaluationException(
+ new InvalidConfigurationException(message));
+ }
DependencyEdge attr = new DependencyEdge(info.getKey(), originalDep.getLabel());
- Dependency resolvedDep = Dependency.withConfigurationAndAspects(originalDep.getLabel(),
- trimmedConfig.getConfiguration(), originalDep.getAspects());
+ Dependency resolvedDep =
+ Dependency.withConfigurationAndAspects(
+ originalDep.getLabel(), trimmedConfig, originalDep.getAspects());
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/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index d278350..c48e125 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
@@ -291,6 +291,9 @@
throw new IllegalStateException("Unexpected exception from BuildConfigurationFunction when "
+ "computing " + key.getAspectConfigurationKey(), e);
}
+ if (aspectConfiguration.trimConfigurationsRetroactively()) {
+ throw new AssertionError("Aspects should NEVER be evaluated in retroactive trimming mode.");
+ }
}
ConfiguredTarget associatedTarget = baseConfiguredTargetValue.getConfiguredTarget();
@@ -320,6 +323,9 @@
configuration =
((BuildConfigurationValue) result.get(associatedTarget.getConfigurationKey()))
.getConfiguration();
+ if (configuration.trimConfigurationsRetroactively()) {
+ throw new AssertionError("Aspects should NEVER be evaluated in retroactive trimming mode.");
+ }
}
try {
associatedConfiguredTargetAndData =
