Encode ViewCreation failures with FailureDetails
Bazel's analysis phase can fail with a ViewCreationFailedException,
encompassing a variety of loading, analysis, and post-analysis evaluation
problems. This change equips that exception with FailureDetails and
eliminates the final use of (non-success) DetailedExitCode without
FailureDetails.
Before this change, a ViewCreationFailedException (VCFE) always led to a
numerical exit code of 1. To ensure backwards compatibility, this change
uses the DetailedExitCode factory method which overrides the numerical
exit code metadata from failure_detail.proto. Most of the detailed failure
subcategories that can end up in a VCFE do have metadata specifying 1 as
their numerical exit code, but not all; e.g., post-analysis query
evaluation failures can cause a VCFE and, as query failures, have 2 or 7
for their metadata.
Ensuring the propagation of failure details from analysis to the VCFE
required the following work.
SkyframeBuildView translated analysis exceptions, designated via the
marker interface SaneAnalysisException, into VCFEs. Likewise, it
translated the loading-phase exceptions NoSuch{Package,Target}Exception
into VCFEs. By making SaneAnalysisException extend DetailedException,
that conversion now propagates any detailed failures described by those
three exception types.
SaneAnalysisException has six implementations. This change implements
DetailedException across them. Doing so with AspectCreationException and
ConfiguredValueCreationException involved moderately complex work. In
AspectFunction and ConfiguredTargetFunction, these two exception types
get instantiated when a nested set of "Cause"s is non-empty (and also in
several other more straightforward contexts).
"Cause" had already been equipped with a DetailedExitCode property, but
AnalysisFailedCause didn't support it (Cause's other implementations
did). This change implements the property for AnalysisFailedCause.
The nested set traversal in
ConfiguredTargetFunction.getPrioritizedDetailedExitCode does not involve
any deep traversals, though that's not obvious. If the nested set
builder ever gets another nested set added to it via addTransitive, then
it does so while handling a dependency's exception, which results in a
DependencyEvaluationException, whose handling avoids the call to
getPrioritizedDetailedExitCode.
This change does *not* detail analysis failures in maximum resolution. To
do so, a future change would need to enhance the error-event-sensitive
failure detection in AspectFunction and ConfiguredTargetFunction, using
e.g. the events-with-properties strategy used by
ErrorSensingEventHandler. It would also need to replace the non-detailed
AspectCreationException and ConfiguredValueCreationException
constructors.
RELNOTES: None.
PiperOrigin-RevId: 333827282
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactPrefixConflictException.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactPrefixConflictException.java
index 8ecd13e..d6fb317 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactPrefixConflictException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactPrefixConflictException.java
@@ -14,6 +14,11 @@
package com.google.devtools.build.lib.actions;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.DetailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.PathFragment;
/**
@@ -21,7 +26,7 @@
* output of another action. Since the first path cannot be both a directory and a file, this would
* lead to an error if both actions were executed in the same build.
*/
-public class ArtifactPrefixConflictException extends Exception {
+public class ArtifactPrefixConflictException extends Exception implements DetailedException {
public ArtifactPrefixConflictException(
PathFragment firstPath, PathFragment secondPath, Label firstOwner, Label secondOwner) {
super(
@@ -31,4 +36,13 @@
+ "files or build just one of them",
firstPath, firstOwner, secondPath, secondOwner));
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(getMessage())
+ .setAnalysis(Analysis.newBuilder().setCode(Code.ARTIFACT_PREFIX_CONFLICT))
+ .build());
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index 6ec3d88..a645223 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -172,16 +172,19 @@
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
"//src/main/java/com/google/devtools/build/lib/util",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/protobuf:protobuf_java",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
index 3d2d83f..ba3182c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
@@ -24,7 +24,11 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.SaneAnalysisException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.util.Set;
/**
@@ -100,6 +104,15 @@
eventListener.handle(Event.error(msg));
}
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(getMessage())
+ .setAnalysis(Analysis.newBuilder().setCode(Code.ACTION_CONFLICT))
+ .build());
+ }
+
private static void addStringDetail(
StringBuilder sb, String key, String valueA, String valueB) {
valueA = valueA != null ? valueA : "(null)";
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 15c1d3f..28b8dc2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1152,6 +1152,10 @@
java_library(
name = "view_creation_failed_exception",
srcs = ["ViewCreationFailedException.java"],
+ deps = [
+ "//src/main/protobuf:failure_details_java_proto",
+ "//third_party:guava",
+ ],
)
java_library(
@@ -1897,6 +1901,9 @@
":constraints/supported_environments_provider",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:jsr305",
],
)
@@ -1989,6 +1996,7 @@
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
],
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 74f0b84..7dd75d0 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
@@ -333,16 +333,21 @@
Label.parseAbsolute(
bzlFileLoadLikeString, /* repositoryMapping= */ ImmutableMap.of());
} catch (LabelSyntaxException e) {
+ String errorMessage = String.format("Invalid aspect '%s': %s", aspect, e.getMessage());
throw new ViewCreationFailedException(
- String.format("Invalid aspect '%s': %s", aspect, e.getMessage()), e);
+ errorMessage,
+ createFailureDetail(errorMessage, Analysis.Code.ASPECT_LABEL_SYNTAX_ERROR),
+ e);
}
String starlarkFunctionName = aspect.substring(delimiterPosition + 1);
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
if (targetSpec.getConfiguration() != null
&& targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
+ String errorMessage =
+ "Aspects were requested, but are not supported in retroactive trimming mode.";
throw new ViewCreationFailedException(
- "Aspects were requested, but are not supported in retroactive trimming mode.");
+ errorMessage, createFailureDetail(errorMessage, Analysis.Code.ASPECT_PREREQ_UNMET));
}
aspectConfigurations.put(
Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration());
@@ -364,8 +369,11 @@
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
if (targetSpec.getConfiguration() != null
&& targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
+ String errorMessage =
+ "Aspects were requested, but are not supported in retroactive trimming mode.";
throw new ViewCreationFailedException(
- "Aspects were requested, but are not supported in retroactive trimming mode.");
+ errorMessage,
+ createFailureDetail(errorMessage, Analysis.Code.ASPECT_PREREQ_UNMET));
}
// 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.
@@ -379,7 +387,9 @@
configuration));
}
} else {
- throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
+ String errorMessage = "Aspect '" + aspect + "' is unknown";
+ throw new ViewCreationFailedException(
+ errorMessage, createFailureDetail(errorMessage, Analysis.Code.ASPECT_NOT_FOUND));
}
}
}
@@ -617,6 +627,13 @@
return null;
}
+ private static FailureDetail createFailureDetail(String errorMessage, Analysis.Code code) {
+ return FailureDetail.newBuilder()
+ .setMessage(errorMessage)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .build();
+ }
+
private static NestedSet<Artifact> getBaselineCoverageArtifacts(
Collection<ConfiguredTarget> configuredTargets,
ArtifactsToOwnerLabels.Builder topLevelArtifactsToOwnerLabels) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index 5d80db2..61d6120 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -213,7 +213,7 @@
* representing the given target and configuration.
*
* <p>Otherwise {@code aspects} represents an aspect path. The function returns dependent nodes of
- * the entire path applied to given target and configuration. These are the depenent nodes of the
+ * the entire path applied to given target and configuration. These are the dependent nodes of the
* last aspect in the path.
*
* <p>This also implements the first step of applying configuration transitions, namely, split
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 47f687c..b4cb8dc 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
@@ -89,8 +89,12 @@
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.LabelClass;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.SaneAnalysisException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -1333,6 +1337,15 @@
InvalidExecGroupException(String message) {
super(message);
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(getMessage())
+ .setAnalysis(Analysis.newBuilder().setCode(Code.EXEC_GROUP_MISSING))
+ .build());
+ }
}
@VisibleForTesting
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ViewCreationFailedException.java b/src/main/java/com/google/devtools/build/lib/analysis/ViewCreationFailedException.java
index cb02316..17707e8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ViewCreationFailedException.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ViewCreationFailedException.java
@@ -13,23 +13,35 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+
/**
- * An exception indicating that there was a problem during the view
- * construction (loading and analysis phases) for one or more targets, that the
- * configured target graph could not be successfully constructed, and that
- * a build cannot be started.
+ * An exception indicating that there was a problem during the view construction (loading and
+ * analysis phases) for one or more targets, that the configured target graph could not be
+ * successfully constructed, and that a build cannot be started.
*/
public class ViewCreationFailedException extends Exception {
- public ViewCreationFailedException(String message) {
+ private final FailureDetail failureDetail;
+
+ public ViewCreationFailedException(String message, FailureDetail failureDetail) {
super(message);
+ this.failureDetail = checkNotNull(failureDetail);
}
- public ViewCreationFailedException(String message, Throwable cause) {
+ public ViewCreationFailedException(String message, FailureDetail failureDetail, Throwable cause) {
super(message + ": " + cause.getMessage(), cause);
+ this.failureDetail = checkNotNull(failureDetail);
}
- public ViewCreationFailedException(Throwable cause) {
+ public ViewCreationFailedException(FailureDetail failureDetail, Throwable cause) {
super(cause);
+ this.failureDetail = checkNotNull(failureDetail);
+ }
+
+ public FailureDetail getFailureDetail() {
+ return failureDetail;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
index bd7be99..3d8696e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
@@ -18,6 +18,11 @@
import com.google.devtools.build.lib.packages.EnvironmentGroup;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.DetailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.util.Map;
import javax.annotation.Nullable;
@@ -129,16 +134,18 @@
static EnvironmentGroup getEnvironmentGroup(Target envTarget) throws EnvironmentLookupException {
if (!(envTarget instanceof Rule)
|| !((Rule) envTarget).getRuleClass().equals(ConstraintConstants.ENVIRONMENT_RULE)) {
- throw new EnvironmentLookupException(
- envTarget.getLabel() + " is not a valid environment definition");
+ throw createEnvironmentLookupException(
+ envTarget.getLabel() + " is not a valid environment definition",
+ Code.INVALID_ENVIRONMENT);
}
for (EnvironmentGroup group : envTarget.getPackage().getTargets(EnvironmentGroup.class)) {
if (group.getEnvironments().contains(envTarget.getLabel())) {
return group;
}
}
- throw new EnvironmentLookupException(
- "cannot find the group for environment " + envTarget.getLabel());
+ throw createEnvironmentLookupException(
+ "cannot find the group for environment " + envTarget.getLabel(),
+ Code.ENVIRONMENT_MISSING_FROM_GROUPS);
}
/**
@@ -187,10 +194,31 @@
EnvironmentCollection.Builder refinedEnvironments,
Map<Label, RemovedEnvironmentCulprit> removedEnvironmentCulprits);
+ /**
+ * Returns an {@link EnvironmentLookupException} with the specified message and detailed failure
+ * code.
+ */
+ static EnvironmentLookupException createEnvironmentLookupException(String message, Code code) {
+ return new EnvironmentLookupException(
+ DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .build()));
+ }
+
/** Exception indicating errors finding/parsing environments or their containing groups. */
- class EnvironmentLookupException extends Exception {
- private EnvironmentLookupException(String message) {
- super(message);
+ class EnvironmentLookupException extends Exception implements DetailedException {
+ private final DetailedExitCode detailedExitCode;
+
+ private EnvironmentLookupException(DetailedExitCode detailedExitCode) {
+ super(detailedExitCode.getFailureDetail().getMessage());
+ this.detailedExitCode = detailedExitCode;
+ }
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
index bbeffe7..f8ea49f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
@@ -34,6 +34,9 @@
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.PackageManager;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue.Key;
import java.util.Collection;
import java.util.List;
@@ -141,24 +144,28 @@
// Check auto-detected CPU environments.
try {
- if (!getMissingEnvironments(topLevelTarget,
- autoConfigureTargetEnvironments(config, config.getAutoCpuEnvironmentGroup()))
+ if (!getMissingEnvironments(
+ topLevelTarget,
+ autoConfigureTargetEnvironments(config, config.getAutoCpuEnvironmentGroup()))
.isEmpty()) {
badTargets.add(topLevelTarget);
}
- } catch (NoSuchPackageException
- | NoSuchTargetException e) {
- throw new ViewCreationFailedException("invalid target environment", e);
+ } catch (NoSuchPackageException | NoSuchTargetException e) {
+ throw new ViewCreationFailedException(
+ "invalid target environment", e.getDetailedExitCode().getFailureDetail(), e);
}
}
if (!exceptionInducingTargets.isEmpty()) {
- throw new ViewCreationFailedException(getBadTargetsUserMessage(exceptionInducingTargets));
+ String badTargetsUserMessage = getBadTargetsUserMessage(exceptionInducingTargets);
+ throw new ViewCreationFailedException(
+ badTargetsUserMessage,
+ FailureDetail.newBuilder()
+ .setMessage(badTargetsUserMessage)
+ .setAnalysis(Analysis.newBuilder().setCode(Code.TARGETS_MISSING_ENVIRONMENTS))
+ .build());
}
- return ImmutableSet.copyOf(
- badTargets
- .addAll(exceptionInducingTargets.keySet())
- .build());
+ return ImmutableSet.copyOf(badTargets.addAll(exceptionInducingTargets.keySet()).build());
}
/**
@@ -208,9 +215,11 @@
Target env = packageManager.getTarget(eventHandler, envLabel);
expectedEnvironmentsBuilder.put(
ConstraintSemantics.getEnvironmentGroup(env).getEnvironmentLabels(), envLabel);
- } catch (NoSuchPackageException | NoSuchTargetException
+ } catch (NoSuchPackageException
+ | NoSuchTargetException
| ConstraintSemantics.EnvironmentLookupException e) {
- throw new ViewCreationFailedException("invalid target environment", e);
+ throw new ViewCreationFailedException(
+ "invalid target environment", e.getDetailedExitCode().getFailureDetail(), e);
}
}
EnvironmentCollection expectedEnvironments = expectedEnvironmentsBuilder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 9cc3efc..d418563 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -430,14 +430,11 @@
reportExceptionError(environmentPendingAbruptExitException);
result.setCatastrophe();
}
- } catch (TargetParsingException e) {
- detailedExitCode = e.getDetailedExitCode();
- reportExceptionError(e);
- } catch (LoadingFailedException e) {
+ } catch (TargetParsingException | LoadingFailedException e) {
detailedExitCode = e.getDetailedExitCode();
reportExceptionError(e);
} catch (ViewCreationFailedException e) {
- detailedExitCode = DetailedExitCode.justExitCode(ExitCode.PARSING_FAILURE);
+ detailedExitCode = DetailedExitCode.of(ExitCode.PARSING_FAILURE, e.getFailureDetail());
reportExceptionError(e);
} catch (ExitException e) {
detailedExitCode = e.getDetailedExitCode();
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java
index 4562c97..74d688a 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java
@@ -77,11 +77,23 @@
env.getSkyframeExecutor().getTransitiveConfigurationKeys(),
queryRuntimeHelper,
queryExpression);
- } catch (QueryException | IOException e) {
+ } catch (QueryException e) {
+ String errorMessage = "Error doing post analysis query";
if (!request.getKeepGoing()) {
- throw new ViewCreationFailedException("Error doing post analysis query", e);
+ throw new ViewCreationFailedException(errorMessage, e.getFailureDetail(), e);
}
- env.getReporter().error(null, "Error doing post analysis query", e);
+ env.getReporter().error(null, errorMessage, e);
+ } catch (IOException e) {
+ String errorMessage = "I/O error doing post analysis query";
+ if (!request.getKeepGoing()) {
+ FailureDetail failureDetail =
+ FailureDetail.newBuilder()
+ .setMessage(errorMessage + ": " + e.getMessage())
+ .setQuery(Query.newBuilder().setCode(Code.OUTPUT_FORMATTER_IO_EXCEPTION))
+ .build();
+ throw new ViewCreationFailedException(errorMessage, failureDetail, e);
+ }
+ env.getReporter().error(null, errorMessage, e);
} catch (QueryRuntimeHelperException e) {
throw new ExitException(DetailedExitCode.of(e.getFailureDetail()));
}
diff --git a/src/main/java/com/google/devtools/build/lib/causes/AnalysisFailedCause.java b/src/main/java/com/google/devtools/build/lib/causes/AnalysisFailedCause.java
index 21b6869..ed6ab45 100644
--- a/src/main/java/com/google/devtools/build/lib/causes/AnalysisFailedCause.java
+++ b/src/main/java/com/google/devtools/build/lib/causes/AnalysisFailedCause.java
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.util.Objects;
import javax.annotation.Nullable;
@@ -28,20 +29,26 @@
public class AnalysisFailedCause implements Cause {
private final Label label;
@Nullable private final ConfigurationId configuration;
- private final String msg;
+ private final DetailedExitCode detailedExitCode;
- public AnalysisFailedCause(Label label, @Nullable ConfigurationId configuration, String msg) {
+ public AnalysisFailedCause(
+ Label label, @Nullable ConfigurationId configuration, DetailedExitCode detailedExitCode) {
this.label = label;
this.configuration = configuration;
- this.msg = msg;
+ this.detailedExitCode = detailedExitCode;
}
@Override
public String toString() {
+ // TODO(mschaller): Tests expect non-escaped message strings, and protobuf (the FailureDetail in
+ // detailedExitCode) escapes them. Better versions of tests would check structured data, and
+ // doing that requires unwinding test infrastructure. Note the "inTest" blocks in
+ // SkyframeBuildView#processErrors.
return MoreObjects.toStringHelper(this)
.add("label", label)
.add("configuration", configuration)
- .add("msg", msg)
+ .add("detailedExitCode", detailedExitCode)
+ .add("msg", detailedExitCode.getFailureDetail().getMessage())
.toString();
}
@@ -71,6 +78,11 @@
}
@Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+
+ @Override
public boolean equals(Object o) {
if (this == o) {
return true;
@@ -80,11 +92,11 @@
AnalysisFailedCause a = (AnalysisFailedCause) o;
return Objects.equals(label, a.label)
&& Objects.equals(configuration, a.configuration)
- && Objects.equals(msg, a.msg);
+ && Objects.equals(detailedExitCode, a.detailedExitCode);
}
@Override
public int hashCode() {
- return Objects.hash(label, configuration, msg);
+ return Objects.hash(label, configuration, detailedExitCode);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/causes/Cause.java b/src/main/java/com/google/devtools/build/lib/causes/Cause.java
index 96b2dc3..c058852 100644
--- a/src/main/java/com/google/devtools/build/lib/causes/Cause.java
+++ b/src/main/java/com/google/devtools/build/lib/causes/Cause.java
@@ -16,11 +16,8 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.util.DetailedExitCode;
-import javax.annotation.Nullable;
-/**
- * Interface for classes identifying root causes for a target to fail to build.
- */
+/** Interface for classes identifying root causes for a target to fail to build. */
public interface Cause {
/** Return the label associated with the failure. */
@@ -29,8 +26,6 @@
/** Return the event id for the cause in the format of the build event protocol. */
BuildEventStreamProtos.BuildEventId getIdProto();
- @Nullable // TODO(janakr): override for all callers and make never null.
- default DetailedExitCode getDetailedExitCode() {
- return null;
- }
+ /** Return details describing the failure. */
+ DetailedExitCode getDetailedExitCode();
}
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetParsingException.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetParsingException.java
index a415831..1b592b6 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetParsingException.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetParsingException.java
@@ -34,11 +34,6 @@
this.detailedExitCode = DetailedExitCode.of(createFailureDetail(message, code));
}
- public TargetParsingException(String message, Throwable cause, FailureDetail failureDetail) {
- super(Preconditions.checkNotNull(message), cause);
- this.detailedExitCode = DetailedExitCode.of(Preconditions.checkNotNull(failureDetail));
- }
-
public TargetParsingException(
String message, Throwable cause, DetailedExitCode detailedExitCode) {
super(Preconditions.checkNotNull(message), cause);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
index 2fcc40b..6b9f0b2 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
@@ -119,20 +119,19 @@
/**
* Evaluate the specified query expression in this environment, streaming results to the given
- * {@code callback}. {@code callback.start()} will be called before query evaluation and
- * {@code callback.close()} will be unconditionally called at the end of query evaluation
- * (i.e. regardless of whether it was successful).
+ * {@code callback}. {@code callback.start()} will be called before query evaluation and {@code
+ * callback.close()} will be unconditionally called at the end of query evaluation (i.e.
+ * regardless of whether it was successful).
*
* @return a {@link QueryEvalResult} object that contains the resulting set of targets and a bit
- * to indicate whether errors occurred during evaluation; note that the
- * success status can only be false if {@code --keep_going} was in effect
- * @throws QueryException if the evaluation failed and {@code --nokeep_going} was in
- * effect
+ * to indicate whether errors occurred during evaluation; note that the success status can
+ * only be false if {@code --keep_going} was in effect
+ * @throws QueryException if the evaluation failed and {@code --nokeep_going} was in effect
+ * @throws IOException for output formatter failures from {@code callback}
*/
public QueryEvalResult evaluateQuery(
- QueryExpression expr,
- ThreadSafeOutputFormatterCallback<T> callback)
- throws QueryException, InterruptedException, IOException {
+ QueryExpression expr, ThreadSafeOutputFormatterCallback<T> callback)
+ throws QueryException, InterruptedException, IOException {
EmptinessSensingCallback<T> emptySensingCallback = new EmptinessSensingCallback<>(callback);
long startTime = System.currentTimeMillis();
// In the --nokeep_going case, errors are reported in the order in which the patterns are
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCreationException.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCreationException.java
index c792d4f..89897d6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCreationException.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCreationException.java
@@ -17,9 +17,14 @@
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.LabelCause;
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.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import javax.annotation.Nullable;
/** An exception indicating that there was a problem creating an aspect. */
@@ -29,30 +34,70 @@
}
private final NestedSet<Cause> causes;
+ // TODO(b/138456686): if warranted by a need for finer-grained details, replace the constructors
+ // that specify the general Code.ASPECT_CREATION_FAILED
+ private final DetailedExitCode detailedExitCode;
- public AspectCreationException(String message, NestedSet<Cause> causes) {
+ public AspectCreationException(
+ String message, NestedSet<Cause> causes, DetailedExitCode detailedExitCode) {
super(message);
this.causes = causes;
+ this.detailedExitCode = detailedExitCode;
+ }
+
+ public AspectCreationException(
+ String message,
+ Label currentTarget,
+ @Nullable BuildConfiguration configuration,
+ DetailedExitCode detailedExitCode) {
+ this(
+ message,
+ NestedSetBuilder.<Cause>stableOrder()
+ .add(new AnalysisFailedCause(currentTarget, toId(configuration), detailedExitCode))
+ .build(),
+ detailedExitCode);
}
public AspectCreationException(
String message, Label currentTarget, @Nullable BuildConfiguration configuration) {
this(
message,
- NestedSetBuilder.<Cause>stableOrder()
- .add(new AnalysisFailedCause(currentTarget, toId(configuration), message))
- .build());
+ currentTarget,
+ configuration,
+ createDetailedExitCode(message, Code.ASPECT_CREATION_FAILED));
+ }
+
+ public AspectCreationException(
+ String message, Label currentTarget, DetailedExitCode detailedExitCode) {
+ this(message, currentTarget, null, detailedExitCode);
}
public AspectCreationException(String message, Label currentTarget) {
- this(message, currentTarget, null);
+ this(
+ message, currentTarget, null, createDetailedExitCode(message, Code.ASPECT_CREATION_FAILED));
}
- public AspectCreationException(String message, Cause cause) {
- this(message, NestedSetBuilder.<Cause>stableOrder().add(cause).build());
+ public AspectCreationException(String message, LabelCause cause) {
+ this(
+ message,
+ NestedSetBuilder.<Cause>stableOrder().add(cause).build(),
+ cause.getDetailedExitCode());
}
public NestedSet<Cause> getCauses() {
return causes;
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+
+ private static DetailedExitCode createDetailedExitCode(String message, Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .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 83d20e0..c0d460a 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
@@ -67,7 +67,6 @@
import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException;
-import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredTargetFunctionException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -179,7 +178,10 @@
"%s from %s is not an aspect", starlarkValueName, extensionLabel.toString()));
}
return (StarlarkAspect) starlarkValue;
- } catch (BzlLoadFailedException | ConversionException e) {
+ } catch (BzlLoadFailedException e) {
+ env.getListener().handle(Event.error(e.getMessage()));
+ throw new AspectCreationException(e.getMessage(), extensionLabel, e.getDetailedExitCode());
+ } catch (ConversionException e) {
env.getListener().handle(Event.error(e.getMessage()));
throw new AspectCreationException(e.getMessage(), extensionLabel);
}
@@ -253,7 +255,7 @@
(ConfiguredTargetValue) baseAndAspectValues.get(key.getBaseConfiguredTargetKey()).get();
} catch (ConfiguredValueCreationException e) {
throw new AspectFunctionException(
- new AspectCreationException(e.getMessage(), e.getRootCauses()));
+ new AspectCreationException(e.getMessage(), e.getRootCauses(), e.getDetailedExitCode()));
}
if (aspectHasConfiguration) {
@@ -433,15 +435,20 @@
transitivePackagesForPackageRootResolution,
transitiveRootCauses,
defaultBuildOptions);
- } catch (ConfiguredTargetFunctionException e) {
- throw new AspectCreationException(e.getMessage(), key.getLabel(), aspectConfiguration);
+ } catch (ConfiguredValueCreationException e) {
+ throw new AspectCreationException(
+ e.getMessage(), key.getLabel(), aspectConfiguration, e.getDetailedExitCode());
}
if (depValueMap == null) {
return null;
}
if (!transitiveRootCauses.isEmpty()) {
+ NestedSet<Cause> causes = transitiveRootCauses.build();
throw new AspectFunctionException(
- new AspectCreationException("Loading failed", transitiveRootCauses.build()));
+ new AspectCreationException(
+ "Loading failed",
+ causes,
+ ConfiguredTargetFunction.getPrioritizedDetailedExitCode(causes)));
}
// Load the requested toolchains into the ToolchainContext, now that we have dependencies.
@@ -477,7 +484,8 @@
if (e.getCause() instanceof ConfiguredValueCreationException) {
ConfiguredValueCreationException cause = (ConfiguredValueCreationException) e.getCause();
throw new AspectFunctionException(
- new AspectCreationException(cause.getMessage(), cause.getRootCauses()));
+ new AspectCreationException(
+ cause.getMessage(), cause.getRootCauses(), cause.getDetailedExitCode()));
} else if (e.getCause() instanceof InconsistentAspectOrderException) {
InconsistentAspectOrderException cause = (InconsistentAspectOrderException) e.getCause();
throw new AspectFunctionException(
@@ -491,7 +499,11 @@
// DependencyEvaluationException constructors, you may need to change this code, too.
InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause();
throw new AspectFunctionException(
- new AspectCreationException(cause.getMessage(), key.getLabel(), aspectConfiguration));
+ new AspectCreationException(
+ cause.getMessage(),
+ key.getLabel(),
+ aspectConfiguration,
+ cause.getDetailedExitCode()));
}
} catch (AspectCreationException e) {
throw new AspectFunctionException(e);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 67d96c7..fa674c4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -848,6 +848,8 @@
"//src/main/java/com/google/devtools/build/lib/causes",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:jsr305",
],
)
@@ -1209,7 +1211,8 @@
"//src/main/java/com/google/devtools/build/lib/causes",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
- "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:jsr305",
],
)
@@ -2279,6 +2282,9 @@
java_library(
name = "sane_analysis_exception",
srcs = ["SaneAnalysisException.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
+ ],
)
java_library(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
index c995d1f..cd64b5b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
@@ -41,7 +41,11 @@
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading;
+import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading.Code;
import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction.BuiltinsFailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -438,7 +442,7 @@
if (!loadStack.add(key)) {
ImmutableList<BzlLoadValue.Key> cycle =
CycleUtils.splitIntoPathAndChain(Predicates.equalTo(key), loadStack).second;
- throw new BzlLoadFailedException("Starlark load cycle: " + cycle);
+ throw new BzlLoadFailedException("Starlark load cycle: " + cycle, Code.CYCLE);
}
}
@@ -514,7 +518,7 @@
BuildFileNotFoundException.class,
InconsistentFilesystemException.class);
} catch (BuildFileNotFoundException | InconsistentFilesystemException e) {
- throw BzlLoadFailedException.errorFindingPackage(label.toPathFragment(), e);
+ throw BzlLoadFailedException.errorFindingContainingPackage(label.toPathFragment(), e);
}
if (packageLookup == null) {
return null;
@@ -560,7 +564,7 @@
// Ensure the .bzl exists and passes static checks (parsing, resolving).
// (A missing prelude file still returns a valid but empty BzlCompileValue.)
if (!compileValue.lookupSuccessful()) {
- throw new BzlLoadFailedException(compileValue.getError());
+ throw new BzlLoadFailedException(compileValue.getError(), Code.COMPILE_ERROR);
}
StarlarkFile file = compileValue.getAST();
if (!file.ok()) {
@@ -1055,61 +1059,103 @@
static final class BzlLoadFailedException extends Exception implements SaneAnalysisException {
private final Transience transience;
+ private final DetailedExitCode detailedExitCode;
- private BzlLoadFailedException(String errorMessage) {
+ private BzlLoadFailedException(
+ String errorMessage, DetailedExitCode detailedExitCode, Transience transience) {
super(errorMessage);
- this.transience = Transience.PERSISTENT;
+ this.transience = transience;
+ this.detailedExitCode = detailedExitCode;
}
- private BzlLoadFailedException(String errorMessage, Exception cause, Transience transience) {
+ private BzlLoadFailedException(String errorMessage, DetailedExitCode detailedExitCode) {
+ this(errorMessage, detailedExitCode, Transience.PERSISTENT);
+ }
+
+ private BzlLoadFailedException(
+ String errorMessage,
+ DetailedExitCode detailedExitCode,
+ Exception cause,
+ Transience transience) {
super(errorMessage, cause);
this.transience = transience;
+ this.detailedExitCode = detailedExitCode;
+ }
+
+ private BzlLoadFailedException(String errorMessage, Code code) {
+ this(errorMessage, createDetailedExitCode(errorMessage, code), Transience.PERSISTENT);
+ }
+
+ private BzlLoadFailedException(
+ String errorMessage, Code code, Exception cause, Transience transience) {
+ this(errorMessage, createDetailedExitCode(errorMessage, code), cause, transience);
}
Transience getTransience() {
return transience;
}
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+
+ private static DetailedExitCode createDetailedExitCode(String message, Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setStarlarkLoading(StarlarkLoading.newBuilder().setCode(code))
+ .build());
+ }
+
// TODO(bazel-team): This exception should hold a Location of the requesting file's load
// statement, and code that catches it should use the location in the Event they create.
static BzlLoadFailedException whileLoadingDep(
String requestingFile, BzlLoadFailedException cause) {
// Don't chain exception cause, just incorporate the message with a prefix.
- return new BzlLoadFailedException("in " + requestingFile + ": " + cause.getMessage());
+ return new BzlLoadFailedException(
+ "in " + requestingFile + ": " + cause.getMessage(), cause.getDetailedExitCode());
}
static BzlLoadFailedException errors(PathFragment file) {
- return new BzlLoadFailedException(String.format("Extension file '%s' has errors", file));
+ return new BzlLoadFailedException(
+ String.format("Extension file '%s' has errors", file), Code.EVAL_ERROR);
}
- static BzlLoadFailedException errorFindingPackage(PathFragment file, Exception cause) {
- return new BzlLoadFailedException(
+ static BzlLoadFailedException errorFindingContainingPackage(
+ PathFragment file, Exception cause) {
+ String errorMessage =
String.format(
- "Encountered error while reading extension file '%s': %s", file, cause.getMessage()),
- cause,
- Transience.PERSISTENT);
+ "Encountered error while reading extension file '%s': %s", file, cause.getMessage());
+ DetailedExitCode detailedExitCode =
+ cause instanceof DetailedException
+ ? ((DetailedException) cause).getDetailedExitCode()
+ : createDetailedExitCode(errorMessage, Code.CONTAINING_PACKAGE_NOT_FOUND);
+ return new BzlLoadFailedException(
+ errorMessage, detailedExitCode, cause, Transience.PERSISTENT);
}
static BzlLoadFailedException errorReadingBzl(
PathFragment file, BzlCompileFunction.FailedIOException cause) {
- return new BzlLoadFailedException(
+ String errorMessage =
String.format(
- "Encountered error while reading extension file '%s': %s", file, cause.getMessage()),
- cause,
- cause.getTransience());
+ "Encountered error while reading extension file '%s': %s", file, cause.getMessage());
+ return new BzlLoadFailedException(errorMessage, Code.IO_ERROR, cause, cause.getTransience());
}
static BzlLoadFailedException noBuildFile(Label file, @Nullable String reason) {
if (reason != null) {
return new BzlLoadFailedException(
- String.format("Unable to find package for %s: %s.", file, reason));
+ String.format("Unable to find package for %s: %s.", file, reason),
+ Code.PACKAGE_NOT_FOUND);
}
return new BzlLoadFailedException(
String.format(
"Every .bzl file must have a corresponding package, but '%s' does not have one."
+ " Please create a BUILD file in the same or any parent directory. Note that"
+ " this BUILD file does not need to do anything except exist.",
- file));
+ file),
+ Code.PACKAGE_NOT_FOUND);
}
static BzlLoadFailedException labelCrossesPackageBoundary(
@@ -1122,11 +1168,13 @@
// message for the user.
containingPackageLookupValue.getContainingPackageRoot(),
label,
- containingPackageLookupValue));
+ containingPackageLookupValue),
+ Code.LABEL_CROSSES_PACKAGE_BOUNDARY);
}
static BzlLoadFailedException starlarkErrors(PathFragment file) {
- return new BzlLoadFailedException(String.format("Extension '%s' has errors", file));
+ return new BzlLoadFailedException(
+ String.format("Extension '%s' has errors", file), Code.PARSE_ERROR);
}
static BzlLoadFailedException builtinsFailed(Label file, BuiltinsFailedException cause) {
@@ -1134,6 +1182,7 @@
String.format(
"Internal error while loading Starlark builtins for %s: %s",
file, cause.getMessage()),
+ Code.BUILTINS_ERROR,
cause,
cause.getTransience());
}
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 f15da2a..0d6a5e6 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
@@ -79,9 +79,12 @@
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.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -225,7 +228,8 @@
target = pkg.getTarget(label.getName());
} catch (NoSuchTargetException e) {
throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(e.getMessage(), label, configuration));
+ new ConfiguredValueCreationException(
+ e.getMessage(), label, configuration, e.getDetailedExitCode()));
}
if (pkg.containsErrors()) {
FailureDetail failureDetail = pkg.contextualizeFailureDetailForTarget(target);
@@ -305,9 +309,13 @@
// attributes.
if (!transitiveRootCauses.isEmpty()
&& !Objects.equals(configConditions, NO_CONFIG_CONDITIONS)) {
+ NestedSet<Cause> causes = transitiveRootCauses.build();
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
- "Cannot compute config conditions", configuration, transitiveRootCauses.build()));
+ "Cannot compute config conditions",
+ configuration,
+ causes,
+ getPrioritizedDetailedExitCode(causes)));
}
// Calculate the dependencies of this target.
@@ -328,9 +336,10 @@
transitiveRootCauses,
defaultBuildOptions);
if (!transitiveRootCauses.isEmpty()) {
+ NestedSet<Cause> causes = transitiveRootCauses.build();
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
- "Analysis failed", configuration, transitiveRootCauses.build()));
+ "Analysis failed", configuration, causes, getPrioritizedDetailedExitCode(causes)));
}
if (env.valuesMissing()) {
return null;
@@ -408,7 +417,7 @@
env.getListener().handle(Event.error(cause.getMessage()));
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
- cause.getMessage(), target.getLabel(), configuration));
+ cause.getMessage(), target.getLabel(), configuration, cause.getDetailedExitCode()));
} else if (e.getCause() instanceof TransitionException) {
TransitionException cause = (TransitionException) e.getCause();
env.getListener().handle(Event.error(cause.getMessage()));
@@ -422,24 +431,23 @@
} catch (AspectCreationException e) {
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
- e.getMessage(),
- configuration,
- e.getCauses()));
+ e.getMessage(), configuration, e.getCauses(), e.getDetailedExitCode()));
} catch (ToolchainException e) {
// We need to throw a ConfiguredValueCreationException, so either find one or make one.
ConfiguredValueCreationException cvce = asConfiguredValueCreationException(e);
if (cvce == null) {
cvce =
- new ConfiguredValueCreationException(e.getMessage(), target.getLabel(), configuration);
+ new ConfiguredValueCreationException(
+ e.getMessage(), target.getLabel(), configuration, e.getDetailedExitCode());
}
- env.getListener()
- .handle(
- Event.error(
- String.format(
- "While resolving toolchains for target %s: %s",
- target.getLabel(), e.getMessage())));
+ String message =
+ String.format(
+ "While resolving toolchains for target %s: %s", target.getLabel(), e.getMessage());
+ env.getListener().handle(Event.error(message));
throw new ConfiguredTargetFunctionException(cvce);
+ } catch (ConfiguredValueCreationException e) {
+ throw new ConfiguredTargetFunctionException(e);
} finally {
cpuBoundSemaphore.release();
}
@@ -631,7 +639,7 @@
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
NestedSetBuilder<Cause> transitiveRootCauses,
BuildOptions defaultBuildOptions)
- throws DependencyEvaluationException, ConfiguredTargetFunctionException,
+ throws DependencyEvaluationException, ConfiguredValueCreationException,
AspectCreationException, InterruptedException {
// Create the map from attributes to set of (target, transition) pairs.
OrderedSetMultimap<DependencyKind, DependencyKey> initialDependencies;
@@ -702,8 +710,7 @@
env.getListener().handle(
Event.error(ctgValue.getTarget().getLocation(), e.getMessage()));
- throw new ConfiguredTargetFunctionException(
- new ConfiguredValueCreationException(e.getMessage(), label, configuration));
+ throw new ConfiguredValueCreationException(e.getMessage(), label, configuration);
}
}
@@ -848,6 +855,7 @@
throws DependencyEvaluationException, InterruptedException {
boolean missedValues = env.valuesMissing();
String failWithMessage = null;
+ DetailedExitCode detailedExitCode = 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.
@@ -946,7 +954,12 @@
}
} catch (ConfiguredValueCreationException e) {
transitiveRootCauses.addTransitive(e.getRootCauses());
- failWithMessage = e.getMessage();
+ detailedExitCode =
+ DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie(
+ e.getDetailedExitCode(), detailedExitCode);
+ if (e.getDetailedExitCode().equals(detailedExitCode)) {
+ failWithMessage = e.getMessage();
+ }
}
}
if (aliasDepsToRedo.isEmpty()) {
@@ -958,7 +971,10 @@
if (failWithMessage != null) {
throw new DependencyEvaluationException(
new ConfiguredValueCreationException(
- failWithMessage, ctgValue.getConfiguration(), transitiveRootCauses.build()));
+ failWithMessage,
+ ctgValue.getConfiguration(),
+ transitiveRootCauses.build(),
+ detailedExitCode));
} else if (missedValues) {
return null;
} else {
@@ -1026,7 +1042,8 @@
configuration == null
? null
: configuration.getEventId().getConfiguration(),
- event.getMessage()))
+ createDetailedExitCode(
+ event.getMessage(), Code.CONFIGURED_VALUE_CREATION_FAILED)))
.collect(Collectors.toList()));
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
@@ -1071,6 +1088,25 @@
}
}
+ private static DetailedExitCode createDetailedExitCode(String message, Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .build());
+ }
+
+ static DetailedExitCode getPrioritizedDetailedExitCode(NestedSet<Cause> causes) {
+ DetailedExitCode[] prioritizedDetailedExitCodeWrapper = {null};
+ causes.forEachElement(
+ o -> true,
+ c ->
+ prioritizedDetailedExitCodeWrapper[0] =
+ DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie(
+ prioritizedDetailedExitCodeWrapper[0], c.getDetailedExitCode()));
+ return prioritizedDetailedExitCodeWrapper[0];
+ }
+
/**
* Used to declare all the exception types that can be wrapped in the exception thrown by {@link
* ConfiguredTargetFunction#compute}.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredValueCreationException.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredValueCreationException.java
index 0a6f7e5..3c238f2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredValueCreationException.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredValueCreationException.java
@@ -20,31 +20,41 @@
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.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import javax.annotation.Nullable;
/**
* An exception indicating that there was a problem during the construction of a
* ConfiguredTargetValue.
*/
-@AutoCodec
public final class ConfiguredValueCreationException extends Exception
implements SaneAnalysisException {
@Nullable private final BuildEventId configuration;
private final NestedSet<Cause> rootCauses;
+ // TODO(b/138456686): if warranted by a need for finer-grained details, replace the constructors
+ // that specify the general Code.CONFIGURED_VALUE_CREATION_FAILED
+ private final DetailedExitCode detailedExitCode;
- @AutoCodec.VisibleForSerialization
- @AutoCodec.Instantiator
- ConfiguredValueCreationException(
- String message, @Nullable BuildEventId configuration, NestedSet<Cause> rootCauses) {
+ private ConfiguredValueCreationException(
+ String message,
+ @Nullable BuildEventId configuration,
+ NestedSet<Cause> rootCauses,
+ DetailedExitCode detailedExitCode) {
super(message);
this.configuration = configuration;
this.rootCauses = rootCauses;
+ this.detailedExitCode = detailedExitCode;
}
public ConfiguredValueCreationException(
- String message, Label currentTarget, @Nullable BuildConfiguration configuration) {
+ String message,
+ Label currentTarget,
+ @Nullable BuildConfiguration configuration,
+ DetailedExitCode detailedExitCode) {
this(
message,
configuration == null ? null : configuration.getEventId(),
@@ -53,13 +63,39 @@
new AnalysisFailedCause(
currentTarget,
configuration == null ? null : configuration.getEventId().getConfiguration(),
- message))
- .build());
+ detailedExitCode))
+ .build(),
+ detailedExitCode);
+ }
+
+ public ConfiguredValueCreationException(
+ String message, Label currentTarget, @Nullable BuildConfiguration configuration) {
+ this(
+ message,
+ currentTarget,
+ configuration,
+ createDetailedExitCode(message, Code.CONFIGURED_VALUE_CREATION_FAILED));
+ }
+
+ public ConfiguredValueCreationException(
+ String message,
+ @Nullable BuildConfiguration configuration,
+ NestedSet<Cause> rootCauses,
+ DetailedExitCode detailedExitCode) {
+ this(
+ message,
+ configuration == null ? null : configuration.getEventId(),
+ rootCauses,
+ detailedExitCode);
}
public ConfiguredValueCreationException(
String message, @Nullable BuildConfiguration configuration, NestedSet<Cause> rootCauses) {
- this(message, configuration == null ? null : configuration.getEventId(), rootCauses);
+ this(
+ message,
+ configuration,
+ rootCauses,
+ createDetailedExitCode(message, Code.CONFIGURED_VALUE_CREATION_FAILED));
}
public NestedSet<Cause> getRootCauses() {
@@ -70,4 +106,17 @@
public BuildEventId getConfiguration() {
return configuration;
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+
+ private static DetailedExitCode createDetailedExitCode(String message, Analysis.Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .build());
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
index 7cd23c6..6b1dba1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
@@ -32,8 +32,12 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -196,20 +200,28 @@
* Used to indicate that the given {@link Label} represents a {@link ConfiguredTarget} which is
* not a valid {@link PlatformInfo} provider.
*/
- static final class InvalidExecutionPlatformLabelException extends Exception
+ private static final class InvalidExecutionPlatformLabelException extends Exception
implements SaneAnalysisException {
- public InvalidExecutionPlatformLabelException(
- TargetPatternUtil.InvalidTargetPatternException e) {
+ InvalidExecutionPlatformLabelException(TargetPatternUtil.InvalidTargetPatternException e) {
this(e.getInvalidPattern(), e.getTpe());
}
- public InvalidExecutionPlatformLabelException(String invalidPattern, TargetParsingException e) {
+ InvalidExecutionPlatformLabelException(String invalidPattern, TargetParsingException e) {
super(
String.format(
"invalid registered execution platform '%s': %s", invalidPattern, e.getMessage()),
e);
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(getMessage())
+ .setAnalysis(Analysis.newBuilder().setCode(Code.INVALID_EXECUTION_PLATFORM))
+ .build());
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SaneAnalysisException.java b/src/main/java/com/google/devtools/build/lib/skyframe/SaneAnalysisException.java
index 1a188fe..a4aff69 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SaneAnalysisException.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SaneAnalysisException.java
@@ -15,6 +15,6 @@
/**
* Marker interface for exceptions during loading/analysis that do not prevent targets from being
- * configured. See {@link SkyframeBuildView#isSaneAnalysisError}.
+ * configured. See {@link SkyframeBuildView#convertToAnalysisException}.
*/
-public interface SaneAnalysisException {}
+public interface SaneAnalysisException extends DetailedException {}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index b4ec47f..2ee83f2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -478,9 +478,12 @@
Collection<Exception> reportedExceptions = Sets.newHashSet();
for (Map.Entry<ActionAnalysisMetadata, ConflictException> bad : actionConflicts.entrySet()) {
ConflictException ex = bad.getValue();
+ DetailedExitCode detailedExitCode;
try {
ex.rethrowTyped();
+ throw new IllegalStateException("ConflictException.rethrowTyped must throw");
} catch (ActionConflictException ace) {
+ detailedExitCode = ace.getDetailedExitCode();
ace.reportTo(eventHandler);
if (keepGoing) {
eventHandler.handle(
@@ -490,13 +493,14 @@
+ "': it will not be built"));
}
} catch (ArtifactPrefixConflictException apce) {
+ detailedExitCode = apce.getDetailedExitCode();
if (reportedExceptions.add(apce)) {
eventHandler.handle(Event.error(apce.getMessage()));
}
}
// TODO(ulfjack): Don't throw here in the nokeep_going case, but report all known issues.
if (!keepGoing) {
- throw new ViewCreationFailedException(ex);
+ throw new ViewCreationFailedException(detailedExitCode.getFailureDetail(), ex);
}
}
@@ -644,7 +648,7 @@
for (Map.Entry<SkyKey, ErrorInfo> errorEntry : result.errorMap().entrySet()) {
SkyKey errorKey = errorEntry.getKey();
ErrorInfo errorInfo = errorEntry.getValue();
- assertSaneAnalysisError(errorInfo, errorKey, result.getWalkableGraph());
+ assertValidAnalysisException(errorInfo, errorKey, result.getWalkableGraph());
skyframeExecutor
.getCyclesReporter().reportCycles(errorInfo.getCycleInfo(), errorKey, eventHandler);
Exception cause = errorInfo.getException();
@@ -653,18 +657,12 @@
if (errorKey.argument() instanceof AspectValueKey) {
// We skip Aspects in the keepGoing case; the failures should already have been reported to
// the event handler.
- if (!keepGoing) {
+ if (!keepGoing && noKeepGoingException == null) {
AspectValueKey aspectKey = (AspectValueKey) errorKey.argument();
String errorMsg =
String.format(
"Analysis of aspect '%s' failed; build aborted", aspectKey.getDescription());
- if (noKeepGoingException == null) {
- if (cause != null) {
- noKeepGoingException = new ViewCreationFailedException(errorMsg, cause);
- } else {
- noKeepGoingException = new ViewCreationFailedException(errorMsg);
- }
- }
+ noKeepGoingException = createViewCreationFailedException(cause, errorMsg);
}
continue;
}
@@ -720,11 +718,7 @@
Order.STABLE_ORDER,
new LabelCause(
analysisRootCause,
- DetailedExitCode.of(
- FailureDetail.newBuilder()
- .setMessage("Dependency cycle")
- .setAnalysis(Analysis.newBuilder().setCode(Code.CYCLE))
- .build())))
+ DetailedExitCode.of(createFailureDetail("Dependency cycle", Code.CYCLE))))
// TODO(ulfjack): We need to report the dependency cycle here. How?
: NestedSetBuilder.emptySet(Order.STABLE_ORDER);
} else if (cause instanceof ActionConflictException) {
@@ -738,12 +732,14 @@
configurationLookupSupplier.get().get(label.getConfigurationKey());
ConfigurationId configId = configuration.getEventId().getConfiguration();
AnalysisFailedCause analysisFailedCause =
- new AnalysisFailedCause(topLevelLabel, configId, cause.getMessage());
+ new AnalysisFailedCause(
+ topLevelLabel, configId, ((NoSuchPackageException) cause).getDetailedExitCode());
rootCauses = NestedSetBuilder.create(Order.STABLE_ORDER, analysisFailedCause);
} else {
// TODO(ulfjack): Report something!
rootCauses = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}
+
if (keepGoing) {
eventHandler.handle(
Event.warn(
@@ -753,12 +749,9 @@
} else if (noKeepGoingException == null) {
String errorMsg =
String.format("Analysis of target '%s' failed; build aborted", topLevelLabel);
- if (cause != null) {
- noKeepGoingException = new ViewCreationFailedException(errorMsg, cause);
- } else {
- noKeepGoingException = new ViewCreationFailedException(errorMsg);
- }
+ noKeepGoingException = createViewCreationFailedException(cause, errorMsg);
}
+
if (!inTest) {
BuildConfiguration configuration =
configurationLookupSupplier.get().get(label.getConfigurationKey());
@@ -773,6 +766,41 @@
return Pair.of(hasLoadingError, noKeepGoingException);
}
+ private static ViewCreationFailedException createViewCreationFailedException(
+ @Nullable Exception e, String errorMsg) {
+ if (e == null) {
+ return new ViewCreationFailedException(
+ errorMsg, createFailureDetail(errorMsg + " due to cycle", Code.CYCLE));
+ }
+ return new ViewCreationFailedException(
+ errorMsg, maybeContextualizeFailureDetail(e, errorMsg), e);
+ }
+
+ /**
+ * Returns a {@link FailureDetail} with message prefixed by {@code errorMsg} derived from the
+ * failure detail in {@code e} if it's a {@link DetailedException}, and otherwise returns one with
+ * {@code errorMsg} and {@link Code#UNEXPECTED_ANALYSIS_EXCEPTION}.
+ */
+ private static FailureDetail maybeContextualizeFailureDetail(
+ @Nullable Exception e, String errorMsg) {
+ DetailedException detailedException = convertToAnalysisException(e);
+ if (detailedException == null) {
+ return createFailureDetail(errorMsg, Code.UNEXPECTED_ANALYSIS_EXCEPTION);
+ }
+ FailureDetail originalFailureDetail =
+ detailedException.getDetailedExitCode().getFailureDetail();
+ return originalFailureDetail.toBuilder()
+ .setMessage(errorMsg + ": " + originalFailureDetail.getMessage())
+ .build();
+ }
+
+ private static FailureDetail createFailureDetail(String errorMessage, Code code) {
+ return FailureDetail.newBuilder()
+ .setMessage(errorMessage)
+ .setAnalysis(Analysis.newBuilder().setCode(code))
+ .build();
+ }
+
/** Returns a map of collected package names to root paths. */
private static ImmutableMap<PackageIdentifier, Root> collectPackageRoots(
Collection<Package> packages) {
@@ -805,50 +833,61 @@
return null;
}
- private static void assertSaneAnalysisError(
+ private static void assertValidAnalysisException(
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph) throws InterruptedException {
Throwable cause = errorInfo.getException();
- // We should only be trying to configure targets when the loading phase succeeds, meaning
- // that the only errors should be analysis errors.
- if (cause != null && !isSaneAnalysisError(cause)) {
- // Walk the graph to find a path to the lowest-level node that threw unexpected exception.
- List<SkyKey> path = new ArrayList<>();
- try {
- SkyKey currentKey = key;
- boolean foundDep;
- do {
- path.add(currentKey);
- foundDep = false;
+ if (cause == null) {
+ // Cycle.
+ return;
+ }
- Map<SkyKey, Exception> missingMap =
- walkableGraph.getMissingAndExceptions(ImmutableList.of(currentKey));
- if (missingMap.containsKey(currentKey) && missingMap.get(currentKey) == null) {
- // This can happen in a no-keep-going build, where we don't write the bubbled-up error
- // nodes to the graph.
+ if (convertToAnalysisException(cause) != null) {
+ // Valid exception type.
+ return;
+ }
+
+ // Walk the graph to find a path to the lowest-level node that threw unexpected exception.
+ List<SkyKey> path = new ArrayList<>();
+ try {
+ SkyKey currentKey = key;
+ boolean foundDep;
+ do {
+ path.add(currentKey);
+ foundDep = false;
+
+ Map<SkyKey, Exception> missingMap =
+ walkableGraph.getMissingAndExceptions(ImmutableList.of(currentKey));
+ if (missingMap.containsKey(currentKey) && missingMap.get(currentKey) == null) {
+ // This can happen in a no-keep-going build, where we don't write the bubbled-up error
+ // nodes to the graph.
+ break;
+ }
+
+ for (SkyKey dep : walkableGraph.getDirectDeps(currentKey)) {
+ if (cause.equals(walkableGraph.getException(dep))) {
+ currentKey = dep;
+ foundDep = true;
break;
}
-
- for (SkyKey dep : walkableGraph.getDirectDeps(currentKey)) {
- if (cause.equals(walkableGraph.getException(dep))) {
- currentKey = dep;
- foundDep = true;
- break;
- }
- }
- } while (foundDep);
- } finally {
- BugReport.sendBugReport(
- new IllegalStateException(
- "Unexpected analysis error: " + key + " -> " + errorInfo + ", (" + path + ")"));
- }
+ }
+ } while (foundDep);
+ } finally {
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ "Unexpected analysis error: " + key + " -> " + errorInfo + ", (" + path + ")"));
}
}
- private static boolean isSaneAnalysisError(Throwable cause) {
- return cause instanceof SaneAnalysisException
- // Only if we run the reduced loading phase and then analyze with --nokeep_going.
+ @Nullable
+ private static DetailedException convertToAnalysisException(Throwable cause) {
+ // The cause may be NoSuch{Target,Package}Exception if we run the reduced loading phase and then
+ // analyze with --nokeep_going.
+ if (cause instanceof SaneAnalysisException
|| cause instanceof NoSuchTargetException
- || cause instanceof NoSuchPackageException;
+ || cause instanceof NoSuchPackageException) {
+ return (DetailedException) cause;
+ }
+ return null;
}
public ArtifactFactory getArtifactFactory() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
index 7473931..64415b5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
@@ -156,7 +156,7 @@
configId = configuration.getEventId().getConfiguration();
}
env.getListener().post(new AnalysisRootCauseEvent(configuration, label, e.getMessage()));
- rootCauses.add(new AnalysisFailedCause(label, configId, e.getMessage()));
+ rootCauses.add(new AnalysisFailedCause(label, configId, e.getDetailedExitCode()));
missingEdgeHook(fromTarget, entry.getKey(), label, e);
continue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java b/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
index 3b0146b..847694a 100644
--- a/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
+++ b/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
@@ -27,7 +27,7 @@
import java.util.Objects;
import javax.annotation.Nullable;
-/** An {@link ExitCode} and an optional {@link FailureDetail}. */
+/** An {@link ExitCode} that has a {@link FailureDetail} unless it's {@link ExitCode#SUCCESS}. */
public class DetailedExitCode {
private final ExitCode exitCode;
@Nullable private final FailureDetail failureDetail;
@@ -65,25 +65,6 @@
}
/**
- * Returns a {@link DetailedExitCode} specifying {@link ExitCode} but no {@link FailureDetail}.
- *
- * <p>This method exists in order to allow for code which has not yet been wired for {@link
- * FailureDetail) support to interact with {@link FailureDetail}-handling code infrastructure.
- *
- * <p>Callsites should migrate to using either:
- *
- * <ul>
- * <li>{@link #of(ExitCode, FailureDetail)}, when they're wired for {@link FailureDetail}
- * support but not yet ready to have {@link FailureDetail} metadata determine exit code behavior
- * <li>{@link #of(FailureDetail)}, when changing exit code behavior is desired.
- * </ul>
- *
- */
- public static DetailedExitCode justExitCode(ExitCode exitCode) {
- return new DetailedExitCode(checkNotNull(exitCode), null);
- }
-
- /**
* Returns a {@link DetailedExitCode} combining the provided {@link FailureDetail} and {@link
* ExitCode}.
*
@@ -91,9 +72,9 @@
* FailureDetail)-handling code infrastructure without requiring any simultaneous change in exit
* code behavior.
*
- * <p>Callsites should migrate to using {@link #of(FailureDetail)} instead.
+ * <p>Unless contrained by backwards-compatibility needs, callers should use {@link
+ * #of(FailureDetail)} instead.
*/
- // TODO(b/138456686): consider controlling this behavior by flag if migration appears risky.
public static DetailedExitCode of(ExitCode exitCode, FailureDetail failureDetail) {
return new DetailedExitCode(checkNotNull(exitCode), checkNotNull(failureDetail));
}
@@ -103,7 +84,7 @@
* FailureDetail}'s metadata.
*/
public static DetailedExitCode of(FailureDetail failureDetail) {
- return new DetailedExitCode(getExitCode(failureDetail), failureDetail);
+ return new DetailedExitCode(getExitCode(failureDetail), checkNotNull(failureDetail));
}
@Override
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index 4bcde36..639b574 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -144,6 +144,7 @@
Analysis analysis = 174;
PackageLoading package_loading = 175;
Toolchain toolchain = 177;
+ StarlarkLoading starlark_loading = 179;
}
reserved 102; // For internal use
@@ -160,6 +161,8 @@
reserved 155 to 157; // For internal use
reserved 165; // For internal use
reserved 170 to 171; // For internal use
+ reserved 176; // For internal use
+ reserved 178; // For internal use
}
message Interrupted {
@@ -1102,6 +1105,19 @@
NOT_ALL_TARGETS_ANALYZED = 3 [(metadata) = { exit_code: 1 }];
CYCLE = 4 [(metadata) = { exit_code: 1 }];
PARAMETERIZED_TOP_LEVEL_ASPECT_INVALID = 5 [(metadata) = { exit_code: 1 }];
+ ASPECT_LABEL_SYNTAX_ERROR = 6 [(metadata) = { exit_code: 1 }];
+ ASPECT_PREREQ_UNMET = 7 [(metadata) = { exit_code: 1 }];
+ ASPECT_NOT_FOUND = 8 [(metadata) = { exit_code: 1 }];
+ ACTION_CONFLICT = 9 [(metadata) = { exit_code: 1 }];
+ ARTIFACT_PREFIX_CONFLICT = 10 [(metadata) = { exit_code: 1 }];
+ UNEXPECTED_ANALYSIS_EXCEPTION = 11 [(metadata) = { exit_code: 1 }];
+ TARGETS_MISSING_ENVIRONMENTS = 12 [(metadata) = { exit_code: 1 }];
+ INVALID_ENVIRONMENT = 13 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_MISSING_FROM_GROUPS = 14 [(metadata) = { exit_code: 1 }];
+ EXEC_GROUP_MISSING = 15 [(metadata) = { exit_code: 1 }];
+ INVALID_EXECUTION_PLATFORM = 16 [(metadata) = { exit_code: 1 }];
+ ASPECT_CREATION_FAILED = 17 [(metadata) = { exit_code: 1 }];
+ CONFIGURED_VALUE_CREATION_FAILED = 18 [(metadata) = { exit_code: 1 }];
}
Code code = 1;
@@ -1146,15 +1162,6 @@
Code code = 1;
}
-message Blueprint {
- enum Code {
- BLUEPRINT_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
- UNSUPPORTED_COMMAND = 1 [(metadata) = { exit_code: 2 }];
- }
-
- Code code = 1;
-}
-
message Toolchain {
enum Code {
TOOLCHAIN_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
@@ -1169,3 +1176,20 @@
Code code = 1;
}
+
+message StarlarkLoading {
+ enum Code {
+ STARLARK_LOADING_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
+ CYCLE = 1 [(metadata) = { exit_code: 1 }];
+ COMPILE_ERROR = 2 [(metadata) = { exit_code: 1 }];
+ PARSE_ERROR = 3 [(metadata) = { exit_code: 1 }];
+ EVAL_ERROR = 4 [(metadata) = { exit_code: 1 }];
+ CONTAINING_PACKAGE_NOT_FOUND = 5 [(metadata) = { exit_code: 1 }];
+ PACKAGE_NOT_FOUND = 6 [(metadata) = { exit_code: 1 }];
+ IO_ERROR = 7 [(metadata) = { exit_code: 1 }];
+ LABEL_CROSSES_PACKAGE_BOUNDARY = 8 [(metadata) = { exit_code: 1 }];
+ BUILTINS_ERROR = 9 [(metadata) = { exit_code: 1 }];
+ }
+
+ Code code = 1;
+}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java
index 378097b..ed1302b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java
@@ -28,8 +28,13 @@
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.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestSpec;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
@@ -38,15 +43,14 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Analysis failure reporting tests.
- */
+/** Analysis failure reporting tests. */
@TestSpec(size = Suite.SMALL_TESTS)
@RunWith(JUnit4.class)
public class AnalysisFailureReportingTest extends AnalysisTestCase {
private final AnalysisFailureEventCollector collector = new AnalysisFailureEventCollector();
- // TODO(ulfjack): Don't check for exact error message wording; instead, add machine-readable
+ // TODO(mschaller): The below is closer now because of e.g. DetailedExitCode/FailureDetail.
+ // original(ulfjack): Don't check for exact error message wording; instead, add machine-readable
// details to the events, and check for those. Also check if we can remove duplicate test coverage
// for these errors, i.e., consolidate the failure reporting tests in this class.
@@ -85,7 +89,8 @@
@Test
public void testMissingDependency() throws Exception {
- scratch.file("foo/BUILD",
+ scratch.file(
+ "foo/BUILD",
"genrule(name = 'foo',",
" tools = ['//bar'],",
" cmd = 'command',",
@@ -102,9 +107,12 @@
toId(
Iterables.getOnlyElement(result.getTopLevelTargetsWithConfigs())
.getConfiguration()),
- "no such package 'bar': BUILD file not found in any of the following "
- + "directories. Add a BUILD file to a directory to mark it as a package.\n"
- + " - /workspace/bar"));
+ createPackageLoadingDetailedExitCode(
+ "BUILD file not found in any of the following"
+ + " directories. Add a BUILD file to a directory to mark it as a"
+ + " package.\n"
+ + " - /workspace/bar",
+ Code.BUILD_FILE_MISSING)));
}
/**
@@ -114,14 +122,11 @@
*/
@Test
public void testSymlinkCycleReportedExactlyOnce() throws Exception {
- scratch.file("gp/BUILD",
- "sh_library(name = 'gp', deps = ['//p'])");
- scratch.file("p/BUILD",
- "sh_library(name = 'p', deps = ['//c'])");
- scratch.file("c/BUILD",
- "sh_library(name = 'c', deps = ['//cycles1'])");
- Path cycles1BuildFilePath = scratch.file("cycles1/BUILD",
- "sh_library(name = 'cycles1', srcs = glob(['*.sh']))");
+ scratch.file("gp/BUILD", "sh_library(name = 'gp', deps = ['//p'])");
+ scratch.file("p/BUILD", "sh_library(name = 'p', deps = ['//c'])");
+ scratch.file("c/BUILD", "sh_library(name = 'c', deps = ['//cycles1'])");
+ Path cycles1BuildFilePath =
+ scratch.file("cycles1/BUILD", "sh_library(name = 'cycles1', srcs = glob(['*.sh']))");
cycles1BuildFilePath
.getParentDirectory()
.getRelative("cycles1.sh")
@@ -131,6 +136,9 @@
assertThat(result.hasError()).isTrue();
Label topLevel = Label.parseAbsoluteUnchecked("//gp");
+ String message =
+ "Symlink issue while evaluating globs: Symlink cycle:" + " /workspace/cycles1/cycles1.sh";
+ Code code = Code.EVAL_GLOBS_SYMLINK_ERROR;
assertThat(collector.events.get(topLevel))
.containsExactly(
new AnalysisFailedCause(
@@ -138,16 +146,13 @@
toId(
Iterables.getOnlyElement(result.getTopLevelTargetsWithConfigs())
.getConfiguration()),
- "no such package 'cycles1': Symlink issue while evaluating globs: Symlink cycle: "
- + "/workspace/cycles1/cycles1.sh"));
+ createPackageLoadingDetailedExitCode(message, code)));
}
@Test
public void testVisibilityError() throws Exception {
- scratch.file("foo/BUILD",
- "sh_library(name = 'foo', deps = ['//bar'])");
- scratch.file("bar/BUILD",
- "sh_library(name = 'bar', visibility = ['//visibility:private'])");
+ scratch.file("foo/BUILD", "sh_library(name = 'foo', deps = ['//bar'])");
+ scratch.file("bar/BUILD", "sh_library(name = 'bar', visibility = ['//visibility:private'])");
AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//foo");
assertThat(result.hasError()).isTrue();
@@ -160,9 +165,10 @@
toId(
Iterables.getOnlyElement(result.getTopLevelTargetsWithConfigs())
.getConfiguration()),
- "in sh_library rule //foo:foo: target '//bar:bar' is not visible from target "
- + "'//foo:foo'. Check the visibility declaration of the former target if you "
- + "think the dependency is legitimate"));
+ createAnalysisDetailedExitCode(
+ "in sh_library rule //foo:foo: target '//bar:bar' is not visible from"
+ + " target '//foo:foo'. Check the visibility declaration of the"
+ + " former target if you think the dependency is legitimate")));
}
@Test
@@ -183,18 +189,24 @@
toId(
Iterables.getOnlyElement(result.getTopLevelTargetsWithConfigs())
.getConfiguration()),
- "in sh_library rule //foo:foo: target '//bar:bar.sh' is not visible from target "
- + "'//foo:foo'. Check the visibility declaration of the former target if you "
- + "think the dependency is legitimate. To set the visibility of that source "
- + "file target, use the exports_files() function"));
+ DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(
+ "in sh_library rule //foo:foo: target '//bar:bar.sh' is not visible"
+ + " from target '//foo:foo'. Check the visibility declaration of"
+ + " the former target if you think the dependency is legitimate."
+ + " To set the visibility of that source file target, use the"
+ + " exports_files() function")
+ .setAnalysis(
+ Analysis.newBuilder()
+ .setCode(Analysis.Code.CONFIGURED_VALUE_CREATION_FAILED))
+ .build())));
}
@Test
public void testVisibilityErrorNoKeepGoing() throws Exception {
- scratch.file("foo/BUILD",
- "sh_library(name = 'foo', deps = ['//bar'])");
- scratch.file("bar/BUILD",
- "sh_library(name = 'bar', visibility = ['//visibility:private'])");
+ scratch.file("foo/BUILD", "sh_library(name = 'foo', deps = ['//bar'])");
+ scratch.file("bar/BUILD", "sh_library(name = 'bar', visibility = ['//visibility:private'])");
try {
update(eventBus, defaultFlags(), "//foo");
@@ -207,15 +219,35 @@
Iterables.getOnlyElement(
skyframeExecutor
.getSkyframeBuildView()
- .getBuildConfigurationCollection().getTargetConfigurations());
+ .getBuildConfigurationCollection()
+ .getTargetConfigurations());
+ String message =
+ "in sh_library rule //foo:foo: target '//bar:bar' is not visible from"
+ + " target '//foo:foo'. Check the visibility declaration of the"
+ + " former target if you think the dependency is legitimate";
assertThat(collector.events.get(topLevel))
.containsExactly(
new AnalysisFailedCause(
Label.parseAbsolute("//foo", ImmutableMap.of()),
toId(expectedConfig),
- "in sh_library rule //foo:foo: target '//bar:bar' is not visible from target "
- + "'//foo:foo'. Check the visibility declaration of the former target if you "
- + "think the dependency is legitimate"));
+ createAnalysisDetailedExitCode(message)));
+ }
+
+ public static DetailedExitCode createPackageLoadingDetailedExitCode(String message, Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setPackageLoading(PackageLoading.newBuilder().setCode(code))
+ .build());
+ }
+
+ public static DetailedExitCode createAnalysisDetailedExitCode(String message) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setAnalysis(
+ Analysis.newBuilder().setCode(Analysis.Code.CONFIGURED_VALUE_CREATION_FAILED))
+ .build());
}
// TODO(ulfjack): Add more tests for
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
index 0517a88..dfebf6f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -138,6 +138,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:depsutils",
"//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
"//src/main/java/com/google/devtools/build/lib/util",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/util:shell_escaper",
@@ -149,8 +150,8 @@
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
- "//src/main/java/net/starlark/java/syntax",
"//src/main/protobuf:extra_actions_base_java_proto",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//src/test/java/com/google/devtools/build/lib/analysis/testing",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/starlark/BUILD
index b9153c5..c9cccfe 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD
@@ -67,6 +67,7 @@
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
"//src/main/java/net/starlark/java/syntax",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
index 875a953..fb74540 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
@@ -46,6 +46,9 @@
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
import com.google.devtools.build.lib.rules.objc.ObjcProtoProvider;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis;
+import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import net.starlark.java.eval.Sequence;
@@ -1576,7 +1579,13 @@
AnalysisResult result = update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
if (result.hasError()) {
assertThat(keepGoing()).isTrue();
- throw new ViewCreationFailedException("Analysis failed");
+ String errorMessage = "Analysis failed";
+ throw new ViewCreationFailedException(
+ errorMessage,
+ FailureDetail.newBuilder()
+ .setMessage(errorMessage)
+ .setAnalysis(Analysis.newBuilder().setCode(Code.ANALYSIS_UNKNOWN))
+ .build());
}
return getConfiguredTarget("//test:xxx");
