Collect and relay detailed failures in TargetEdgeErrorObservers
Annotates (non-exhaustively) non-exceptional failures during package
creation and evaluation, and surfaces those failures during query
evaluation, to be included in its detailed failure reporting.
Some rule-specific failures remain undetailed, for now, subject to
follow-up work.
RELNOTES: None.
PiperOrigin-RevId: 331586901
diff --git a/src/main/java/com/google/devtools/build/lib/events/Event.java b/src/main/java/com/google/devtools/build/lib/events/Event.java
index 18f9c5d..9d764ed 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Event.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Event.java
@@ -28,6 +28,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
+import java.util.function.Function;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@@ -411,6 +412,22 @@
}
/**
+ * Converts a list of {@link SyntaxError}s to events, each with a specified property, and replays
+ * them on {@code handler}.
+ */
+ public static <T> void replayEventsOn(
+ EventHandler handler,
+ List<SyntaxError> errors,
+ Class<T> propertyType,
+ Function<SyntaxError, T> toProperty) {
+ for (SyntaxError error : errors) {
+ handler.handle(
+ Event.error(error.location(), error.message())
+ .withProperty(propertyType, toProperty.apply(error)));
+ }
+ }
+
+ /**
* Returns a {@link StarlarkThread.PrintHandler} that sends {@link EventKind#DEBUG} events to the
* provided {@link EventHandler}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
index 21cbb7a..587d626 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.syntax.Location;
import java.util.ArrayList;
import java.util.Collections;
@@ -129,16 +130,22 @@
for (Label environment :
Iterables.filter(environmentLabels.environments, new DifferentPackage(containingPackage))) {
events.add(
- Event.error(
+ Package.error(
location,
- environment + " is not in the same package as group " + environmentLabels.label));
+ environment + " is not in the same package as group " + environmentLabels.label,
+ Code.ENVIRONMENT_IN_DIFFERENT_PACKAGE));
}
// The defaults must be a subset of the member environments.
for (Label unknownDefault :
Sets.difference(environmentLabels.defaults, environmentLabels.environments)) {
- events.add(Event.error(location, "default " + unknownDefault + " is not a "
- + "declared environment for group " + getLabel()));
+ events.add(
+ Package.error(
+ location,
+ String.format(
+ "default %s is not a declared environment for group %s",
+ unknownDefault, getLabel()),
+ Code.DEFAULT_ENVIRONMENT_UNDECLARED));
}
return events;
@@ -213,13 +220,25 @@
private boolean isValidEnvironment(Target env, Label envName, String prefix, List<Event> events) {
if (env == null) {
- events.add(Event.error(location, prefix + "environment " + envName + " does not exist"));
+ events.add(
+ Package.error(
+ location,
+ prefix + "environment " + envName + " does not exist",
+ Code.ENVIRONMENT_DOES_NOT_EXIST));
return false;
} else if (!env.getTargetKind().equals("environment rule")) {
- events.add(Event.error(location, prefix + env.getLabel() + " is not a valid environment"));
+ events.add(
+ Package.error(
+ location,
+ prefix + env.getLabel() + " is not a valid environment",
+ Code.ENVIRONMENT_INVALID));
return false;
} else if (!environmentLabels.environments.contains(env.getLabel())) {
- events.add(Event.error(location, prefix + env.getLabel() + " is not a member of this group"));
+ events.add(
+ Package.error(
+ location,
+ prefix + env.getLabel() + " is not a member of this group",
+ Code.ENVIRONMENT_NOT_IN_GROUP));
return false;
}
return true;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 764edef..20c8635 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -36,11 +36,15 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
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.PackageLoading;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
@@ -49,6 +53,7 @@
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
+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 com.google.devtools.build.lib.vfs.Root;
@@ -172,6 +177,12 @@
*/
private boolean containsErrors;
+ /**
+ * The first detailed error encountered during this package's construction and evaluation, or
+ * {@code null} if there were no such errors or all its errors lacked details.
+ */
+ @Nullable private FailureDetail failureDetail;
+
/** The list of transitive closure of the Starlark file dependencies. */
private ImmutableList<Label> starlarkFileDependencies;
@@ -432,6 +443,7 @@
}
this.buildFile = builder.buildFile;
this.containsErrors = builder.containsErrors;
+ this.failureDetail = builder.getFailureDetail();
this.starlarkFileDependencies = builder.starlarkFileDependencies;
this.defaultLicense = builder.defaultLicense;
this.defaultDistributionSet = builder.defaultDistributionSet;
@@ -530,6 +542,15 @@
return containsErrors;
}
+ /**
+ * Returns the first {@link FailureDetail} describing one of the package's errors, or {@code null}
+ * if it has no errors or all its errors lack details.
+ */
+ @Nullable
+ public FailureDetail getFailureDetail() {
+ return failureDetail;
+ }
+
/** Returns an (immutable, ordered) view of all the targets belonging to this package. */
public ImmutableSortedKeyMap<String, Target> getTargets() {
return targets;
@@ -781,6 +802,24 @@
}
/**
+ * Returns an error {@link Event} with {@link Location} and {@link DetailedExitCode} properties.
+ */
+ public static Event error(Location location, String message, Code code) {
+ Event error = Event.error(location, message);
+ // The DetailedExitCode's message is the base event's toString because that string nicely
+ // includes the location value.
+ return error.withProperty(DetailedExitCode.class, createDetailedCode(error.toString(), code));
+ }
+
+ public static DetailedExitCode createDetailedCode(String errorMessage, Code code) {
+ return DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage(errorMessage)
+ .setPackageLoading(PackageLoading.newBuilder().setCode(code))
+ .build());
+ }
+
+ /**
* A builder for {@link Package} objects. Only intended to be used by {@link PackageFactory} and
* {@link com.google.devtools.build.lib.skyframe.PackageFunction}.
*/
@@ -858,7 +897,19 @@
private final List<Postable> posts = Lists.newArrayList();
@Nullable private String ioExceptionMessage = null;
@Nullable private IOException ioException = null;
+ @Nullable private DetailedExitCode ioExceptionDetailedExitCode = null;
private boolean containsErrors = false;
+ // A package's FailureDetail field derives from its Builder's events. During package
+ // deserialization, those events are unavailable, because those events aren't serialized [*].
+ // Its FailureDetail value is serialized, however. During deserialization, that value is
+ // assigned here, so that it can be assigned to the deserialized package.
+ //
+ // Likewise, during workspace part assembly, errors from parent parts should propagate to their
+ // children.
+ //
+ // [*] Not in the context of the package, anyway. Skyframe values containing a package may
+ // serialize events emitted during its construction/evaluation.
+ @Nullable private FailureDetail failureDetailOverride = null;
private ImmutableList<Label> defaultApplicableLicenses = ImmutableList.of();
private License defaultLicense = License.NO_LICENSE;
@@ -1145,9 +1196,10 @@
return this;
}
- Builder setIOExceptionAndMessage(IOException e, String message) {
+ Builder setIOException(IOException e, String message, DetailedExitCode detailedExitCode) {
this.ioException = e;
this.ioExceptionMessage = message;
+ this.ioExceptionDetailedExitCode = detailedExitCode;
return setContainsErrors();
}
@@ -1182,6 +1234,28 @@
return this;
}
+ public void setFailureDetailOverride(FailureDetail failureDetail) {
+ failureDetailOverride = failureDetail;
+ }
+
+ @Nullable
+ public FailureDetail getFailureDetail() {
+ if (failureDetailOverride != null) {
+ return failureDetailOverride;
+ }
+
+ for (Event event : this.events) {
+ if (event.getKind() != EventKind.ERROR) {
+ continue;
+ }
+ DetailedExitCode detailedExitCode = event.getProperty(DetailedExitCode.class);
+ if (detailedExitCode != null && detailedExitCode.getFailureDetail() != null) {
+ return detailedExitCode.getFailureDetail();
+ }
+ }
+ return null;
+ }
+
Builder setStarlarkFileDependencies(ImmutableList<Label> starlarkFileDependencies) {
this.starlarkFileDependencies = starlarkFileDependencies;
return this;
@@ -1231,8 +1305,8 @@
/**
* Sets the default value to use for a rule's {@link RuleClass#COMPATIBLE_ENVIRONMENT_ATTR}
- * attribute when not explicitly specified by the rule. Records a package error if
- * any labels are duplicated.
+ * attribute when not explicitly specified by the rule. Records a package error if any labels
+ * are duplicated.
*/
void setDefaultCompatibleWith(List<Label> environments, String attrName, Location location) {
if (hasDuplicateLabels(
@@ -1427,8 +1501,12 @@
EventHandler eventHandler) {
Set<Label> dupes = CollectionUtils.duplicatedElementsOf(labels);
for (Label dupe : dupes) {
- eventHandler.handle(Event.error(location, String.format(
- "label '%s' is duplicated in the '%s' list of '%s'", dupe, attrName, owner)));
+ eventHandler.handle(
+ error(
+ location,
+ String.format(
+ "label '%s' is duplicated in the '%s' list of '%s'", dupe, attrName, owner),
+ Code.DUPLICATE_LABEL));
}
return !dupes.isEmpty();
}
@@ -1467,8 +1545,13 @@
for (Label environment : group.getEnvironments()) {
EnvironmentGroup otherGroup = environmentGroups.get(environment);
if (otherGroup != null) {
- eventHandler.handle(Event.error(location, "environment " + environment + " belongs to"
- + " both " + group.getLabel() + " and " + otherGroup.getLabel()));
+ eventHandler.handle(
+ error(
+ location,
+ String.format(
+ "environment %s belongs to both %s and %s",
+ environment, group.getLabel(), otherGroup.getLabel()),
+ Code.ENVIRONMENT_IN_MULTIPLE_GROUPS));
setContainsErrors();
} else {
environmentGroups.put(environment, group);
@@ -1518,7 +1601,8 @@
Preconditions.checkNotNull(buildFileLabel);
Preconditions.checkNotNull(makeEnv);
if (ioException != null) {
- throw new NoSuchPackageException(getPackageIdentifier(), ioExceptionMessage, ioException);
+ throw new NoSuchPackageException(
+ getPackageIdentifier(), ioExceptionMessage, ioException, ioExceptionDetailedExitCode);
}
// We create the original BUILD InputFile when the package filename is set; however, the
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 8d86c01..02825ae 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
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.syntax.Argument;
import com.google.devtools.build.lib.syntax.CallExpression;
import com.google.devtools.build.lib.syntax.DefStatement;
@@ -991,7 +992,11 @@
// Report scan/parse errors.
if (!file.ok()) {
- Event.replayEventsOn(pkgContext.eventHandler, file.errors());
+ Event.replayEventsOn(
+ pkgContext.eventHandler,
+ file.errors(),
+ DetailedExitCode.class,
+ syntaxError -> Package.createDetailedCode(syntaxError.toString(), Code.SYNTAX_ERROR));
return false;
}
@@ -1000,7 +1005,8 @@
// after we've parsed the BUILD file and created the Package.
String error = LabelValidator.validatePackageName(packageId.getPackageFragment().toString());
if (error != null) {
- pkgContext.eventHandler.handle(Event.error(file.getStartLocation(), error));
+ pkgContext.eventHandler.handle(
+ Package.error(file.getStartLocation(), error, Code.PACKAGE_NAME_INVALID));
return false;
}
@@ -1022,7 +1028,11 @@
try {
prog = Program.compileFile(file, module);
} catch (SyntaxError.Exception ex) {
- Event.replayEventsOn(pkgContext.eventHandler, ex.errors());
+ Event.replayEventsOn(
+ pkgContext.eventHandler,
+ ex.errors(),
+ DetailedExitCode.class,
+ syntaxError -> Package.createDetailedCode(syntaxError.toString(), Code.SYNTAX_ERROR));
return false;
}
@@ -1082,7 +1092,8 @@
try {
Starlark.execFileProgram(prog, module, thread);
} catch (EvalException ex) {
- pkgContext.eventHandler.handle(Event.error(null, ex.getMessageWithStack()));
+ pkgContext.eventHandler.handle(
+ Package.error(null, ex.getMessageWithStack(), Code.STARLARK_EVAL_ERROR));
return false;
}
@@ -1119,7 +1130,7 @@
NodeVisitor checker =
new NodeVisitor() {
void error(Location loc, String message) {
- eventHandler.handle(Event.error(loc, message));
+ eventHandler.handle(Package.error(loc, message, Code.SYNTAX_ERROR));
success[0] = false;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
index a30657c..ca1aa12 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
@@ -18,10 +18,10 @@
import com.google.common.collect.ImmutableList;
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.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.syntax.Location;
import java.util.Collection;
import java.util.Collections;
@@ -53,7 +53,7 @@
this.containingPackage = pkg;
this.includes = ImmutableList.copyOf(includes);
- // TODO(bazel-team): Consider refactoring so constructor takes a PackageGroupContents.
+ // TODO(bazel-team): Consider refactoring so constructor takes a PackageGroupContents.
ImmutableList.Builder<PackageSpecification> packagesBuilder = ImmutableList.builder();
for (String packageSpecification : packageSpecifications) {
PackageSpecification specification = null;
@@ -63,7 +63,8 @@
label.getPackageIdentifier().getRepository(), packageSpecification);
} catch (PackageSpecification.InvalidPackageSpecificationException e) {
containsErrors = true;
- eventHandler.handle(Event.error(location, e.getMessage()));
+ eventHandler.handle(
+ Package.error(location, e.getMessage(), Code.INVALID_PACKAGE_SPECIFICATION));
}
if (specification != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java
index c7f993d..3fe9883 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java
@@ -19,11 +19,11 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.collect.nestedset.Depset;
-import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Location;
@@ -296,7 +296,8 @@
License license = BuildType.LICENSE.convert(licensesList, "'licenses' operand");
context.pkgBuilder.setDefaultLicense(license);
} catch (ConversionException e) {
- context.eventHandler.handle(Event.error(thread.getCallerLocation(), e.getMessage()));
+ context.eventHandler.handle(
+ Package.error(thread.getCallerLocation(), e.getMessage(), Code.LICENSE_PARSE_FAILURE));
context.pkgBuilder.setContainsErrors();
}
return Starlark.NONE;
@@ -320,7 +321,9 @@
BuildType.DISTRIBUTIONS.convert(object, "'distribs' operand");
context.pkgBuilder.setDefaultDistribs(distribs);
} catch (ConversionException e) {
- context.eventHandler.handle(Event.error(thread.getCallerLocation(), e.getMessage()));
+ context.eventHandler.handle(
+ Package.error(
+ thread.getCallerLocation(), e.getMessage(), Code.DISTRIBUTIONS_PARSE_FAILURE));
context.pkgBuilder.setContainsErrors();
}
return Starlark.NONE;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
index 975a592..34caa41 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkNativeModuleApi;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -42,6 +43,7 @@
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.StarlarkValue;
import com.google.devtools.build.lib.syntax.Tuple;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@@ -110,8 +112,9 @@
excludes.isEmpty() ? "" : " - [" + Joiner.on(", ").join(excludes) + "]",
e.getMessage());
Location loc = thread.getCallerLocation();
- context.eventHandler.handle(Event.error(loc, errorMessage));
- context.pkgBuilder.setIOExceptionAndMessage(e, errorMessage);
+ Event error = Package.error(loc, errorMessage, Code.GLOB_IO_EXCEPTION);
+ context.eventHandler.handle(error);
+ context.pkgBuilder.setIOException(e, errorMessage, error.getProperty(DetailedExitCode.class));
matches = ImmutableList.of();
} catch (BadGlobException e) {
throw new EvalException(e);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index d0eec4d..dadcacd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Module;
@@ -192,7 +193,8 @@
try {
Starlark.execFileProgram(prog, module, thread);
} catch (EvalException ex) {
- localReporter.handle(Event.error(null, ex.getMessageWithStack()));
+ localReporter.handle(
+ Package.error(null, ex.getMessageWithStack(), Code.STARLARK_EVAL_ERROR));
}
}
@@ -232,6 +234,9 @@
if (aPackage.containsErrors()) {
builder.setContainsErrors();
}
+ if (aPackage.getFailureDetail() != null) {
+ builder.setFailureDetailOverride(aPackage.getFailureDetail());
+ }
builder.addRegisteredExecutionPlatforms(aPackage.getRegisteredExecutionPlatforms());
builder.addRegisteredToolchains(aPackage.getRegisteredToolchains());
builder.addRepositoryMappings(aPackage);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
index da42901..073bd11 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
@@ -477,7 +477,6 @@
* QueryException} or emitting an event, depending on whether the evaluation is running in a "keep
* going" mode.
*/
- // TODO(b/138456686): make detailedExitCode non-nullable
void handleError(
QueryExpression expression, String message, @Nullable DetailedExitCode detailedExitCode)
throws QueryException;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
index 181ff3c..3824acc 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
@@ -323,7 +323,7 @@
handleError(
caller,
"errors were encountered while computing transitive closure",
- /*detailedExitCode=*/ null);
+ errorObserver.getDetailedExitCode());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java b/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
index eab4435..370d5e4 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/ErrorPrintingTargetEdgeErrorObserver.java
@@ -21,10 +21,11 @@
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.util.DetailedExitCode;
/**
* Record errors, such as missing package/target or rules containing errors, encountered during
- * visitation. Emit an error message upon encountering missing edges
+ * visitation. Emit an error message upon encountering missing edges.
*
* <p>The accessor {@link #hasErrors()}) may not be called until the concurrent phase is over, i.e.
* all external calls to visit() methods have completed.
@@ -43,9 +44,12 @@
@ThreadSafety.ThreadSafe
@Override
public void missingEdge(Target target, Label label, NoSuchThingException e) {
+ DetailedExitCode detailedExitCode = e.getDetailedExitCode();
eventHandler.handle(
Event.error(
- TargetUtils.getLocationMaybe(target), TargetUtils.formatMissingEdge(target, label, e)));
+ TargetUtils.getLocationMaybe(target),
+ TargetUtils.formatMissingEdge(target, label, e))
+ .withProperty(DetailedExitCode.class, detailedExitCode));
super.missingEdge(target, label, e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
index bf2f887..4406229 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
@@ -230,7 +230,7 @@
handleError(
caller,
"errors were encountered while computing transitive closure",
- /*detailedExitCode=*/ null);
+ errorObserver.getDetailedExitCode());
}
callback.process(result);
}
@@ -306,7 +306,7 @@
handleError(
caller,
"errors were encountered while computing transitive closure",
- /*detailedExitCode=*/ null);
+ errorObserver.getDetailedExitCode());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java b/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
index 725ffcc..cadea50 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/TargetEdgeErrorObserver.java
@@ -21,6 +21,10 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.TargetEdgeObserver;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.annotation.Nullable;
/**
* Record errors, such as missing package/target or rules containing errors, encountered during
@@ -41,6 +45,8 @@
*/
private volatile boolean hasErrors = false;
+ private final AtomicReference<DetailedExitCode> errorCode = new AtomicReference<>();
+
/**
* Reports an unresolved label error and records the fact that an error was encountered.
*
@@ -52,6 +58,7 @@
@Override
public void missingEdge(Target target, Label label, NoSuchThingException e) {
hasErrors = true;
+ errorCode.compareAndSet(/*expect=*/ null, /*update=*/ e.getDetailedExitCode());
}
/**
@@ -67,6 +74,12 @@
return hasErrors;
}
+ /** Returns the first {@link DetailedExitCode} encountered, or {@code null} if there were none. */
+ @Nullable
+ public DetailedExitCode getDetailedExitCode() {
+ return errorCode.get();
+ }
+
@Override
public void edge(Target from, Attribute attribute, Target to) {
// No-op.
@@ -76,7 +89,11 @@
public void node(Target node) {
if (node.getPackage().containsErrors()
|| ((node instanceof Rule) && ((Rule) node).containsErrors())) {
- this.hasErrors = true; // Note, this is thread-safe.
+ this.hasErrors = true;
+ FailureDetail failureDetail = node.getPackage().getFailureDetail();
+ if (failureDetail != null) {
+ errorCode.compareAndSet(/*expect=*/ null, /*update=*/ DetailedExitCode.of(failureDetail));
+ }
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 7273a1e..f92fb2d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -58,6 +58,7 @@
import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper;
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.skyframe.BzlLoadFunction.BzlLoadFailedException;
import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -854,7 +855,7 @@
String message =
ContainingPackageLookupValue.getErrorMessageForLabelCrossingPackageBoundary(
pkgRoot, label, containingPkgLookupValue);
- pkgBuilder.addEvent(Event.error(location, message));
+ pkgBuilder.addEvent(Package.error(location, message, Code.LABEL_CROSSES_PACKAGE_BOUNDARY));
return true;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
index 4de40d1..50a0330 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader;
import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.ErrorInfo;
import com.google.devtools.build.skyframe.EvaluationResult;
@@ -89,8 +90,7 @@
if (!keepGoing) {
throw e;
}
- eventHandler.handle(
- Event.error("Skipping '" + patternLookup.pattern + "': " + e.getMessage()));
+ eventHandler.handle(createPatternParsingError(e, patternLookup.pattern));
eventHandler.post(PatternExpandingError.skipped(patternLookup.pattern, e.getMessage()));
resultBuilder.put(patternLookup.pattern, ImmutableSet.of());
}
@@ -126,7 +126,7 @@
throw new IllegalStateException(error.toString());
}
if (keepGoing) {
- eventHandler.handle(Event.error("Skipping '" + rawPattern + "': " + errorMessage));
+ eventHandler.handle(createPatternParsingError(targetParsingException, rawPattern));
eventHandler.post(PatternExpandingError.skipped(rawPattern, errorMessage));
} else {
eventHandler.post(PatternExpandingError.failed(patternLookup.pattern, errorMessage));
@@ -158,7 +158,7 @@
if (!keepGoing) {
throw e;
}
- eventHandler.handle(Event.error("Skipping '" + targetPattern + "': " + e.getMessage()));
+ eventHandler.handle(createPatternParsingError(e, targetPattern));
return null;
}
}
@@ -179,6 +179,11 @@
throw new AssertionError();
}
+ private static Event createPatternParsingError(TargetParsingException e, String pattern) {
+ return Event.error("Skipping '" + pattern + "': " + e.getMessage())
+ .withProperty(DetailedExitCode.class, e.getDetailedExitCode());
+ }
+
private abstract static class PatternLookup {
protected final String pattern;
@Nullable private final SkyKey skyKey;
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index 4b27892..533df4b 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -1115,6 +1115,21 @@
PACKAGE_MISSING = 11 [(metadata) = { exit_code: 1 }];
TARGET_MISSING = 12 [(metadata) = { exit_code: 1 }];
NO_SUCH_THING = 13 [(metadata) = { exit_code: 1 }];
+ GLOB_IO_EXCEPTION = 14 [(metadata) = { exit_code: 1 }];
+ DUPLICATE_LABEL = 15 [(metadata) = { exit_code: 1 }];
+ INVALID_PACKAGE_SPECIFICATION = 16 [(metadata) = { exit_code: 1 }];
+ SYNTAX_ERROR = 17 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_IN_DIFFERENT_PACKAGE = 18 [(metadata) = { exit_code: 1 }];
+ DEFAULT_ENVIRONMENT_UNDECLARED = 19 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_IN_MULTIPLE_GROUPS = 20 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_DOES_NOT_EXIST = 21 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_INVALID = 22 [(metadata) = { exit_code: 1 }];
+ ENVIRONMENT_NOT_IN_GROUP = 23 [(metadata) = { exit_code: 1 }];
+ PACKAGE_NAME_INVALID = 24 [(metadata) = { exit_code: 1 }];
+ STARLARK_EVAL_ERROR = 25 [(metadata) = { exit_code: 1 }];
+ LICENSE_PARSE_FAILURE = 26 [(metadata) = { exit_code: 1 }];
+ DISTRIBUTIONS_PARSE_FAILURE = 27 [(metadata) = { exit_code: 1 }];
+ LABEL_CROSSES_PACKAGE_BOUNDARY = 28 [(metadata) = { exit_code: 1 }];
}
Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
index 558e4b0..48cecfd 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
@@ -54,6 +54,7 @@
"//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/vfs",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/query2/testutil",
"//third_party:guava",
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
index e091461..688550c 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
import com.google.devtools.build.lib.query2.engine.QueryException;
+import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery.Code;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.vfs.Path;
import java.util.Collection;
@@ -163,7 +164,9 @@
String targetConfiguration = myRule.getConfigurationChecksum();
// Test that the proper error is thrown when requesting an attribute that doesn't exist.
- assertThat(evalThrows("labels('fake_attr', //test:my_rule)", true))
+ EvalThrowsResult evalThrowsResult = evalThrows("labels('fake_attr', //test:my_rule)", true);
+ assertConfigurableQueryCode(evalThrowsResult.getFailureDetail(), Code.ATTRIBUTE_MISSING);
+ assertThat(evalThrowsResult.getMessage())
.isEqualTo(
String.format(
"in 'fake_attr' of rule //test:my_rule: ConfiguredTarget(//test:my_rule, %s) "
@@ -336,10 +339,14 @@
getHelper().setWholeTestUniverseScope("test:my_rule");
assertThat(eval("config(//test:target_dep, target)")).isEqualTo(eval("//test:target_dep"));
- assertThat(evalThrows("config(//test:host_dep, target)", true))
+ EvalThrowsResult hostResult = evalThrows("config(//test:host_dep, target)", true);
+ assertThat(hostResult.getMessage())
.isEqualTo("No target (in) //test:host_dep could be found in the 'target' configuration");
- assertThat(evalThrows("config(//test:exec_dep, target)", true))
+ assertConfigurableQueryCode(hostResult.getFailureDetail(), Code.TARGET_MISSING);
+ EvalThrowsResult execResult = evalThrows("config(//test:exec_dep, target)", true);
+ assertThat(execResult.getMessage())
.isEqualTo("No target (in) //test:exec_dep could be found in the 'target' configuration");
+ assertConfigurableQueryCode(execResult.getFailureDetail(), Code.TARGET_MISSING);
BuildConfiguration configuration =
getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, target)")));
@@ -356,11 +363,15 @@
getHelper().setWholeTestUniverseScope("test:my_rule");
- assertThat(evalThrows("config(//test:target_dep, host)", true))
+ EvalThrowsResult targetResult = evalThrows("config(//test:target_dep, host)", true);
+ assertThat(targetResult.getMessage())
.isEqualTo("No target (in) //test:target_dep could be found in the 'host' configuration");
+ assertConfigurableQueryCode(targetResult.getFailureDetail(), Code.TARGET_MISSING);
assertThat(eval("config(//test:host_dep, host)")).isEqualTo(eval("//test:host_dep"));
- assertThat(evalThrows("config(//test:exec_dep, host)", true))
+ EvalThrowsResult hostResult = evalThrows("config(//test:exec_dep, host)", true);
+ assertThat(hostResult.getMessage())
.isEqualTo("No target (in) //test:exec_dep could be found in the 'host' configuration");
+ assertConfigurableQueryCode(hostResult.getFailureDetail(), Code.TARGET_MISSING);
BuildConfiguration configuration =
getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, host)")));
@@ -448,6 +459,7 @@
QueryException e =
assertThrows(
QueryException.class, () -> eval("config(//mytest:mytarget," + wrongPrefix + ")"));
+ assertConfigurableQueryCode(e.getFailureDetail(), Code.INCORRECT_CONFIG_ARGUMENT_ERROR);
assertThat(e)
.hasMessageThat()
.contains("config()'s second argument must identify a unique configuration");
@@ -478,7 +490,7 @@
helper.setKeepGoing(false);
getHelper().turnOffFailFast();
- assertThat(evalThrows("//parent/...", true))
+ assertThat(evalThrows("//parent/...", true).getMessage())
.isEqualTo(
"no such package 'parent/child': Symlink cycle detected while trying to "
+ "find BUILD file /workspace/parent/child/BUILD");
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
index 6e80330..591b604 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.graph.LabelSerializer;
import com.google.devtools.build.lib.graph.Node;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
import com.google.devtools.build.lib.query2.engine.DigraphQueryEvalResult;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
@@ -43,6 +44,10 @@
import com.google.devtools.build.lib.query2.engine.QueryEvalResult;
import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.query2.testutil.AbstractQueryTest.QueryHelper.ResultAndTargets;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
+import com.google.devtools.build.lib.server.FailureDetails.Query;
+import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.vfs.Path;
@@ -167,15 +172,39 @@
return result.getResultSet();
}
- // Like eval(), but asserts that evaluation completes abruptly with a
- // QueryException, whose message is returned.
- protected String evalThrows(String query, boolean unconditionallyThrows) throws Exception {
+ // Like eval(), but asserts that evaluation completes abruptly with a QueryException, whose
+ // message and FailureDetail is returned.
+ protected EvalThrowsResult evalThrows(String query, boolean unconditionallyThrows)
+ throws Exception {
try {
helper.evaluateQuery(query);
fail("evaluateQuery completed normally: " + query);
- return null; // unreachable
+ throw new IllegalStateException();
} catch (QueryException e) {
- return e.getCause() != null ? e.getCause().getMessage() : e.getMessage();
+ String message = e.getCause() != null ? e.getCause().getMessage() : e.getMessage();
+ return new EvalThrowsResult(message, e.getFailureDetail());
+ }
+ }
+
+ /**
+ * Error message and {@link FailureDetail} from the failing query evaluation performed by {@link
+ * #evalThrows}.
+ */
+ protected static class EvalThrowsResult {
+ private final String message;
+ private final FailureDetail failureDetail;
+
+ protected EvalThrowsResult(String message, FailureDetail failureDetail) {
+ this.message = message;
+ this.failureDetail = failureDetail;
+ }
+
+ public String getMessage() {
+ return message;
+ }
+
+ public FailureDetail getFailureDetail() {
+ return failureDetail;
}
}
@@ -206,10 +235,32 @@
assertThat(x.containsAll(y)).isFalse();
}
+ protected static void assertPackageLoadingCode(ResultAndTargets<Target> result, Code code) {
+ FailureDetail failureDetail =
+ result.getQueryEvalResult().getDetailedExitCode().getFailureDetail();
+ assertThat(failureDetail).isNotNull();
+ assertPackageLoadingCode(failureDetail, code);
+ }
+
+ protected static void assertPackageLoadingCode(FailureDetail failureDetail, Code code) {
+ assertThat(failureDetail.getPackageLoading().getCode()).isEqualTo(code);
+ }
+
+ protected static void assertQueryCode(ResultAndTargets<Target> result, Query.Code code) {
+ FailureDetail failureDetail =
+ result.getQueryEvalResult().getDetailedExitCode().getFailureDetail();
+ assertThat(failureDetail).isNotNull();
+ assertQueryCode(failureDetail, code);
+ }
+
+ protected static void assertQueryCode(FailureDetail failureDetail, Query.Code code) {
+ assertThat(failureDetail.getQuery().getCode()).isEqualTo(code);
+ }
+
@Test
public void testTargetLiteralWithMissingTargets() throws Exception {
writeFile("a/BUILD");
- assertThat(evalThrows("//a:b", false))
+ assertThat(evalThrows("//a:b", false).getMessage())
.isEqualTo(
"no such target '//a:b': target 'b' not declared in package 'a' "
+ "defined by "
@@ -239,8 +290,16 @@
@Test
public void testBadTargetLiterals() throws Exception {
- assertThat(evalThrows("bad:*:*", false))
- .isEqualTo("Invalid package name 'bad:*': " + BAD_PACKAGE_NAME);
+ runBadTargetLiteralsTest(true);
+ }
+
+ protected void runBadTargetLiteralsTest(boolean checkDetailedCode) throws Exception {
+ EvalThrowsResult result = evalThrows("bad:*:*", false);
+ if (checkDetailedCode) {
+ assertThat(result.getFailureDetail().getTargetPatterns().getCode())
+ .isEqualTo(TargetPatterns.Code.LABEL_SYNTAX_ERROR);
+ }
+ assertThat(result.getMessage()).isEqualTo("Invalid package name 'bad:*': " + BAD_PACKAGE_NAME);
}
@Test
@@ -391,7 +450,9 @@
assertContains(eval("c:*"), eval("some(c:*)"));
assertThat(evalToString("some(//c:q)")).isEqualTo("//c:q");
- assertThat(evalThrows("some(//c:q intersect //c:p)", true)).isEqualTo("argument set is empty");
+ EvalThrowsResult result = evalThrows("some(//c:q intersect //c:p)", true);
+ assertThat(result.getMessage()).isEqualTo("argument set is empty");
+ assertQueryCode(result.getFailureDetail(), Query.Code.ARGUMENTS_MISSING);
}
protected void writeBuildFiles3() throws Exception {
@@ -784,7 +845,9 @@
assertContains(
eval("//b + //c + //d"),
eval("let x = //a in deps($x) except $x" + getDependencyCorrectionWithGen()));
- assertThat(evalThrows("$undefined", true)).isEqualTo("undefined variable 'undefined'");
+ EvalThrowsResult result = evalThrows("$undefined", true);
+ assertThat(result.getMessage()).isEqualTo("undefined variable 'undefined'");
+ assertQueryCode(result.getFailureDetail(), Query.Code.VARIABLE_UNDEFINED);
}
@Test
@@ -944,13 +1007,20 @@
@Test
public void testCycleInStarlark() throws Exception {
+ runCycleInStarlarkTest(/*checkFailureDetail=*/ true);
+ }
+
+ protected void runCycleInStarlarkTest(boolean checkFailureDetail) throws Exception {
writeFile("a/BUILD", "load('//a:cycle1.bzl', 'C1')", "sh_library(name = 'a')");
writeFile("a/cycle1.bzl", "load('//a:cycle2.bzl', 'C2')", "C1 = struct()");
writeFile("a/cycle2.bzl", "load('//a:cycle1.bzl', 'C1')", "C2 = struct()");
- try {
- evalThrows("//a:all", false);
- } catch (QueryException e) {
- // Expected.
+ EvalThrowsResult result = evalThrows("//a:all", false);
+ // TODO(mschaller): evalThrows's message can be non-deterministic if events are too. It probably
+ // needs to be refactored to deal with underlying event non-determinism, because fixing query
+ // engines' event non-determinism is probably hard.
+ if (checkFailureDetail) {
+ assertThat(result.getFailureDetail().getTargetPatterns().getCode())
+ .isEqualTo(TargetPatterns.Code.CYCLE);
}
}
@@ -1075,9 +1145,11 @@
writeFile("c/BUILD", "test_suite(name='c', tests=['//d'])");
writeFile("d/BUILD");
+ EvalThrowsResult result = evalThrows("tests(//c)", false);
assertStartsWith(
"couldn't expand 'tests' attribute of test_suite //c:c: " + "no such target '//d:d'",
- evalThrows("tests(//c)", false));
+ result.getMessage());
+ assertPackageLoadingCode(result.getFailureDetail(), Code.TARGET_MISSING);
}
@Test
@@ -1199,7 +1271,7 @@
writeFile("x/BUILD", "cc_library(name='x', srcs=['a.cc', 'a.cc'])");
String expectedError = "Label '//x:a.cc' is duplicated in the 'srcs' attribute of rule 'x'";
if (helper.isKeepGoing()) {
- assertThat(evalThrows("//x", false)).isEqualTo(expectedError);
+ assertThat(evalThrows("//x", false).getMessage()).isEqualTo(expectedError);
} else {
evalThrows("//x", false);
assertContainsEvent(expectedError);
@@ -1353,10 +1425,12 @@
public void testStrictTestSuiteWithFile() throws Exception {
helper.setQuerySettings(Setting.TESTS_EXPRESSION_STRICT);
writeFile("x/BUILD", "test_suite(name='a', tests=['a.txt'])");
- assertThat(evalThrows("tests(//x:a)", false))
+ EvalThrowsResult result = evalThrows("tests(//x:a)", false);
+ assertThat(result.getMessage())
.isEqualTo(
"The label '//x:a.txt' in the test_suite '//x:a' does not refer to a test or "
+ "test_suite rule!");
+ assertQueryCode(result.getFailureDetail(), Query.Code.INVALID_LABEL_IN_TEST_SUITE);
}
@Test
@@ -1430,10 +1504,13 @@
// When the query environment is queried for targets belonging to packages beneath the
// package "a/b", which doesn't exist,
String missingPackage = "a/b";
- String s = evalThrows("//" + missingPackage + "/...", false);
+ EvalThrowsResult result = evalThrows("//" + missingPackage + "/...", false);
+ String s = result.getMessage();
// Then an exception is thrown that says that the pattern matched nothing.
assertThat(s).containsMatch("no targets found beneath '" + missingPackage + "'");
+ assertThat(result.getFailureDetail().getTargetPatterns().getCode())
+ .isEqualTo(TargetPatterns.Code.TARGETS_MISSING);
}
@Test
@@ -2050,3 +2127,4 @@
String getLabel(T target);
}
}
+
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
index 3aec7e5..93bcc69 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
@@ -38,6 +38,8 @@
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
import com.google.devtools.build.lib.query2.engine.QueryExpression;
import com.google.devtools.build.lib.query2.engine.QueryParser;
+import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.Collections;
@@ -101,9 +103,10 @@
}
@Override
- protected String evalThrows(String query, boolean unconditionallyThrows) throws Exception {
+ protected EvalThrowsResult evalThrows(String query, boolean unconditionallyThrows)
+ throws Exception {
maybeParseUniverseScope(query);
- String queryResult = super.evalThrows(query, unconditionallyThrows);
+ EvalThrowsResult queryResult = super.evalThrows(query, unconditionallyThrows);
if (!getHelper().isWholeTestUniverse()) {
helper.setUniverseScope(getDefaultUniverseScope());
}
@@ -165,7 +168,8 @@
@Test
public void testBadTargetLiterals() throws Exception {
getHelper().turnOffFailFast();
- super.testBadTargetLiterals();
+ // Post-analysis query test infrastructure clobbers certain detailed failures.
+ runBadTargetLiteralsTest(/*checkDetailedCode=*/ false);
}
@Override
@@ -436,22 +440,26 @@
@Test
public void testVisibleFunctionDoesNotWork() throws Exception {
writeSimpleTarget();
- assertThat(evalThrows("visible(//test:target, //test:*)", true))
- .isEqualTo("visible() is not supported on configured targets");
+ EvalThrowsResult result = evalThrows("visible(//test:target, //test:*)", true);
+ assertThat(result.getMessage()).isEqualTo("visible() is not supported on configured targets");
+ assertConfigurableQueryCode(result.getFailureDetail(), Code.VISIBLE_FUNCTION_NOT_SUPPORTED);
}
@Test
public void testSiblingsFunctionDoesNotWork() throws Exception {
writeSimpleTarget();
- assertThat(evalThrows("siblings(//test:target)", true))
- .isEqualTo("siblings() not supported for post analysis queries");
+ EvalThrowsResult result = evalThrows("siblings(//test:target)", true);
+ assertThat(result.getMessage()).isEqualTo("siblings() not supported for post analysis queries");
+ assertConfigurableQueryCode(result.getFailureDetail(), Code.SIBLINGS_FUNCTION_NOT_SUPPORTED);
}
@Test
public void testBuildfilesFunctionDoesNotWork() throws Exception {
writeSimpleTarget();
- assertThat(evalThrows("buildfiles(//test:target)", true))
+ EvalThrowsResult result = evalThrows("buildfiles(//test:target)", true);
+ assertThat(result.getMessage())
.isEqualTo("buildfiles() doesn't make sense for the configured target graph");
+ assertConfigurableQueryCode(result.getFailureDetail(), Code.BUILDFILES_FUNCTION_NOT_SUPPORTED);
}
// LabelListAttr not currently supported.
@@ -685,4 +693,8 @@
@Override
@Test
public void testDefaultVisibilityReturnedInDeps_nonEmptyDependencyFilter() throws Exception {}
+
+ protected static void assertConfigurableQueryCode(FailureDetail failureDetail, Code code) {
+ assertThat(failureDetail.getConfigurableQuery().getCode()).isEqualTo(code);
+ }
}
