Move analysis root cause tracking to the ConfiguredTargetFunction.
The main remaining problem with interleaved loading and analysis is error
handling. When interleaving, we don't run a real loading phase anymore, and
loading errors can occur during the analysis phase, and need to be handled
there.
The plan is to have ConfiguredTargetFunction throw a
ConfiguredValueCreationException with a list of all loading root causes, which
requires that we also catch ConfiguredValueCreationException here, which in
turn breaks analysis root cause handling, as that is currently relying on
Skyframe root cause tracking.
Moving analysis root cause handling into CTFunction makes it possible to
subsequently also implement loading root cause handling here. This is also
necessary if we want to have complete root cause handling in the general case:
a target may have any number (and combination) of loading and analysis root
causes at the same time.
For now, we only pass a single analysis root cause, which mirrors the current
Skyframe-based implementation.
--
MOS_MIGRATED_REVID=112930871
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 97b6d36..1d6329a 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
@@ -88,27 +88,16 @@
* Exception class that signals an error during the evaluation of a dependency.
*/
public static class DependencyEvaluationException extends Exception {
- private final SkyKey rootCauseSkyKey;
-
- public DependencyEvaluationException(Exception cause) {
+ public DependencyEvaluationException(InvalidConfigurationException cause) {
super(cause);
- this.rootCauseSkyKey = null;
}
- public DependencyEvaluationException(SkyKey rootCauseSkyKey, Exception cause) {
+ public DependencyEvaluationException(ConfiguredValueCreationException cause) {
super(cause);
- this.rootCauseSkyKey = rootCauseSkyKey;
- }
-
- /**
- * Returns the key of the root cause or null if the problem was with this target.
- */
- public SkyKey getRootCauseSkyKey() {
- return rootCauseSkyKey;
}
@Override
- public Exception getCause() {
+ public synchronized Exception getCause() {
return (Exception) super.getCause();
}
}
@@ -193,10 +182,24 @@
view, env, target, configuration, depValueMap, configConditions, transitivePackages);
return ans;
} catch (DependencyEvaluationException e) {
- throw new ConfiguredTargetFunctionException(e.getRootCauseSkyKey(), e.getCause());
+ if (e.getCause() instanceof ConfiguredValueCreationException) {
+ throw new ConfiguredTargetFunctionException(
+ (ConfiguredValueCreationException) e.getCause());
+ } else {
+ // Cast to InvalidConfigurationException as a consistency check. If you add any
+ // DependencyEvaluationException constructors, you may need to change this code, too.
+ InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause();
+ throw new ConfiguredTargetFunctionException(
+ new ConfiguredValueCreationException(cause.getMessage(), target.getLabel()));
+ }
} catch (AspectCreationException e) {
+ // getAnalysisRootCause may be null if the analysis of the aspect itself failed.
+ Label rootCause = target.getLabel();
+ if (e.getAnalysisRootCause() != null) {
+ rootCause = e.getAnalysisRootCause();
+ }
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(e.getMessage()));
+ new ConfiguredValueCreationException(e.getMessage(), rootCause));
}
}
@@ -234,8 +237,10 @@
depValueNames =
resolver.dependentNodeMap(ctgValue, hostConfiguration, aspect, configConditions);
} 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()));
+ throw new DependencyEvaluationException(
+ new ConfiguredValueCreationException(e.print(), ctgValue.getLabel()));
}
// Trim each dep's configuration so it only includes the fragments needed by its transitive
@@ -683,7 +688,8 @@
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));
+ throw new DependencyEvaluationException(new ConfiguredValueCreationException(
+ message, target.getLabel()));
}
}
@@ -697,13 +703,21 @@
*/
@Nullable
private static Map<SkyKey, ConfiguredTarget> resolveConfiguredTargetDependencies(
- Environment env, Collection<Dependency> deps, NestedSetBuilder<Package> transitivePackages) {
+ Environment env, Collection<Dependency> deps, NestedSetBuilder<Package> transitivePackages)
+ throws DependencyEvaluationException {
boolean ok = !env.valuesMissing();
Iterable<SkyKey> depKeys = Iterables.transform(deps, TO_KEYS);
- Map<SkyKey, SkyValue> depValues = env.getValues(depKeys);
+ Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> depValues =
+ env.getValuesOrThrow(depKeys, ConfiguredValueCreationException.class);
Map<SkyKey, ConfiguredTarget> result = Maps.newHashMapWithExpectedSize(depValues.size());
- for (Map.Entry<SkyKey, SkyValue> entry : depValues.entrySet()) {
- ConfiguredTargetValue depValue = (ConfiguredTargetValue) entry.getValue();
+ for (Map.Entry<SkyKey, ValueOrException<ConfiguredValueCreationException>> entry :
+ depValues.entrySet()) {
+ ConfiguredTargetValue depValue;
+ try {
+ depValue = (ConfiguredTargetValue) entry.getValue().get();
+ } catch (ConfiguredValueCreationException e) {
+ throw new DependencyEvaluationException(e);
+ }
if (depValue == null) {
ok = false;
} else {
@@ -748,7 +762,8 @@
if (events.hasErrors()) {
analysisEnvironment.disable(target);
throw new ConfiguredTargetFunctionException(new ConfiguredValueCreationException(
- "Analysis of target '" + target.getLabel() + "' failed; build aborted"));
+ "Analysis of target '" + target.getLabel() + "' failed; build aborted",
+ target.getLabel()));
}
Preconditions.checkState(!analysisEnvironment.hasErrors(),
"Analysis environment hasError() but no errors reported");
@@ -791,8 +806,16 @@
* a ConfiguredTargetValue.
*/
public static final class ConfiguredValueCreationException extends Exception {
- public ConfiguredValueCreationException(String message) {
+ // TODO(ulfjack): Collect all analysis root causes, not just the first one.
+ private final Label analysisRootCause;
+
+ public ConfiguredValueCreationException(String message, Label currentTarget) {
super(message);
+ this.analysisRootCause = Preconditions.checkNotNull(currentTarget);
+ }
+
+ public Label getAnalysisRootCause() {
+ return analysisRootCause;
}
}
@@ -805,17 +828,12 @@
super(e, Transience.PERSISTENT);
}
- private ConfiguredTargetFunctionException(ConfiguredValueCreationException error) {
- super(error, Transience.PERSISTENT);
+ private ConfiguredTargetFunctionException(ConfiguredValueCreationException e) {
+ super(e, Transience.PERSISTENT);
}
private ConfiguredTargetFunctionException(ActionConflictException e) {
super(e, Transience.PERSISTENT);
}
-
- private ConfiguredTargetFunctionException(
- @Nullable SkyKey childKey, Exception transitiveError) {
- super(transitiveError, childKey);
- }
}
}
