Refactor root cause reporting in ConfiguredTargetFunction
We now track Causes instead of plain Labels, which will allow us to do better reporting in the future. Add basic tests.
PiperOrigin-RevId: 198380468
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 748451b..a310fad 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
@@ -41,11 +41,16 @@
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.buildeventstream.BuildEventId;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
+import com.google.devtools.build.lib.causes.AnalysisFailedCause;
+import com.google.devtools.build.lib.causes.Cause;
+import com.google.devtools.build.lib.causes.LoadingFailedCause;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.Attribute;
@@ -78,6 +83,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Semaphore;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
/**
@@ -161,7 +167,7 @@
SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
- NestedSetBuilder<Label> transitiveLoadingRootCauses = NestedSetBuilder.stableOrder();
+ NestedSetBuilder<Cause> transitiveRootCauses = NestedSetBuilder.stableOrder();
ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key.argument();
Label label = configuredTargetKey.getLabel();
@@ -197,7 +203,8 @@
new ConfiguredValueCreationException(e.getMessage(), label, configuration));
}
if (pkg.containsErrors()) {
- transitiveLoadingRootCauses.add(label);
+ transitiveRootCauses.add(
+ new LoadingFailedCause(label, new NoSuchTargetException(target).getMessage()));
}
if (transitivePackagesForPackageRootResolution != null) {
transitivePackagesForPackageRootResolution.add(pkg);
@@ -241,7 +248,7 @@
resolver,
ctgValue,
transitivePackagesForPackageRootResolution,
- transitiveLoadingRootCauses,
+ transitiveRootCauses,
((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory());
if (env.valuesMissing()) {
return null;
@@ -253,9 +260,10 @@
// more root causes during computeDependencies.
// Note that this doesn't apply to AspectFunction, because aspects can't have configurable
// attributes.
- if (!transitiveLoadingRootCauses.isEmpty() && configConditions != NO_CONFIG_CONDITIONS) {
+ if (!transitiveRootCauses.isEmpty() && configConditions != NO_CONFIG_CONDITIONS) {
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(transitiveLoadingRootCauses.build()));
+ new ConfiguredValueCreationException(
+ "Cannot compute config conditions", configuration, transitiveRootCauses.build()));
}
// Determine what toolchains are needed by this target.
@@ -288,14 +296,15 @@
ruleClassProvider,
view.getHostConfiguration(configuration),
transitivePackagesForPackageRootResolution,
- transitiveLoadingRootCauses,
+ transitiveRootCauses,
defaultBuildOptions);
if (env.valuesMissing()) {
return null;
}
- if (!transitiveLoadingRootCauses.isEmpty()) {
+ if (!transitiveRootCauses.isEmpty()) {
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(transitiveLoadingRootCauses.build()));
+ new ConfiguredValueCreationException(
+ "Analysis failed", configuration, transitiveRootCauses.build()));
}
Preconditions.checkNotNull(depValueMap);
ConfiguredTargetValue ans =
@@ -315,21 +324,15 @@
// Check if this is caused by an unresolved toolchain, and report it as such.
if (toolchainContext != null) {
- ImmutableSet.Builder<Label> causes = new ImmutableSet.Builder<Label>();
- if (cvce.getAnalysisRootCause() != null) {
- causes.add(cvce.getAnalysisRootCause());
- }
- if (!cvce.getRootCauses().isEmpty()) {
- causes.addAll(cvce.getRootCauses());
- }
Set<Label> toolchainDependencyErrors =
- toolchainContext.filterToolchainLabels(causes.build());
+ toolchainContext.filterToolchainLabels(
+ Iterables.transform(cvce.getRootCauses(), Cause::getLabel));
if (!toolchainDependencyErrors.isEmpty()) {
env.getListener()
.handle(
Event.error(
String.format(
- "While resolving toolchains for target %s: %s",
+ "XXX - While resolving toolchains for target %s: %s",
target.getLabel(), e.getCause().getMessage())));
}
}
@@ -352,13 +355,11 @@
new ConfiguredValueCreationException(e.getMessage(), target.getLabel(), configuration));
}
} catch (AspectCreationException e) {
- // getAnalysisRootCause may be null if the analysis of the aspect itself failed.
- Label analysisRootCause = target.getLabel();
- if (e.getAnalysisRootCause() != null) {
- analysisRootCause = e.getAnalysisRootCause();
- }
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(e.getMessage(), analysisRootCause, configuration));
+ new ConfiguredValueCreationException(
+ e.getMessage(),
+ configuration,
+ e.getCauses()));
} catch (ToolchainContextException e) {
// We need to throw a ConfiguredValueCreationException, so either find one or make one.
ConfiguredValueCreationException cvce;
@@ -415,7 +416,7 @@
RuleClassProvider ruleClassProvider,
BuildConfiguration hostConfiguration,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
- NestedSetBuilder<Label> transitiveLoadingRootCauses,
+ NestedSetBuilder<Cause> transitiveRootCauses,
BuildOptions defaultBuildOptions)
throws DependencyEvaluationException, ConfiguredTargetFunctionException,
AspectCreationException, InterruptedException {
@@ -431,14 +432,15 @@
toolchainContext == null
? ImmutableSet.of()
: toolchainContext.getResolvedToolchainLabels(),
- transitiveLoadingRootCauses,
+ transitiveRootCauses,
defaultBuildOptions,
((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory());
} catch (EvalException e) {
// EvalException can only be thrown by computed Skylark attributes in the current rule.
env.getListener().handle(Event.error(e.getLocation(), e.getMessage()));
throw new DependencyEvaluationException(
- new ConfiguredValueCreationException(e.print(), ctgValue.getLabel()));
+ new ConfiguredValueCreationException(
+ e.print(), ctgValue.getLabel(), ctgValue.getConfiguration()));
} catch (InvalidConfigurationException e) {
throw new DependencyEvaluationException(e);
} catch (InconsistentAspectOrderException e) {
@@ -469,9 +471,10 @@
Map<SkyKey, ConfiguredTargetAndData> depValues =
resolveConfiguredTargetDependencies(
env,
+ ctgValue,
depValueNames.values(),
transitivePackagesForPackageRootResolution,
- transitiveLoadingRootCauses);
+ transitiveRootCauses);
if (depValues == null) {
return null;
}
@@ -492,7 +495,8 @@
Event.error(ctgValue.getTarget().getLocation(), e.getMessage()));
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(e.getMessage(), ctgValue.getLabel()));
+ new ConfiguredValueCreationException(
+ e.getMessage(), ctgValue.getLabel(), ctgValue.getConfiguration()));
}
}
@@ -510,7 +514,7 @@
SkyframeDependencyResolver resolver,
TargetAndConfiguration ctgValue,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
- NestedSetBuilder<Label> transitiveLoadingRootCauses,
+ NestedSetBuilder<Cause> transitiveRootCauses,
@Nullable RuleTransitionFactory trimmingTransitionFactory)
throws DependencyEvaluationException, InterruptedException {
if (!(target instanceof Rule)) {
@@ -538,7 +542,7 @@
Collection<Dependency> configValueNames;
try {
configValueNames = resolver.resolveRuleLabels(
- ctgValue, configLabelMap, transitiveLoadingRootCauses, trimmingTransitionFactory);
+ ctgValue, configLabelMap, transitiveRootCauses, trimmingTransitionFactory);
} catch (InconsistentAspectOrderException e) {
throw new DependencyEvaluationException(e);
}
@@ -560,9 +564,10 @@
Map<SkyKey, ConfiguredTargetAndData> configValues =
resolveConfiguredTargetDependencies(
env,
+ ctgValue,
configValueNames,
transitivePackagesForPackageRootResolution,
- transitiveLoadingRootCauses);
+ transitiveRootCauses);
if (configValues == null) {
return null;
}
@@ -580,8 +585,9 @@
String message =
entry.getLabel() + " is not a valid configuration key for " + target.getLabel();
env.getListener().handle(Event.error(TargetUtils.getLocationMaybe(target), message));
- throw new DependencyEvaluationException(new ConfiguredValueCreationException(
- message, target.getLabel()));
+ throw new DependencyEvaluationException(
+ new ConfiguredValueCreationException(
+ message, ctgValue.getLabel(), ctgValue.getConfiguration()));
}
}
@@ -597,12 +603,13 @@
@Nullable
private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDependencies(
Environment env,
+ TargetAndConfiguration ctgValue,
Collection<Dependency> deps,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
- NestedSetBuilder<Label> transitiveLoadingRootCauses)
+ NestedSetBuilder<Cause> transitiveRootCauses)
throws DependencyEvaluationException, InterruptedException {
boolean missedValues = env.valuesMissing();
- boolean failed = false;
+ String failWithMessage = null;
// Naively we would like to just fetch all requested ConfiguredTargets, together with their
// Packages. However, some ConfiguredTargets are AliasConfiguredTargets, which means that their
// associated Targets (and therefore associated Packages) don't correspond to their own Labels.
@@ -677,12 +684,8 @@
}
}
} catch (ConfiguredValueCreationException e) {
- // TODO(ulfjack): If there is an analysis root cause, we drop all loading root causes.
- if (e.getAnalysisRootCause() != null) {
- throw new DependencyEvaluationException(e);
- }
- transitiveLoadingRootCauses.addTransitive(e.loadingRootCauses);
- failed = true;
+ transitiveRootCauses.addTransitive(e.rootCauses);
+ failWithMessage = e.getMessage();
}
}
if (aliasDepsToRedo.isEmpty()) {
@@ -693,9 +696,10 @@
}
if (missedValues) {
return null;
- } else if (failed) {
+ } else if (failWithMessage != null) {
throw new DependencyEvaluationException(
- new ConfiguredValueCreationException(transitiveLoadingRootCauses.build()));
+ new ConfiguredValueCreationException(
+ failWithMessage, ctgValue.getConfiguration(), transitiveRootCauses.build()));
} else {
return result;
}
@@ -753,11 +757,21 @@
events.replayOn(env.getListener());
if (events.hasErrors()) {
analysisEnvironment.disable(target);
+ NestedSet<Cause> rootCauses = NestedSetBuilder.wrap(
+ Order.STABLE_ORDER,
+ events.getEvents().stream()
+ .filter((event) -> event.getKind() == EventKind.ERROR)
+ .map((event) ->
+ new AnalysisFailedCause(
+ target.getLabel(),
+ ConfiguredValueCreationException.toId(configuration),
+ event.getMessage()))
+ .collect(Collectors.toList()));
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
"Analysis of target '" + target.getLabel() + "' failed; build aborted",
- target.getLabel(),
- configuration));
+ configuration,
+ rootCauses));
}
Preconditions.checkState(!analysisEnvironment.hasErrors(),
"Analysis environment hasError() but no errors reported");
@@ -804,51 +818,41 @@
*/
@AutoCodec
public static final class ConfiguredValueCreationException extends Exception {
- private final NestedSet<Label> loadingRootCauses;
- // TODO(ulfjack): Collect all analysis root causes, not just the first one.
- @Nullable private final Label analysisRootCause;
- @Nullable private final BuildEventId configuration;
-
- private ConfiguredValueCreationException(
- String message, Label currentTarget, BuildConfiguration configuration) {
- this(
- message,
- Preconditions.checkNotNull(currentTarget),
- NestedSetBuilder.<Label>emptySet(Order.STABLE_ORDER),
- configuration == null ? null : configuration.getEventId());
+ private static ConfigurationId toId(BuildConfiguration config) {
+ return config == null ? null : config.getEventId().asStreamProto().getConfiguration();
}
+ @Nullable private final BuildEventId configuration;
+ private final NestedSet<Cause> rootCauses;
+
@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
ConfiguredValueCreationException(
String message,
- Label analysisRootCause,
- NestedSet<Label> loadingRootCauses,
- BuildEventId configuration) {
+ @Nullable BuildEventId configuration,
+ NestedSet<Cause> rootCauses) {
super(message);
- this.loadingRootCauses = loadingRootCauses;
- this.analysisRootCause = analysisRootCause;
+ this.rootCauses = rootCauses;
this.configuration = configuration;
}
- private ConfiguredValueCreationException(String message, Label currentTarget) {
- this(message, currentTarget, /*configuration=*/ null);
+ private ConfiguredValueCreationException(
+ String message, Label currentTarget, @Nullable BuildConfiguration configuration) {
+ this(
+ message,
+ configuration == null ? null : configuration.getEventId(),
+ NestedSetBuilder.<Cause>stableOrder()
+ .add(new AnalysisFailedCause(currentTarget, toId(configuration), message))
+ .build());
}
- private ConfiguredValueCreationException(String message, NestedSet<Label> rootCauses) {
- this(message, /*analysisRootCause=*/ null, rootCauses, /*configuration=*/ null);
+ private ConfiguredValueCreationException(
+ String message, @Nullable BuildConfiguration configuration, NestedSet<Cause> rootCauses) {
+ this(message, configuration == null ? null : configuration.getEventId(), rootCauses);
}
- private ConfiguredValueCreationException(NestedSet<Label> rootCauses) {
- this("Loading failed", rootCauses);
- }
-
- public NestedSet<Label> getRootCauses() {
- return loadingRootCauses;
- }
-
- @Nullable public Label getAnalysisRootCause() {
- return analysisRootCause;
+ public NestedSet<Cause> getRootCauses() {
+ return rootCauses;
}
@Nullable public BuildEventId getConfiguration() {
