Provide more accurate ExitCode and FailureDetails when an exception is caught during the loading phase.
In each failure instance of PackageFunctionException, a specific FailureDetail code will be used to describe the exception. As the error gets bubbled up, we have to make sure the information is not lost and is properly passed up the call chain.
RELNOTES: None.
PiperOrigin-RevId: 318929283
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index c4165ca..02b70d1 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -351,6 +351,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:configuration_phase_started_event",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_progress_receiver",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:execution_finished_event",
"//src/main/java/com/google/devtools/build/lib/skyframe:loading_phase_started_event",
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 9252143..8325367 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Interrupted.Code;
+import com.google.devtools.build.lib.skyframe.DetailedTargetParsingException;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.CrashFailureDetails;
import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -313,6 +314,9 @@
reportExceptionError(environmentPendingAbruptExitException);
result.setCatastrophe();
}
+ } catch (DetailedTargetParsingException e) {
+ detailedExitCode = e.getDetailedExitCode();
+ reportExceptionError(e);
} catch (TargetParsingException | LoadingFailedException | ViewCreationFailedException e) {
detailedExitCode = DetailedExitCode.justExitCode(ExitCode.PARSING_FAILURE);
reportExceptionError(e);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BUILD b/src/main/java/com/google/devtools/build/lib/packages/BUILD
index 03d8627..3afc9f0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/packages/BUILD
@@ -49,6 +49,7 @@
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/profiler",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi",
@@ -56,6 +57,8 @@
"//src/main/java/com/google/devtools/build/lib/syntax:evaluator",
"//src/main/java/com/google/devtools/build/lib/syntax:frontend",
"//src/main/java/com/google/devtools/build/lib/util",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/lib/vfs",
@@ -65,6 +68,7 @@
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/spelling",
"//src/main/protobuf:build_java_proto",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildFileContainsErrorsException.java b/src/main/java/com/google/devtools/build/lib/packages/BuildFileContainsErrorsException.java
index b527b92..bb2ee84 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildFileContainsErrorsException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildFileContainsErrorsException.java
@@ -15,12 +15,12 @@
package com.google.devtools.build.lib.packages;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.io.IOException;
/**
- * Exception indicating a failed attempt to access a package that could not
- * be read or had syntax errors.
+ * Exception indicating a failed attempt to access a package that could not be read or had syntax
+ * errors.
*/
public class BuildFileContainsErrorsException extends NoSuchPackageException {
@@ -39,6 +39,19 @@
super(packageIdentifier, message, cause);
}
+ public BuildFileContainsErrorsException(
+ PackageIdentifier packageIdentifier, String message, DetailedExitCode detailedExitCode) {
+ super(packageIdentifier, message, detailedExitCode);
+ }
+
+ public BuildFileContainsErrorsException(
+ PackageIdentifier packageIdentifier,
+ String message,
+ IOException cause,
+ DetailedExitCode detailedExitCode) {
+ super(packageIdentifier, message, cause, detailedExitCode);
+ }
+
@Override
public String getMessage() {
return String.format("%s '%s': %s", "error loading package", getPackageId(), getRawMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildFileNotFoundException.java b/src/main/java/com/google/devtools/build/lib/packages/BuildFileNotFoundException.java
index ca4ffed..6283874 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildFileNotFoundException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildFileNotFoundException.java
@@ -15,7 +15,7 @@
package com.google.devtools.build.lib.packages;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-
+import com.google.devtools.build.lib.util.DetailedExitCode;
import java.io.IOException;
/**
@@ -32,4 +32,9 @@
IOException cause) {
super(packageIdentifier, message, cause);
}
+
+ public BuildFileNotFoundException(
+ PackageIdentifier packageIdentifier, String message, DetailedExitCode detailedExitCode) {
+ super(packageIdentifier, message, detailedExitCode);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/InvalidPackageNameException.java b/src/main/java/com/google/devtools/build/lib/packages/InvalidPackageNameException.java
index 373eb32..9f18376 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/InvalidPackageNameException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/InvalidPackageNameException.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.packages;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.util.DetailedExitCode;
/** Exception indicating that a package name was invalid. */
public class InvalidPackageNameException extends NoSuchPackageException {
@@ -22,4 +23,9 @@
public InvalidPackageNameException(PackageIdentifier packageIdentifier, String message) {
super(packageIdentifier, message);
}
+
+ public InvalidPackageNameException(
+ PackageIdentifier packageIdentifier, String message, DetailedExitCode detailedExitCode) {
+ super(packageIdentifier, message, detailedExitCode);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java b/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java
index 1ea372e..992dbd8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java
@@ -15,25 +15,50 @@
package com.google.devtools.build.lib.packages;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.skyframe.DetailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import javax.annotation.Nullable;
/**
- * Exception indicating an attempt to access a package which is not found, does
- * not exist, or can't be parsed into a package.
+ * Exception indicating an attempt to access a package which is not found, does not exist, or can't
+ * be parsed into a package.
*
* <p>Prefer using more-specific subclasses, when appropriate.
*/
-public class NoSuchPackageException extends NoSuchThingException {
+public class NoSuchPackageException extends NoSuchThingException implements DetailedException {
+ // TODO(b/138456686): Remove Nullable and add Precondition#checkNotNull in constructor when all
+ // subclasses are instantiated with DetailedExitCode.
+ @Nullable private final DetailedExitCode detailedExitCode;
private final PackageIdentifier packageId;
public NoSuchPackageException(PackageIdentifier packageId, String message) {
super(message);
this.packageId = packageId;
+ this.detailedExitCode = null;
}
public NoSuchPackageException(PackageIdentifier packageId, String message, Exception cause) {
super(message, cause);
this.packageId = packageId;
+ this.detailedExitCode = null;
+ }
+
+ public NoSuchPackageException(
+ PackageIdentifier packageId, String message, DetailedExitCode detailedExitCode) {
+ super(message);
+ this.packageId = packageId;
+ this.detailedExitCode = detailedExitCode;
+ }
+
+ public NoSuchPackageException(
+ PackageIdentifier packageId,
+ String message,
+ Exception cause,
+ DetailedExitCode detailedExitCode) {
+ super(message, cause);
+ this.packageId = packageId;
+ this.detailedExitCode = detailedExitCode;
}
public PackageIdentifier getPackageId() {
@@ -48,4 +73,9 @@
public String getMessage() {
return String.format("%s '%s': %s", "no such package", packageId, getRawMessage());
}
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java b/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java
index f55e575..f1cf382 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java
@@ -14,10 +14,7 @@
package com.google.devtools.build.lib.packages;
-/**
- * Exception indicating an attempt to access something which is not found or
- * does not exist.
- */
+/** Exception indicating an attempt to access something which is not found or does not exist. */
public class NoSuchThingException extends Exception {
public NoSuchThingException(String message) {
@@ -27,5 +24,4 @@
public NoSuchThingException(String message, Throwable cause) {
super(message, cause);
}
-
}
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 2024fa8..70ca4e7 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
@@ -37,6 +37,8 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
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.syntax.Argument;
import com.google.devtools.build.lib.syntax.CallExpression;
import com.google.devtools.build.lib.syntax.DefStatement;
@@ -66,6 +68,8 @@
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.StringLiteral;
import com.google.devtools.build.lib.syntax.Tuple;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -670,11 +674,22 @@
long maxSteps = starlarkSemantics.maxComputationSteps();
long steps = pkg.getComputationSteps();
if (maxSteps > 0 && steps > maxSteps) {
- throw new InvalidPackageException(
- pkg.getPackageIdentifier(),
+ String message =
String.format(
"BUILD file computation took %d steps, but --max_computation_steps=%d",
- steps, maxSteps));
+ steps, maxSteps);
+ throw new InvalidPackageException(
+ pkg.getPackageIdentifier(),
+ message,
+ DetailedExitCode.of(
+ ExitCode.BUILD_FAILURE,
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setPackageLoading(
+ PackageLoading.newBuilder()
+ .setCode(PackageLoading.Code.MAX_COMPUTATION_STEPS_EXCEEDED)
+ .build())
+ .build()));
}
packageLoadingListener.onLoadingCompleteAndSuccessful(pkg, starlarkSemantics, loadTimeNanos);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageValidator.java b/src/main/java/com/google/devtools/build/lib/packages/PackageValidator.java
index eb9cd8b..b4756e4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageValidator.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageValidator.java
@@ -15,6 +15,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.DetailedExitCode;
/** Provides loaded-package validation functionality. */
public interface PackageValidator {
@@ -27,6 +28,11 @@
public InvalidPackageException(PackageIdentifier pkgId, String message) {
super(pkgId, message);
}
+
+ public InvalidPackageException(
+ PackageIdentifier pkgId, String message, DetailedExitCode detailedExitCode) {
+ super(pkgId, message, detailedExitCode);
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
index c45435b..b61e6a6 100644
--- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
@@ -679,7 +679,6 @@
"Command dispatch interrupted", Interrupted.Code.COMMAND_DISPATCH));
commandId = ""; // The default value, the client will ignore it
}
-
RunResponse.Builder response = RunResponse.newBuilder()
.setCookie(responseCookie)
.setCommandId(commandId)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 2f98e88..72ec5c66 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -142,6 +142,7 @@
":containing_package_lookup_value",
":coverage_report_value",
":cycle_utils",
+ ":detailed_exceptions",
":diff_awareness",
":diff_awareness_manager",
":directory_listing_function",
@@ -331,6 +332,19 @@
)
java_library(
+ name = "detailed_exceptions",
+ srcs = [
+ "DetailedException.java",
+ "DetailedTargetParsingException.java",
+ ],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//third_party:jsr305",
+ ],
+)
+
+java_library(
name = "abstract_label_cycle_reporter",
srcs = ["AbstractLabelCycleReporter.java"],
deps = [
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DetailedException.java b/src/main/java/com/google/devtools/build/lib/skyframe/DetailedException.java
new file mode 100644
index 0000000..a7ac7ed
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DetailedException.java
@@ -0,0 +1,29 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import javax.annotation.Nullable;
+
+/** An interface for Exceptions that will contain a {@link DetailedExitCode} */
+public interface DetailedException {
+ DetailedExitCode getDetailedExitCode();
+
+ @Nullable
+ static DetailedExitCode getDetailedExitCode(Exception exception) {
+ return exception instanceof DetailedException
+ ? ((DetailedException) exception).getDetailedExitCode()
+ : null;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DetailedTargetParsingException.java b/src/main/java/com/google/devtools/build/lib/skyframe/DetailedTargetParsingException.java
new file mode 100644
index 0000000..8ee2493
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DetailedTargetParsingException.java
@@ -0,0 +1,34 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+
+/** A {@link TargetParsingException} with {@link DetailedExitCode}. */
+public class DetailedTargetParsingException extends TargetParsingException
+ implements DetailedException {
+
+ private final DetailedExitCode detailedExitCode;
+
+ public DetailedTargetParsingException(Throwable cause, DetailedExitCode detailedExitCode) {
+ super(detailedExitCode.getFailureDetail().getMessage(), cause);
+ this.detailedExitCode = detailedExitCode;
+ }
+
+ @Override
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+}
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 322940f..fdf14ac 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
@@ -13,9 +13,12 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
-import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
@@ -53,6 +56,8 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
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.skyframe.BzlLoadFunction.BzlLoadFailedException;
import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -63,6 +68,8 @@
import com.google.devtools.build.lib.syntax.StarlarkFile;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.Statement;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -85,6 +92,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
@@ -264,8 +272,7 @@
Environment env,
boolean packageWasInError)
throws InternalInconsistentFilesystemException, FileSymlinkException, InterruptedException {
- Preconditions.checkState(
- Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
+ checkState(Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
FileSymlinkException arbitraryFse = null;
for (Map.Entry<SkyKey, ValueOrException2<IOException, BuildFileNotFoundException>> entry :
env.getValuesOrThrow(
@@ -320,11 +327,14 @@
EvalException.class,
BzlLoadFailedException.class);
} catch (IOException | EvalException | BzlLoadFailedException e) {
- throw new PackageFunctionException(
- new NoSuchPackageException(
- LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
- "Error encountered while dealing with the WORKSPACE file: " + e.getMessage()),
- Transience.PERSISTENT);
+ String message = "Error encountered while dealing with the WORKSPACE file: " + e.getMessage();
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setTransience(Transience.PERSISTENT)
+ .setPackageIdentifier(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)
+ .setMessage(message)
+ .setPackageLoadingCode(PackageLoading.Code.WORKSPACE_FILE_ERROR)
+ .build();
}
if (workspace == null) {
return null;
@@ -369,32 +379,48 @@
throw new PackageFunctionException(e, Transience.PERSISTENT);
} catch (InconsistentFilesystemException e) {
// This error is not transient from the perspective of the PackageFunction.
- throw new PackageFunctionException(
- new NoSuchPackageException(packageId, e.getMessage(), e), Transience.PERSISTENT);
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setTransience(Transience.PERSISTENT)
+ .setPackageIdentifier(packageId)
+ .setMessage(e.getMessage())
+ .setException(e)
+ .setPackageLoadingCode(PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR)
+ .build();
}
if (packageLookupValue == null) {
return null;
}
if (!packageLookupValue.packageExists()) {
+ PackageFunctionException.Builder exceptionBuilder =
+ PackageFunctionException.builder()
+ .setPackageIdentifier(packageId)
+ .setTransience(Transience.PERSISTENT);
switch (packageLookupValue.getErrorReason()) {
case NO_BUILD_FILE:
- throw new PackageFunctionException(
- new BuildFileNotFoundException(
- packageId, PackageLookupFunction.explainNoBuildFileValue(packageId, env)),
- Transience.PERSISTENT);
+ String message = PackageLookupFunction.explainNoBuildFileValue(packageId, env);
+ throw exceptionBuilder
+ .setType(PackageFunctionException.Type.BUILD_FILE_NOT_FOUND)
+ .setMessage(message)
+ .setPackageLoadingCode(PackageLoading.Code.BUILD_FILE_MISSING)
+ .build();
case DELETED_PACKAGE:
case REPOSITORY_NOT_FOUND:
- throw new PackageFunctionException(
- new BuildFileNotFoundException(packageId, packageLookupValue.getErrorMsg()),
- Transience.PERSISTENT);
+ throw exceptionBuilder
+ .setType(PackageFunctionException.Type.BUILD_FILE_NOT_FOUND)
+ .setMessage(packageLookupValue.getErrorMsg())
+ .setPackageLoadingCode(PackageLoading.Code.REPOSITORY_MISSING)
+ .build();
case INVALID_PACKAGE_NAME:
- throw new PackageFunctionException(new InvalidPackageNameException(packageId,
- packageLookupValue.getErrorMsg()), Transience.PERSISTENT);
- default:
- // We should never get here.
- throw new IllegalStateException();
+ throw exceptionBuilder
+ .setType(PackageFunctionException.Type.INVALID_PACKAGE_NAME)
+ .setMessage(packageLookupValue.getErrorMsg())
+ .setPackageLoadingCode(PackageLoading.Code.INVALID_NAME)
+ .build();
}
+ // We should never get here.
+ throw new IllegalStateException();
}
WorkspaceNameValue workspaceNameValue =
@@ -439,10 +465,14 @@
ErrorReadingStarlarkExtensionException.class,
InconsistentFilesystemException.class);
} catch (ErrorReadingStarlarkExtensionException | InconsistentFilesystemException e) {
- throw new PackageFunctionException(
- new NoSuchPackageException(
- packageId, "Error encountered while reading the prelude file: " + e.getMessage()),
- Transience.PERSISTENT);
+ String message = "Error encountered while reading the prelude file: " + e.getMessage();
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setTransience(Transience.PERSISTENT)
+ .setPackageIdentifier(packageId)
+ .setMessage(message)
+ .setPackageLoadingCode(PackageLoading.Code.PRELUDE_FILE_READ_ERROR)
+ .build();
}
if (astLookupValue == null) {
return null;
@@ -501,8 +531,12 @@
packageLookupValue.getRoot(), packageId, pkgBuilder, env);
} catch (InternalInconsistentFilesystemException e) {
packageFunctionCache.invalidate(packageId);
+ PackageLoading.Code packageLoadingCode =
+ e.isTransient()
+ ? PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR
+ : PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR;
throw new PackageFunctionException(
- e.toNoSuchPackageException(),
+ e.toNoSuchPackageException(packageLoadingCode),
e.isTransient() ? Transience.TRANSIENT : Transience.PERSISTENT);
}
Set<SkyKey> globKeys = packageCacheEntry.globDepKeys;
@@ -511,16 +545,23 @@
packageId, globKeys, env, pkgBuilder.containsErrors());
} catch (InternalInconsistentFilesystemException e) {
packageFunctionCache.invalidate(packageId);
+ PackageLoading.Code packageLoadingCode =
+ e.isTransient()
+ ? PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR
+ : PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR;
throw new PackageFunctionException(
- e.toNoSuchPackageException(),
+ e.toNoSuchPackageException(packageLoadingCode),
e.isTransient() ? Transience.TRANSIENT : Transience.PERSISTENT);
} catch (FileSymlinkException e) {
packageFunctionCache.invalidate(packageId);
- throw new PackageFunctionException(
- new NoSuchPackageException(
- packageId, "Symlink issue while evaluating globs: " + e.getUserFriendlyMessage()),
- // Since the symlink issue was detected by Skyframe globbing, it's non-transient.
- Transience.PERSISTENT);
+ String message = "Symlink issue while evaluating globs: " + e.getUserFriendlyMessage();
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setTransience(Transience.PERSISTENT)
+ .setPackageIdentifier(packageId)
+ .setMessage(message)
+ .setPackageLoadingCode(PackageLoading.Code.EVAL_GLOBS_SYMLINK_ERROR)
+ .build();
}
if (env.valuesMissing()) {
return null;
@@ -566,17 +607,23 @@
if (buildFileValue == null) {
return null;
}
- Preconditions.checkState(buildFileValue.exists(),
- "Package lookup succeeded but BUILD file doesn't exist");
+ checkState(buildFileValue.exists(), "Package lookup succeeded but BUILD file doesn't exist");
return buildFileValue;
}
private static BuildFileContainsErrorsException makeBzlLoadFailedException(
PackageIdentifier packageId, BzlLoadFailedException e) {
Throwable rootCause = Throwables.getRootCause(e);
- return (rootCause instanceof IOException)
- ? new BuildFileContainsErrorsException(packageId, e.getMessage(), (IOException) rootCause)
- : new BuildFileContainsErrorsException(packageId, e.getMessage());
+ PackageFunctionException.Builder builder =
+ PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS)
+ .setPackageIdentifier(packageId)
+ .setMessage(e.getMessage())
+ .setPackageLoadingCode(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
+ if (rootCause instanceof IOException) {
+ builder.setException((IOException) rootCause);
+ }
+ return (BuildFileContainsErrorsException) builder.buildCause();
}
/**
@@ -593,13 +640,18 @@
Environment env,
BzlLoadFunction bzlLoadFunctionForInlining)
throws NoSuchPackageException, InterruptedException {
- Preconditions.checkArgument(!packageId.getRepository().isDefault());
+ checkArgument(!packageId.getRepository().isDefault());
// Parse the labels in the file's load statements.
List<Pair<String, Label>> loads =
BzlLoadFunction.getLoadLabels(env.getListener(), file, packageId, repoMapping);
if (loads == null) {
- throw new BuildFileContainsErrorsException(packageId, "malformed load statements");
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS)
+ .setPackageIdentifier(packageId)
+ .setMessage("malformed load statements")
+ .setPackageLoadingCode(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR)
+ .buildCause();
}
// Compute Skyframe key for each label in 'loads'.
@@ -626,7 +678,13 @@
} catch (BzlLoadFailedException e) {
throw makeBzlLoadFailedException(packageId, e);
} catch (InconsistentFilesystemException e) {
- throw new NoSuchPackageException(packageId, e.getMessage(), e);
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setPackageIdentifier(packageId)
+ .setMessage(e.getMessage())
+ .setException(e)
+ .setPackageLoadingCode(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR)
+ .buildCause();
}
if (bzlLoads == null) {
return null; // Skyframe deps unavailable
@@ -692,7 +750,7 @@
continue;
}
if (skyValue == null) {
- Preconditions.checkState(env.valuesMissing(), "no starlark load value for %s", key);
+ checkState(env.valuesMissing(), "no starlark load value for %s", key);
// We continue making inline calls even if some requested values are missing, to
// maximize the number of dependent (non-inlined) SkyFunctions that are requested, thus
// avoiding a quadratic number of restarts.
@@ -1083,10 +1141,9 @@
Map<SkyKey, ValueOrException2<IOException, BuildFileNotFoundException>> globValueMap)
throws SkyframeGlobbingIOException {
ValueOrException2<IOException, BuildFileNotFoundException> valueOrException =
- Preconditions.checkNotNull(
- globValueMap.get(globKey), "%s should not be missing", globKey);
+ checkNotNull(globValueMap.get(globKey), "%s should not be missing", globKey);
try {
- return Preconditions.checkNotNull(
+ return checkNotNull(
(GlobValue) valueOrException.get(), "%s should not be missing", globKey)
.getMatches();
} catch (BuildFileNotFoundException | IOException e) {
@@ -1165,7 +1222,7 @@
env.getListener().handle(Event.progress("Loading package: " + packageId));
}
ParserInput input;
- Preconditions.checkNotNull(buildFileValue, packageId);
+ checkNotNull(buildFileValue, packageId);
byte[] buildFileBytes = null;
try {
buildFileBytes =
@@ -1179,9 +1236,14 @@
if (buildFileBytes == null) {
// Note that we did the work that led to this IOException, so we should
// conservatively report this error as transient.
- throw new PackageFunctionException(
- new BuildFileContainsErrorsException(packageId, e.getMessage(), e),
- Transience.TRANSIENT);
+ throw PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS)
+ .setTransience(Transience.TRANSIENT)
+ .setPackageIdentifier(packageId)
+ .setMessage(e.getMessage())
+ .setException(e)
+ .setPackageLoadingCode(PackageLoading.Code.BUILD_FILE_MISSING)
+ .build();
}
// If control flow reaches here, we're in territory that is deliberately unsound.
// See the javadoc for ActionOnIOExceptionReadingBuildFile.
@@ -1261,8 +1323,7 @@
private static class InternalInconsistentFilesystemException extends Exception {
private boolean isTransient;
-
- private PackageIdentifier packageIdentifier;
+ private final PackageIdentifier packageIdentifier;
/**
* Used to represent a filesystem inconsistency discovered outside the
@@ -1287,20 +1348,161 @@
return isTransient;
}
- private NoSuchPackageException toNoSuchPackageException() {
- return new NoSuchPackageException(
- packageIdentifier, this.getMessage(), (Exception) this.getCause());
+ private NoSuchPackageException toNoSuchPackageException(
+ PackageLoading.Code packageLoadingCode) {
+ return PackageFunctionException.builder()
+ .setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
+ .setPackageIdentifier(packageIdentifier)
+ .setMessage(this.getMessage())
+ .setException((Exception) this.getCause())
+ .setPackageLoadingCode(packageLoadingCode)
+ .buildCause();
}
}
/**
- * Used to declare all the exception types that can be wrapped in the exception thrown by
- * {@link PackageFunction#compute}.
+ * Used to declare all the exception types that can be wrapped in the exception thrown by {@link
+ * PackageFunction#compute}.
*/
static class PackageFunctionException extends SkyFunctionException {
public PackageFunctionException(NoSuchPackageException e, Transience transience) {
super(e, transience);
}
+
+ static Builder builder() {
+ return new Builder();
+ }
+
+ /**
+ * An enum to help create the different types of {@link NoSuchPackageException}. PackageFunction
+ * contains a myriad of different types of exceptions that extend NoSuchPackageException for
+ * different scenarios.
+ */
+ static enum Type {
+ BUILD_FILE_CONTAINS_ERRORS {
+ @Override
+ BuildFileContainsErrorsException create(
+ PackageIdentifier packId, String msg, DetailedExitCode detailedExitCode, Exception e) {
+ return e instanceof IOException
+ ? new BuildFileContainsErrorsException(packId, msg, (IOException) e, detailedExitCode)
+ : new BuildFileContainsErrorsException(packId, msg, detailedExitCode);
+ }
+ },
+ BUILD_FILE_NOT_FOUND {
+ @Override
+ BuildFileNotFoundException create(
+ PackageIdentifier packId, String msg, DetailedExitCode detailedExitCode, Exception e) {
+ return new BuildFileNotFoundException(packId, msg, detailedExitCode);
+ }
+ },
+ INVALID_PACKAGE_NAME {
+ @Override
+ InvalidPackageNameException create(
+ PackageIdentifier packId, String msg, DetailedExitCode detailedExitCode, Exception e) {
+ return new InvalidPackageNameException(packId, msg, detailedExitCode);
+ }
+ },
+ NO_SUCH_PACKAGE {
+ @Override
+ NoSuchPackageException create(
+ PackageIdentifier packId, String msg, DetailedExitCode detailedExitCode, Exception e) {
+ return e != null
+ ? new NoSuchPackageException(packId, msg, e, detailedExitCode)
+ : new NoSuchPackageException(packId, msg, detailedExitCode);
+ }
+ };
+
+ abstract NoSuchPackageException create(
+ PackageIdentifier packId, String msg, DetailedExitCode detailedExitCode, Exception e);
+ }
+
+ /**
+ * The builder class for {@link PackageFunctionException} and its {@link NoSuchPackageException}
+ * cause.
+ */
+ static class Builder {
+ private Type exceptionType;
+ private PackageIdentifier packageIdentifier;
+ private Transience transience;
+ private Exception exception;
+ private String message;
+ private PackageLoading.Code packageLoadingCode;
+
+ Builder setType(Type exceptionType) {
+ this.exceptionType = exceptionType;
+ return this;
+ }
+
+ Builder setPackageIdentifier(PackageIdentifier packageIdentifier) {
+ this.packageIdentifier = packageIdentifier;
+ return this;
+ }
+
+ Builder setTransience(Transience transience) {
+ this.transience = transience;
+ return this;
+ }
+
+ Builder setException(Exception exception) {
+ this.exception = exception;
+ return this;
+ }
+
+ Builder setMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ Builder setPackageLoadingCode(PackageLoading.Code packageLoadingCode) {
+ this.packageLoadingCode = packageLoadingCode;
+ return this;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ exceptionType, packageIdentifier, transience, exception, message, packageLoadingCode);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (this == other) {
+ return true;
+ }
+ if (!(other instanceof PackageFunctionException.Builder)) {
+ return false;
+ }
+ PackageFunctionException.Builder otherBuilder = (PackageFunctionException.Builder) other;
+ return Objects.equals(exceptionType, otherBuilder.exceptionType)
+ && Objects.equals(packageIdentifier, otherBuilder.packageIdentifier)
+ && Objects.equals(transience, otherBuilder.transience)
+ && Objects.equals(exception, otherBuilder.exception)
+ && Objects.equals(message, otherBuilder.message)
+ && Objects.equals(packageLoadingCode, otherBuilder.packageLoadingCode);
+ }
+
+ NoSuchPackageException buildCause() {
+ checkNotNull(exceptionType, "The NoSuchPackageException type must be set.");
+ checkNotNull(packageLoadingCode, "The PackageLoading code must be set.");
+ DetailedExitCode detailedExitCode = createDetailedExitCode(message, packageLoadingCode);
+ return exceptionType.create(packageIdentifier, message, detailedExitCode, exception);
+ }
+
+ PackageFunctionException build() {
+ return new PackageFunctionException(
+ buildCause(), checkNotNull(transience, "Transience must be set"));
+ }
+
+ private static DetailedExitCode createDetailedExitCode(
+ String message, PackageLoading.Code packageLoadingCode) {
+ return DetailedExitCode.of(
+ ExitCode.BUILD_FAILURE,
+ FailureDetail.newBuilder()
+ .setMessage(message)
+ .setPackageLoading(PackageLoading.newBuilder().setCode(packageLoadingCode).build())
+ .build());
+ }
+ }
}
/** A simple value class to store the result of the Starlark loads. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 1e11385..a538035 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -260,6 +260,10 @@
// performance.
System.getenv("TEST_TMPDIR") == null ? 200 : 5;
+ // The limit of how many times we will traverse through an exception chain when catching a
+ // target parsing exception.
+ private static final int EXCEPTION_TRAVERSAL_LIMIT = 10;
+
// Cache of partially constructed Package instances, stored between reruns of the PackageFunction
// (because of missing dependencies, within the same evaluate() run) to avoid loading the same
// package twice (first time loading to find load()ed bzl files and declare Skyframe
@@ -2877,6 +2881,17 @@
eventHandler.post(new LoadingPhaseStartedEvent(packageProgress));
EvaluationResult<TargetPatternPhaseValue> evalResult =
evaluate(ImmutableList.of(key), keepGoing, threadCount, eventHandler);
+ tryThrowTargetParsingException(eventHandler, targetPatterns, key, evalResult);
+ eventHandler.post(new TargetParsingPhaseTimeEvent(timer.stop().elapsed().toMillis()));
+ return evalResult.get(key);
+ }
+
+ private void tryThrowTargetParsingException(
+ ExtendedEventHandler eventHandler,
+ List<String> targetPatterns,
+ SkyKey key,
+ EvaluationResult<TargetPatternPhaseValue> evalResult)
+ throws TargetParsingException {
if (evalResult.hasError()) {
ErrorInfo errorInfo = evalResult.getError(key);
TargetParsingException exc;
@@ -2887,24 +2902,47 @@
// set as being in error.
eventHandler.post(PatternExpandingError.failed(targetPatterns, exc.getMessage()));
} else {
- Exception e = Preconditions.checkNotNull(errorInfo.getException());
-
- // Following SkyframeTargetPatternEvaluator, we convert any exception into a
- // TargetParsingException.
- exc =
- (e instanceof TargetParsingException)
- ? (TargetParsingException) e
- : new TargetParsingException(e.getMessage(), e);
- if (!(e instanceof TargetParsingException)) {
- // If it's a TargetParsingException, then the TargetPatternPhaseFunction has already
- // reported the error, so we don't need to report it again.
- eventHandler.post(PatternExpandingError.failed(targetPatterns, exc.getMessage()));
- }
+ exc = constructNoCycleTargetParsingException(eventHandler, targetPatterns, errorInfo);
}
throw exc;
}
- eventHandler.post(new TargetParsingPhaseTimeEvent(timer.stop().elapsed().toMillis()));
- return evalResult.get(key);
+ }
+
+ private static TargetParsingException constructNoCycleTargetParsingException(
+ ExtendedEventHandler eventHandler, List<String> targetPatterns, ErrorInfo errorInfo) {
+ Exception e = Preconditions.checkNotNull(errorInfo.getException());
+ DetailedExitCode detailedExitCode = traverseExceptionChain(e);
+ if (!(e instanceof TargetParsingException)) {
+ // If it's a TargetParsingException, then the TargetPatternPhaseFunction has already
+ // reported the error, so we don't need to report it again.
+ eventHandler.post(PatternExpandingError.failed(targetPatterns, e.getMessage()));
+ }
+
+ // Following SkyframeTargetPatternEvaluator, we convert any exception into a
+ // TargetParsingException or DetailedTargetParsingException if DetailedExitCode is
+ // available.
+ return detailedExitCode != null
+ ? new DetailedTargetParsingException(
+ (e instanceof TargetParsingException) ? e.getCause() : e, detailedExitCode)
+ : (e instanceof TargetParsingException)
+ ? (TargetParsingException) e
+ : new TargetParsingException(e.getMessage(), e);
+ }
+
+ @Nullable
+ private static DetailedExitCode traverseExceptionChain(Exception topLevelException) {
+ Exception traverseException = topLevelException;
+ DetailedExitCode detailedExitCode = null;
+ int traverseLevel = 0;
+ while (traverseLevel < EXCEPTION_TRAVERSAL_LIMIT) {
+ traverseLevel++;
+ detailedExitCode = DetailedException.getDetailedExitCode(traverseException);
+ if (detailedExitCode != null || traverseException.getCause() == null) {
+ break;
+ }
+ traverseException = (Exception) traverseException.getCause();
+ }
+ return detailedExitCode;
}
public PrepareAnalysisPhaseValue prepareAnalysisPhase(
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index d125f39..656be6a 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -142,6 +142,7 @@
TestAction test_action = 172;
Worker worker = 173;
Analysis analysis = 174;
+ PackageLoading package_loading = 175;
}
reserved 102; // For internal use
@@ -1033,3 +1034,22 @@
Code code = 1;
}
+
+message PackageLoading {
+ enum Code {
+ PACKAGE_LOADING_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
+ WORKSPACE_FILE_ERROR = 1 [(metadata) = { exit_code: 1 }];
+ MAX_COMPUTATION_STEPS_EXCEEDED = 2 [(metadata) = { exit_code: 1 }];
+ BUILD_FILE_MISSING = 3 [(metadata) = { exit_code: 1 }];
+ REPOSITORY_MISSING = 4 [(metadata) = { exit_code: 1 }];
+ PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR = 5
+ [(metadata) = { exit_code: 1 }];
+ TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR = 6 [(metadata) = { exit_code: 1 }];
+ INVALID_NAME = 7 [(metadata) = { exit_code: 1 }];
+ PRELUDE_FILE_READ_ERROR = 8 [(metadata) = { exit_code: 1 }];
+ EVAL_GLOBS_SYMLINK_ERROR = 9 [(metadata) = { exit_code: 1 }];
+ IMPORT_STARLARK_FILE_ERROR = 10 [(metadata) = { exit_code: 1 }];
+ }
+
+ Code code = 1;
+}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index c3a338d..d17899c 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -263,6 +263,7 @@
"//src/main/java/com/google/devtools/build/lib/packages:starlark_semantics_options",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:pattern_expanding_error",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
@@ -273,6 +274,8 @@
"//src/main/java/com/google/devtools/build/lib/syntax:frontend",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
@@ -281,6 +284,7 @@
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/com/google/devtools/common/options:invocation_policy",
+ "//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/analysis/util:test-build-options",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 9ad28f9..88ffeb1 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -50,7 +50,9 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
+import com.google.devtools.build.lib.skyframe.DetailedTargetParsingException;
import com.google.devtools.build.lib.skyframe.PatternExpandingError;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -58,6 +60,8 @@
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.SkyframeExecutorTestHelper;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
@@ -1136,10 +1140,14 @@
assertThat(value.hasError()).isTrue();
tester.assertContainsWarning("Target pattern parsing failed");
} else {
- TargetParsingException exn =
- assertThrows(TargetParsingException.class, () -> tester.load(patterns));
+ DetailedTargetParsingException exn =
+ assertThrows(DetailedTargetParsingException.class, () -> tester.load(patterns));
assertThat(exn).hasCauseThat().isInstanceOf(BuildFileContainsErrorsException.class);
assertThat(exn).hasCauseThat().hasMessageThat().contains("Extension 'bad/f1.bzl' has errors");
+ DetailedExitCode detailedExitCode = exn.getDetailedExitCode();
+ assertThat(detailedExitCode.getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
+ assertThat(detailedExitCode.getFailureDetail().getPackageLoading().getCode())
+ .isEqualTo(PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
tester.assertContainsError("/workspace/bad/f1.bzl:1:1: name 'nope' is not defined");
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
index d530430..80f22a8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -180,6 +180,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_progress_receiver",
"//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_value",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness_manager",
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function",
@@ -239,6 +240,7 @@
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:crash_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index 4412a28..8b3a2b2 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -45,9 +45,12 @@
import com.google.devtools.build.lib.pkgcache.PackageOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.MoreAsserts;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Dirent;
@@ -308,6 +311,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
}
@Test
@@ -344,6 +349,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
}
/** Regression test for unexpected exception type from PackageValue. */
@@ -372,6 +379,8 @@
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage).contains("Inconsistent filesystem operations");
assertThat(errorMessage).contains(expectedMessage);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR);
}
@SuppressWarnings("unchecked") // Cast of srcs attribute to Iterable<Label>.
@@ -617,6 +626,8 @@
"error loading package 'test/starlark': "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file";
assertThat(errorInfo.getException()).hasMessageThat().isEqualTo(expectedMsg);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -646,6 +657,8 @@
"error loading package 'test/starlark': "
+ "in /workspace/test/starlark/extension.bzl: "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file");
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -673,6 +686,8 @@
.isEqualTo(
"error loading package 'test/starlark': Encountered error while reading extension "
+ "file 'test/starlark/extension.bzl': Symlink cycle");
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -855,6 +870,7 @@
assertThat(errorMessage).contains("nope");
assertThat(errorInfo.getException()).isInstanceOf(NoSuchPackageException.class);
assertThat(errorInfo.getException()).hasCauseThat().isInstanceOf(IOException.class);
+ assertDetailedExitCode(errorInfo.getException(), PackageLoading.Code.BUILD_FILE_MISSING);
}
@Test
@@ -874,6 +890,8 @@
assertThat(errorMessage).contains("nope");
assertThat(errorInfo.getException()).isInstanceOf(NoSuchPackageException.class);
assertThat(errorInfo.getException()).hasCauseThat().isInstanceOf(IOException.class);
+ assertDetailedExitCode(
+ errorInfo.getException(), PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
}
@Test
@@ -1309,6 +1327,15 @@
// presence of a symlink cycle encountered during glob evaluation.
}
+ private static void assertDetailedExitCode(
+ Exception exception, PackageLoading.Code expectedPackageLoadingCode) {
+ assertThat(exception).isInstanceOf(DetailedException.class);
+ DetailedExitCode detailedExitCode = ((DetailedException) exception).getDetailedExitCode();
+ assertThat(detailedExitCode.getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
+ assertThat(detailedExitCode.getFailureDetail().getPackageLoading().getCode())
+ .isEqualTo(expectedPackageLoadingCode);
+ }
+
private static class CustomInMemoryFs extends InMemoryFileSystem {
private abstract static class FileStatusOrException {
abstract FileStatus get() throws IOException;
