Clean up SpawnResult documentation and visibility.
This clarifies and improves the accuracy of its documentation. Also, this
reduces its unnecessarily broad visibility.
RELNOTES: None.
PiperOrigin-RevId: 296067625
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
index 9b97352..8d09f96 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
@@ -26,17 +26,10 @@
import java.util.Optional;
import javax.annotation.Nullable;
-/**
- * The result of a spawn execution.
- *
- * <p>DO NOT IMPLEMENT THIS INTERFACE! Use {@link SpawnResult.Builder} to create instances instead.
- * This is a temporary situation as long as we still have separate internal and external
- * implementations - the plan is to merge the two into a single immutable, final class.
- */
-// TODO(ulfjack): Change this from an interface to an immutable, final class.
+/** The result of a {@link Spawn}'s execution. */
public interface SpawnResult {
/** The status of the attempted Spawn execution. */
- public enum Status {
+ enum Status {
/** Subprocess executed successfully, and returned a zero exit code. */
SUCCESS,
@@ -53,26 +46,27 @@
OUT_OF_MEMORY(true),
/**
- * Subprocess did not execute. This error is not catastrophic - Bazel will try to continue the
- * build if keep_going is enabled, possibly attempt to rerun the same spawn, and attempt to run
- * other actions.
+ * Subprocess did not execute, it's not the user's fault, and the error is not catastrophic. If
+ * keep_going is enabled then Bazel will try to continue the build, possibly will attempt to
+ * rerun the same spawn, and possibly will attempt to run other actions.
*/
EXECUTION_FAILED,
/**
- * Subprocess did not execute. Do not retry, and exit immediately even if keep_going is enabled.
+ * Subprocess did not execute, it's not the user's fault, and the error is catastrophic. Bazel
+ * will not rerun this spawn. Bazel will attempt to not run other actions (regardless of whether
+ * keep_going is enabled).
*/
EXECUTION_FAILED_CATASTROPHICALLY,
/**
- * Subprocess did not execute in a way that indicates that the user can fix it. For example, a
- * remote system may have denied the execution due to too many inputs or too large inputs.
+ * Subprocess did not execute, and it may be the user's fault. The user may be able to fix it.
+ * For example, a remote system may have denied the execution due to too many inputs or too
+ * large inputs.
*/
EXECUTION_DENIED(true),
- /**
- * The remote execution system is overloaded and had to refuse execution for this Spawn.
- */
+ /** The remote execution system is overloaded and had to refuse execution for this Spawn. */
REMOTE_EXECUTOR_OVERLOADED,
/**
@@ -83,11 +77,11 @@
private final boolean isUserError;
- private Status(boolean isUserError) {
+ Status(boolean isUserError) {
this.isUserError = isUserError;
}
- private Status() {
+ Status() {
this(false);
}
@@ -109,31 +103,33 @@
/** Returns true if the status was {@link Status#EXECUTION_FAILED_CATASTROPHICALLY}. */
boolean isCatastrophe();
- /** The status of the attempted Spawn execution. */
+ /** Returns the status of the attempted Spawn execution. */
Status status();
/**
- * The exit code of the subprocess if the subprocess was executed. Should only be called if
- * {@link #status} returns {@link Status#SUCCESS} or {@link Status#NON_ZERO_EXIT}.
+ * Returns the exit code of the subprocess if the subprocess was executed.
*
- * <p>This method must return a non-zero exit code if the status is {@link Status#TIMEOUT} or
- * {@link Status#OUT_OF_MEMORY}. It is recommended to return 128 + 14 when the status is
- * {@link Status#TIMEOUT}, which corresponds to the Unix signal SIGALRM.
+ * <p>Returns zero if {@link #status} returns {@link Status#SUCCESS}.
*
- * <p>This method may throw {@link IllegalStateException} if called for any other status.
+ * <p>Returns non-zero if {@link #status} returns {@link Status#NON_ZERO_EXIT} or {@link
+ * Status#OUT_OF_MEMORY}.
+ *
+ * <p>Returns 128 + 14 (corresponding to the Unix signal SIGALRM) if {@link #status} returns
+ * {@link Status#TIMEOUT}.
*/
int exitCode();
/**
- * The host name of the executor or {@code null}. This information is intended for debugging
- * purposes, especially for remote execution systems. Remote caches usually do not store the
- * original host name, so this is generally {@code null} for cache hits.
+ * Returns the host name of the executor or {@code null}. This information is intended for
+ * debugging purposes, especially for remote execution systems. Remote caches usually do not store
+ * the original host name, so this is generally {@code null} for cache hits.
*/
- @Nullable String getExecutorHostName();
+ @Nullable
+ String getExecutorHostName();
/**
- * The name of the SpawnRunner that executed the spawn. It should always be defined, unless
- * isCacheHit is true, in which case the spawn was not actually run.
+ * Returns the name of the SpawnRunner that executed the spawn. It should always be defined,
+ * unless isCacheHit is true, in which case the spawn was not actually run.
*/
String getRunnerName();
@@ -187,7 +183,7 @@
SpawnMetrics getMetrics();
- /** Whether the spawn result was a cache hit. */
+ /** Returns whether the spawn result was a cache hit. */
boolean isCacheHit();
/** Returns an optional custom failure message for the result. */
@@ -196,9 +192,10 @@
}
/**
- * SpawnResults can optionally support returning outputs in-memory. Such outputs can be obtained
- * from this method if so. This behavior is optional, and can be triggered with
- * {@link ExecutionRequirements#REMOTE_EXECUTION_INLINE_OUTPUTS}.
+ * Returns a {@link Spawn}'s output in-memory, if supported and available.
+ *
+ * <p>This behavior may be triggered with {@link
+ * ExecutionRequirements#REMOTE_EXECUTION_INLINE_OUTPUTS}.
*/
@Nullable
default InputStream getInMemoryOutput(ActionInput output) {
@@ -215,11 +212,10 @@
/** Returns a file path to the action metadata log. */
Optional<MetadataLog> getActionMetadataLog();
- /**
- * Basic implementation of {@link SpawnResult}.
- */
- @Immutable @ThreadSafe
- public static final class SimpleSpawnResult implements SpawnResult {
+ /** Basic implementation of {@link SpawnResult}. */
+ @Immutable
+ @ThreadSafe
+ final class SimpleSpawnResult implements SpawnResult {
private final int exitCode;
private final Status status;
private final String executorHostName;
@@ -231,11 +227,13 @@
private final Optional<Long> numBlockOutputOperations;
private final Optional<Long> numBlockInputOperations;
private final Optional<Long> numInvoluntaryContextSwitches;
+ private final Optional<MetadataLog> actionMetadataLog;
private final boolean cacheHit;
private final String failureMessage;
- private final ActionInput inMemoryOutputFile;
- private final ByteString inMemoryContents;
- private final Optional<MetadataLog> actionMetadataLog;
+
+ // Invariant: Either both have a value or both are null.
+ @Nullable private final ActionInput inMemoryOutputFile;
+ @Nullable private final ByteString inMemoryContents;
SimpleSpawnResult(Builder builder) {
this.exitCode = builder.exitCode;
@@ -393,10 +391,8 @@
}
}
- /**
- * Builder class for {@link SpawnResult}.
- */
- public static final class Builder {
+ /** Builder class for {@link SpawnResult}. */
+ final class Builder {
private int exitCode;
private Status status;
private String executorHostName;
@@ -411,9 +407,10 @@
private Optional<MetadataLog> actionMetadataLog = Optional.empty();
private boolean cacheHit;
private String failureMessage = "";
- /* Invariant: Either both have a value or both are null. */
- private ActionInput inMemoryOutputFile;
- private ByteString inMemoryContents;
+
+ // Invariant: Either both have a value or both are null.
+ @Nullable private ActionInput inMemoryOutputFile;
+ @Nullable private ByteString inMemoryContents;
public SpawnResult build() {
Preconditions.checkArgument(!runnerName.isEmpty());
@@ -499,14 +496,14 @@
return this;
}
- public Builder setActionMetadataLog(MetadataLog actionMetadataLog) {
+ Builder setActionMetadataLog(MetadataLog actionMetadataLog) {
this.actionMetadataLog = Optional.of(actionMetadataLog);
return this;
}
}
- /** A tuple containing the name reference to the metadata and the file path to the Metadata */
- public static final class MetadataLog {
+ /** A {@link Spawn}'s metadata name and {@link Path}. */
+ final class MetadataLog {
private final String name;
private final Path filePath;
