Fold FailureDetailUtil into DetailedExitCode
This reduces the number of notable programmatic entities associated
with FailureDetail messages by one.
Removing the "Util" class helps prevent it from being a dumping ground of
FailureDetail factory methods for categories that ought to be coupled
with their subsystems' code instead.
RELNOTES: None.
PiperOrigin-RevId: 303775924
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 47d2898..0db2add 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -208,7 +208,6 @@
"util/Clock.java",
"util/ExitCode.java",
"util/DetailedExitCode.java",
- "util/FailureDetailUtil.java",
"util/FileType.java",
"util/FileTypeSet.java",
"util/GccParamFileEscaper.java",
@@ -334,20 +333,9 @@
srcs = ["util/DetailedExitCode.java"],
deps = [
":exitcode-external",
- ":failure_detail_util",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
- ],
-)
-
-java_library(
- name = "failure_detail_util",
- srcs = ["util/FailureDetailUtil.java"],
- deps = [
- ":exitcode-external",
- "//src/main/protobuf:failure_details_java_proto",
- "//third_party:guava",
"//third_party/protobuf:protobuf_java",
],
)
@@ -1138,7 +1126,6 @@
deps = [
":bug-report",
":exitcode-external",
- ":failure_detail_util",
":os_util",
":runtime",
":string_util",
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 0dd282a..e8e6488 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
@@ -36,9 +36,9 @@
import com.google.devtools.build.lib.server.CommandProtos.RunResponse;
import com.google.devtools.build.lib.server.CommandProtos.ServerInfo;
import com.google.devtools.build.lib.server.CommandProtos.StartupOption;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Interrupted;
import com.google.devtools.build.lib.util.ExitCode;
-import com.google.devtools.build.lib.util.FailureDetailUtil;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.OutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -615,7 +615,11 @@
} catch (InterruptedException e) {
result =
BlazeCommandResult.failureDetail(
- FailureDetailUtil.interrupted(Interrupted.Code.INTERRUPTED_UNSPECIFIED));
+ FailureDetail.newBuilder()
+ .setMessage("interrupted")
+ .setInterrupted(
+ Interrupted.newBuilder().setCode(Interrupted.Code.INTERRUPTED_UNSPECIFIED))
+ .build());
commandId = ""; // The default value, the client will ignore it
}
diff --git a/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java b/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
index de37710..75d8643 100644
--- a/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
+++ b/src/main/java/com/google/devtools/build/lib/util/DetailedExitCode.java
@@ -14,9 +14,15 @@
package com.google.devtools.build.lib.util;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.protobuf.Descriptors.EnumValueDescriptor;
+import com.google.protobuf.Descriptors.FieldDescriptor;
+import com.google.protobuf.MessageOrBuilder;
+import java.util.Map;
import javax.annotation.Nullable;
/** An {@link ExitCode} and an optional {@link FailureDetail}. */
@@ -33,6 +39,15 @@
return exitCode;
}
+ /** Returns the registered {@link ExitCode} associated with a {@link FailureDetail} message. */
+ private static ExitCode getExitCode(FailureDetail failureDetail) {
+ // TODO(mschaller): Consider specializing for unregistered exit codes here, if absolutely
+ // necessary.
+ int numericExitCode = getNumericExitCode(failureDetail);
+ return checkNotNull(
+ ExitCode.forCode(numericExitCode), "No ExitCode for numericExitCode %s", numericExitCode);
+ }
+
@Nullable
public FailureDetail getFailureDetail() {
return failureDetail;
@@ -81,7 +96,7 @@
* FailureDetail}'s metadata.
*/
public static DetailedExitCode of(FailureDetail failureDetail) {
- return new DetailedExitCode(FailureDetailUtil.getExitCode(failureDetail), failureDetail);
+ return new DetailedExitCode(getExitCode(failureDetail), failureDetail);
}
@Override
@@ -89,4 +104,68 @@
return String.format(
"DetailedExitCode{exitCode=%s, failureDetail=%s}", exitCode, failureDetail);
}
+
+ /** Returns the numeric exit code associated with a {@link FailureDetail} message. */
+ private static int getNumericExitCode(FailureDetail failureDetail) {
+ MessageOrBuilder categoryMsg = getCategorySubmessage(failureDetail);
+ EnumValueDescriptor subcategoryDescriptor =
+ getSubcategoryDescriptor(failureDetail, categoryMsg);
+ return getNumericExitCode(subcategoryDescriptor);
+ }
+
+ /**
+ * Returns the numeric exit code associated with a {@link FailureDetail} submessage's subcategory
+ * enum value.
+ */
+ private static int getNumericExitCode(EnumValueDescriptor subcategoryDescriptor) {
+ checkArgument(
+ subcategoryDescriptor.getOptions().hasExtension(FailureDetails.metadata),
+ "Enum value %s has no FailureDetails.metadata",
+ subcategoryDescriptor);
+ return subcategoryDescriptor.getOptions().getExtension(FailureDetails.metadata).getExitCode();
+ }
+
+ /**
+ * Returns the category submessage, i.e. the message in {@link FailureDetail}'s oneof. Throws if
+ * none of those fields are set.
+ */
+ private static MessageOrBuilder getCategorySubmessage(FailureDetail failureDetail) {
+ MessageOrBuilder categoryMsg = null;
+ for (Map.Entry<FieldDescriptor, Object> entry : failureDetail.getAllFields().entrySet()) {
+ FieldDescriptor fieldDescriptor = entry.getKey();
+ if (isCategoryField(fieldDescriptor)) {
+ categoryMsg = (MessageOrBuilder) entry.getValue();
+ break;
+ }
+ }
+ return checkNotNull(
+ categoryMsg, "FailureDetail missing category submessage: %s", failureDetail);
+ }
+
+ /**
+ * Returns whether the {@link FieldDescriptor} describes a field in {@link FailureDetail}'s oneof.
+ *
+ * <p>Uses the field number criteria described in failure_details.proto.
+ */
+ private static boolean isCategoryField(FieldDescriptor fieldDescriptor) {
+ int fieldNum = fieldDescriptor.getNumber();
+ return 100 < fieldNum && fieldNum <= 10_000;
+ }
+
+ /**
+ * Returns the enum value descriptor for the enum field with field number 1 in the {@link
+ * FailureDetail}'s category submessage.
+ */
+ private static EnumValueDescriptor getSubcategoryDescriptor(
+ FailureDetail failureDetail, MessageOrBuilder categoryMsg) {
+ FieldDescriptor fieldNumberOne = categoryMsg.getDescriptorForType().findFieldByNumber(1);
+ checkNotNull(
+ fieldNumberOne, "FailureDetail category submessage has no field #1: %s", failureDetail);
+ Object fieldNumberOneVal = categoryMsg.getField(fieldNumberOne);
+ checkArgument(
+ fieldNumberOneVal instanceof EnumValueDescriptor,
+ "FailureDetail category submessage has non-enum field #1: %s",
+ failureDetail);
+ return (EnumValueDescriptor) fieldNumberOneVal;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/util/FailureDetailUtil.java b/src/main/java/com/google/devtools/build/lib/util/FailureDetailUtil.java
deleted file mode 100644
index 5f37275..0000000
--- a/src/main/java/com/google/devtools/build/lib/util/FailureDetailUtil.java
+++ /dev/null
@@ -1,114 +0,0 @@
-// 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.util;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import com.google.devtools.build.lib.server.FailureDetails;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
-import com.google.devtools.build.lib.server.FailureDetails.Interrupted;
-import com.google.protobuf.Descriptors.EnumValueDescriptor;
-import com.google.protobuf.Descriptors.FieldDescriptor;
-import com.google.protobuf.MessageOrBuilder;
-import java.util.Map;
-
-/** Utilities for constructing and managing {@link FailureDetail}s. */
-public class FailureDetailUtil {
-
- private FailureDetailUtil() {}
-
- /**
- * Returns a {@link FailureDetail} specifying {@code code} in its {@link Interrupted} submessage.
- */
- public static FailureDetail interrupted(Interrupted.Code code) {
- return FailureDetail.newBuilder()
- .setInterrupted(Interrupted.newBuilder().setCode(code))
- .build();
- }
-
- /** Returns the registered {@link ExitCode} associated with a {@link FailureDetail} message. */
- public static ExitCode getExitCode(FailureDetail failureDetail) {
- // TODO(mschaller): Consider specializing for unregistered exit codes here, if absolutely
- // necessary.
- int numericExitCode = FailureDetailUtil.getNumericExitCode(failureDetail);
- return checkNotNull(
- ExitCode.forCode(numericExitCode), "No ExitCode for numericExitCode %s", numericExitCode);
- }
-
- /** Returns the numeric exit code associated with a {@link FailureDetail} message. */
- private static int getNumericExitCode(FailureDetail failureDetail) {
- MessageOrBuilder categoryMsg = getCategorySubmessage(failureDetail);
- EnumValueDescriptor subcategoryDescriptor =
- getSubcategoryDescriptor(failureDetail, categoryMsg);
- return getNumericExitCode(subcategoryDescriptor);
- }
-
- /**
- * Returns the numeric exit code associated with a {@link FailureDetail} submessage's subcategory
- * enum value.
- */
- private static int getNumericExitCode(EnumValueDescriptor subcategoryDescriptor) {
- checkArgument(
- subcategoryDescriptor.getOptions().hasExtension(FailureDetails.metadata),
- "Enum value %s has no FailureDetails.metadata",
- subcategoryDescriptor);
- return subcategoryDescriptor.getOptions().getExtension(FailureDetails.metadata).getExitCode();
- }
-
- /**
- * Returns the category submessage, i.e. the message in {@link FailureDetail}'s oneof. Throws if
- * none of those fields are set.
- */
- private static MessageOrBuilder getCategorySubmessage(FailureDetail failureDetail) {
- MessageOrBuilder categoryMsg = null;
- for (Map.Entry<FieldDescriptor, Object> entry : failureDetail.getAllFields().entrySet()) {
- FieldDescriptor fieldDescriptor = entry.getKey();
- if (isCategoryField(fieldDescriptor)) {
- categoryMsg = (MessageOrBuilder) entry.getValue();
- break;
- }
- }
- return checkNotNull(
- categoryMsg, "FailureDetail missing category submessage: %s", failureDetail);
- }
-
- /**
- * Returns whether the {@link FieldDescriptor} describes a field in {@link FailureDetail}'s oneof.
- *
- * <p>Uses the field number criteria described in failure_details.proto.
- */
- private static boolean isCategoryField(FieldDescriptor fieldDescriptor) {
- int fieldNum = fieldDescriptor.getNumber();
- return 100 < fieldNum && fieldNum <= 10_000;
- }
-
- /**
- * Returns the enum value descriptor for the enum field with field number 1 in the {@link
- * FailureDetail}'s category submessage.
- */
- private static EnumValueDescriptor getSubcategoryDescriptor(
- FailureDetail failureDetail, MessageOrBuilder categoryMsg) {
- FieldDescriptor fieldNumberOne = categoryMsg.getDescriptorForType().findFieldByNumber(1);
- checkNotNull(
- fieldNumberOne, "FailureDetail category submessage has no field #1: %s", failureDetail);
- Object fieldNumberOneVal = categoryMsg.getField(fieldNumberOne);
- checkArgument(
- fieldNumberOneVal instanceof EnumValueDescriptor,
- "FailureDetail category submessage has non-enum field #1: %s",
- failureDetail);
- return (EnumValueDescriptor) fieldNumberOneVal;
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/util/BUILD b/src/test/java/com/google/devtools/build/lib/util/BUILD
index 8a3262a..dccd4d0 100644
--- a/src/test/java/com/google/devtools/build/lib/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/util/BUILD
@@ -38,8 +38,8 @@
deps = [
"//src/main/java/com/google/devtools/build/lib:build",
"//src/main/java/com/google/devtools/build/lib:command-utils",
+ "//src/main/java/com/google/devtools/build/lib:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib:exitcode-external",
- "//src/main/java/com/google/devtools/build/lib:failure_detail_util",
"//src/main/java/com/google/devtools/build/lib:filetype",
"//src/main/java/com/google/devtools/build/lib:os_util",
"//src/main/java/com/google/devtools/build/lib:resource-converter",
@@ -57,6 +57,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/testutil",
+ "//src/test/java/com/google/devtools/build/lib/util/subjects",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:junit4",
diff --git a/src/test/java/com/google/devtools/build/lib/util/DetailedExitCodeTest.java b/src/test/java/com/google/devtools/build/lib/util/DetailedExitCodeTest.java
new file mode 100644
index 0000000..24411a7
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/util/DetailedExitCodeTest.java
@@ -0,0 +1,40 @@
+// 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.util;
+
+import static com.google.devtools.build.lib.util.subjects.DetailedExitCodeSubjectFactory.assertThatDetailedExitCode;
+
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.Interrupted;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link DetailedExitCode}. */
+@RunWith(JUnit4.class)
+public class DetailedExitCodeTest {
+
+ @Test
+ public void testGetInterruptedExitCode() {
+ assertThatDetailedExitCode(
+ DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage("interrupted")
+ .setInterrupted(
+ Interrupted.newBuilder().setCode(Interrupted.Code.INTERRUPTED_UNSPECIFIED))
+ .build()))
+ .hasExitCode(ExitCode.INTERRUPTED);
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/util/FailureDetailUtilTest.java b/src/test/java/com/google/devtools/build/lib/util/FailureDetailUtilTest.java
deleted file mode 100644
index b589ebf..0000000
--- a/src/test/java/com/google/devtools/build/lib/util/FailureDetailUtilTest.java
+++ /dev/null
@@ -1,35 +0,0 @@
-// 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.util;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.devtools.build.lib.server.FailureDetails.Interrupted;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for {@link FailureDetailUtil}. */
-@RunWith(JUnit4.class)
-public class FailureDetailUtilTest {
-
- @Test
- public void testGetInterruptedExitCode() {
- assertThat(
- FailureDetailUtil.getExitCode(
- FailureDetailUtil.interrupted(Interrupted.Code.INTERRUPTED_UNSPECIFIED)))
- .isEqualTo(ExitCode.INTERRUPTED);
- }
-}
