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); + } }