Remove null as an encoding of BUILD_FAILURE (exit code 1)
Future work will deliver details for failures attributable to the user.
RELNOTES: None.
PiperOrigin-RevId: 301809499
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
index c9becf1..a87b933 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.devtools.build.lib.causes.ActionFailed;
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -23,7 +25,6 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
-import javax.annotation.Nullable;
/**
* This exception gets thrown if {@link Action#execute(ActionExecutionContext)} is unsuccessful.
@@ -35,14 +36,14 @@
private final Action action;
private final NestedSet<Cause> rootCauses;
private final boolean catastrophe;
- @Nullable private final DetailedExitCode detailedExitCode;
+ private final DetailedExitCode detailedExitCode;
public ActionExecutionException(Throwable cause, Action action, boolean catastrophe) {
super(cause.getMessage(), cause);
this.action = action;
this.rootCauses = rootCausesFromAction(action);
this.catastrophe = catastrophe;
- this.detailedExitCode = null;
+ this.detailedExitCode = DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
}
public ActionExecutionException(String message,
@@ -51,7 +52,7 @@
this.action = action;
this.rootCauses = rootCausesFromAction(action);
this.catastrophe = catastrophe;
- this.detailedExitCode = null;
+ this.detailedExitCode = DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
}
public ActionExecutionException(
@@ -64,7 +65,7 @@
this.action = action;
this.rootCauses = rootCausesFromAction(action);
this.catastrophe = catastrophe;
- this.detailedExitCode = detailedExitCode;
+ this.detailedExitCode = checkNotNull(detailedExitCode);
}
public ActionExecutionException(String message, Action action, boolean catastrophe) {
@@ -72,7 +73,7 @@
this.action = action;
this.rootCauses = rootCausesFromAction(action);
this.catastrophe = catastrophe;
- this.detailedExitCode = null;
+ this.detailedExitCode = DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
}
public ActionExecutionException(String message, Action action, boolean catastrophe,
@@ -90,20 +91,7 @@
this.action = action;
this.rootCauses = rootCauses;
this.catastrophe = catastrophe;
- this.detailedExitCode = null;
- }
-
- public ActionExecutionException(
- String message,
- Throwable cause,
- Action action,
- NestedSet<Cause> rootCauses,
- boolean catastrophe) {
- super(message, cause);
- this.action = action;
- this.rootCauses = rootCauses;
- this.catastrophe = catastrophe;
- this.detailedExitCode = null;
+ this.detailedExitCode = DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
}
public ActionExecutionException(
@@ -117,7 +105,7 @@
this.action = action;
this.rootCauses = rootCauses;
this.catastrophe = catastrophe;
- this.detailedExitCode = detailedExitCode;
+ this.detailedExitCode = checkNotNull(detailedExitCode);
}
private static NestedSet<Cause> rootCausesFromAction(Action action) {
@@ -163,28 +151,15 @@
/**
* Returns the exit code to return from this Bazel invocation because of this action execution
* failure.
- *
- * <p>Returns {@code null} if the exception is not intended to cause the invocation to fail, or if
- * the failure is attributable to the user. (In the latter case, ExitCode.BUILD_FAILURE with
- * numeric value 1 will be returned.)
*/
- @Nullable
public ExitCode getExitCode() {
- return detailedExitCode == null ? null : detailedExitCode.getExitCode();
+ return detailedExitCode.getExitCode();
}
/**
* Returns the pair of {@link ExitCode} and optional {@link FailureDetail} to return from this
* Bazel invocation because of this action execution failure.
- *
- * <p>Returns {@code null} if the exception is not intended to cause the invocation to fail, or if
- * the failure is attributable to the user. (In the latter case, ExitCode.BUILD_FAILURE with
- * numeric value 1 will be returned for an exit code, and no FailureDetail will be returned.)
*/
- // TODO(b/138456686): for detailed user failures, this must be able to return non-null for
- // user-attributable failures. The meaning of "null" must be changed in code paths handling this
- // returned value.
- @Nullable
public DetailedExitCode getDetailedExitCode() {
return detailedExitCode;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
index d0aa35b..4fa494d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.actions;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -40,31 +42,43 @@
@ThreadSafe
public class BuildFailedException extends Exception {
private final boolean catastrophic;
- private final Action action;
+ @Nullable private final Action action;
private final NestedSet<Cause> rootCauses;
private final boolean errorAlreadyShown;
- @Nullable private final DetailedExitCode detailedExitCode;
+ private final DetailedExitCode detailedExitCode;
public BuildFailedException() {
this(null);
}
public BuildFailedException(String message) {
- this(message, false, null, NestedSetBuilder.emptySet(Order.STABLE_ORDER), false, null);
+ this(
+ message,
+ /*catastrophic=*/ false,
+ /*action=*/ null,
+ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+ /*errorAlreadyShown=*/ false,
+ DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE));
}
public BuildFailedException(String message, DetailedExitCode detailedExitCode) {
this(
message,
- false,
- null,
+ /*catastrophic=*/ false,
+ /*action=*/ null,
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
- false,
+ /*errorAlreadyShown=*/ false,
detailedExitCode);
}
public BuildFailedException(String message, boolean catastrophic) {
- this(message, catastrophic, null, NestedSetBuilder.emptySet(Order.STABLE_ORDER), false, null);
+ this(
+ message,
+ catastrophic,
+ /*action=*/ null,
+ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+ /*errorAlreadyShown=*/ false,
+ DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE));
}
public BuildFailedException(
@@ -73,19 +87,20 @@
Action action,
NestedSet<Cause> rootCauses,
boolean errorAlreadyShown,
- @Nullable DetailedExitCode detailedExitCode) {
+ DetailedExitCode detailedExitCode) {
super(message);
this.catastrophic = catastrophic;
this.rootCauses = rootCauses;
this.action = action;
this.errorAlreadyShown = errorAlreadyShown;
- this.detailedExitCode = detailedExitCode;
+ this.detailedExitCode = checkNotNull(detailedExitCode);
}
public boolean isCatastrophic() {
return catastrophic;
}
+ @Nullable
public Action getAction() {
return action;
}
@@ -101,15 +116,7 @@
/**
* Returns the pair of {@link ExitCode} and optional {@link FailureDetail} to return from this
* Bazel invocation.
- *
- * <p>Returns {@code null} if the failure is attributable to the user. (In this case,
- * ExitCode.BUILD_FAILURE with numeric value 1 will be returned for an exit code, and no
- * FailureDetail will be returned.)
*/
- // TODO(b/138456686): for detailed user failures, this must be able to return non-null for
- // user-attributable failures. The meaning of "null" must be changed in code paths handling this
- // returned value.
- @Nullable
public DetailedExitCode getDetailedExitCode() {
return detailedExitCode;
}
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 45f7607..5f4258b 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
@@ -298,10 +298,7 @@
if (e.isCatastrophic()) {
result.setCatastrophe();
}
- detailedExitCode =
- e.getDetailedExitCode() != null
- ? e.getDetailedExitCode()
- : DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
+ detailedExitCode = e.getDetailedExitCode();
} catch (InterruptedException e) {
// We may have been interrupted by an error, or the user's interruption may have raced with
// an error, so check to see if we should report that error code instead.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index 0cb1821..62f996ac 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.buildtool;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.Range;
import com.google.common.collect.Sets;
@@ -48,6 +47,7 @@
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.skyframe.CycleInfo;
@@ -168,7 +168,7 @@
executionProgressReceiver,
topLevelArtifactContext);
// progressReceiver is finished, so unsynchronized access to builtTargets is now safe.
- Optional<DetailedExitCode> detailedExitCode =
+ DetailedExitCode detailedExitCode =
processResult(
reporter,
result,
@@ -176,7 +176,7 @@
skyframeExecutor);
if (detailedExitCode != null) {
- detailedExitCodes.add(detailedExitCode.orNull());
+ detailedExitCodes.add(detailedExitCode);
}
// Run exclusive tests: either tagged as "exclusive" or is run in an invocation with
@@ -209,7 +209,7 @@
result);
if (detailedExitCode != null) {
- detailedExitCodes.add(detailedExitCode.orNull());
+ detailedExitCodes.add(detailedExitCode);
}
}
} finally {
@@ -218,34 +218,36 @@
statusReporter.unregisterFromEventBus();
}
- if (!detailedExitCodes.isEmpty()) {
- if (options.getOptions(KeepGoingOption.class).keepGoing) {
- // Use the exit code with the highest priority.
- throw new BuildFailedException(
- null, Collections.max(detailedExitCodes, DetailedExitCodeComparator.INSTANCE));
- } else {
- throw new BuildFailedException();
- }
+ if (detailedExitCodes.isEmpty()) {
+ return;
+ }
+
+ if (options.getOptions(KeepGoingOption.class).keepGoing) {
+ // Use the exit code with the highest priority.
+ throw new BuildFailedException(
+ null, Collections.max(detailedExitCodes, DetailedExitCodeComparator.INSTANCE));
+ } else {
+ throw new BuildFailedException();
}
}
/**
* Process an {@link EvaluationResult}, taking into account the keepGoing setting.
*
- * <p>Returns a nullable optional {@link DetailedExitCode} value, as follows:
+ * <p>Returns a nullable {@link DetailedExitCode} value, as follows:
*
* <ol>
* <li>{@code null}, if {@code result} had no errors
- * <li>{@code Optional.absent()}, if result had errors but none of the errors specified a {@link
- * DetailedExitCode}
- * <li>{@code Optional.of(e)} if result had errors and one of them specified a {@link
- * DetailedExitCode} value {@code e}.
+ * <li>{@code e} if result had errors and one of them specified a {@link DetailedExitCode} value
+ * {@code e}
+ * <li>{@code DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE)} if result had errors but
+ * none specified a {@link DetailedExitCode} value
* </ol>
*
- * <p>Throws on catastrophic failures.
+ * <p>Throws on catastrophic failures and, if !keepGoing, on any failure.
*/
@Nullable
- private static Optional<DetailedExitCode> processResult(
+ private static DetailedExitCode processResult(
ExtendedEventHandler eventHandler,
EvaluationResult<?> result,
boolean keepGoing,
@@ -265,7 +267,7 @@
// in the following order:
// 1. First infrastructure error with non-null exit code
// 2. First non-infrastructure error with non-null exit code
- // 3. Null (later default to 1)
+ // 3. If the build fails but no interpretable error is specified, BUILD_FAILURE.
DetailedExitCode detailedExitCode = null;
for (Map.Entry<SkyKey, ErrorInfo> error : result.errorMap().entrySet()) {
Throwable cause = error.getValue().getException();
@@ -280,7 +282,9 @@
}
}
- return Optional.fromNullable(detailedExitCode);
+ return detailedExitCode != null
+ ? detailedExitCode
+ : DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE);
}
ErrorInfo errorInfo = Preconditions.checkNotNull(result.getError(), result);
Exception exception = errorInfo.getException();
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnExecException.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnExecException.java
index 15620d6..428ebef 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnExecException.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnExecException.java
@@ -73,17 +73,13 @@
message, this, action, isCatastrophic(), getDetailedExitCode());
}
- /** Return exit code depending on the spawn result. */
+ /** Return detailed exit code depending on the spawn result. */
private DetailedExitCode getDetailedExitCode() {
- if (result.status().isConsideredUserError()) {
- if (result.failureDetail() != null) {
- return DetailedExitCode.of(ExitCode.BUILD_FAILURE, result.failureDetail());
- }
- return null;
+ ExitCode exitCode =
+ result.status().isConsideredUserError() ? ExitCode.BUILD_FAILURE : ExitCode.REMOTE_ERROR;
+ if (result.failureDetail() == null) {
+ return DetailedExitCode.justExitCode(exitCode);
}
- if (result.failureDetail() != null) {
- return DetailedExitCode.of(ExitCode.REMOTE_ERROR, result.failureDetail());
- }
- return DetailedExitCode.justExitCode(ExitCode.REMOTE_ERROR);
+ return DetailedExitCode.of(exitCode, result.failureDetail());
}
}
