From 4b7c808ec91333bffa84810aa6d712896cf98b37 Mon Sep 17 00:00:00 2001
From: Googler <tjgq@google.com>
Date: Thu, 16 Nov 2023 04:33:08 -0800
Subject: [PATCH] Forbid simultaneously requesting both a JSON and binary
 execution log.

Although this is a breaking change, I find it unlikely that anyone requests both, given how unwieldy these files are.

This avoids future complexity around format conversions as we add more of them (specifically: in order to add a new format while keeping the old formats equally performant, the choice of "raw" format would likely have to vary, so there would be additional pairs to convert between).

The current conversion code can also be simplified, as there's now at most one conversion to make, but I prefer doing that separately.

Related to https://github.com/bazelbuild/bazel/issues/18643.

PiperOrigin-RevId: 582994996
Change-Id: I8e0bc228c73a0da7f3e7b830ade2cab6c14319ff
---
 .../build/lib/bazel/SpawnLogModule.java       | 20 +++++++++++++++++++
 .../build/lib/exec/ExecutionOptions.java      |  4 ++--
 src/main/protobuf/failure_details.proto       |  1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
index aa2ba003f0b452..1d041efa251d6e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.server.FailureDetails;
 import com.google.devtools.build.lib.server.FailureDetails.Execution;
 import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
@@ -87,6 +88,25 @@ private void initOutputs(CommandEnvironment env) throws IOException {
       return;
     }
 
+    if (executionOptions.executionLogBinaryFile != null
+        && executionOptions.executionLogJsonFile != null) {
+      String message =
+          "Must specify at most one of --execution_log_json_file and --execution_log_binary_file";
+      env.getBlazeModuleEnvironment()
+          .exit(
+              new AbruptExitException(
+                  DetailedExitCode.of(
+                      FailureDetail.newBuilder()
+                          .setMessage(message)
+                          .setExecutionOptions(
+                              FailureDetails.ExecutionOptions.newBuilder()
+                                  .setCode(
+                                      FailureDetails.ExecutionOptions.Code
+                                          .MULTIPLE_EXECUTION_LOG_FORMATS))
+                          .build())));
+      return;
+    }
+
     this.env = env;
 
     Path workingDirectory = env.getWorkingDirectory();
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index 5b688ba8cf8a17..96dddc660b2b95 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -399,7 +399,7 @@ public boolean usingLocalTestJobs() {
       help =
           "Log the executed spawns into this file as delimited Spawn protos, according to"
               + " src/main/protobuf/spawn.proto. Related flags:"
-              + " --execution_log_json_file (text JSON format),"
+              + " --execution_log_json_file (text JSON format; mutually exclusive),"
               + " --execution_log_sort (whether to sort the execution log),"
               + " --subcommands (for displaying subcommands in terminal output).")
   public PathFragment executionLogBinaryFile;
@@ -414,7 +414,7 @@ public boolean usingLocalTestJobs() {
       help =
           "Log the executed spawns into this file as a JSON representation of the delimited Spawn"
               + " protos, according to src/main/protobuf/spawn.proto. Related flags:"
-              + " --execution_log_binary_file (binary protobuf format),"
+              + " --execution_log_binary_file (binary protobuf format; mutually exclusive),"
               + " --execution_log_sort (whether to sort the execution log),"
               + " --subcommands (for displaying subcommands in terminal output).")
   public PathFragment executionLogJsonFile;
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index e2f6ed867f7c07..ed165063702a7c 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -528,6 +528,7 @@ message ExecutionOptions {
         [(metadata) = { exit_code: 2 }];
     STRATEGY_NOT_FOUND = 9 [(metadata) = { exit_code: 2 }];
     DYNAMIC_STRATEGY_NOT_SANDBOXED = 10 [(metadata) = { exit_code: 2 }];
+    MULTIPLE_EXECUTION_LOG_FORMATS = 11 [(metadata) = { exit_code: 2 }];
 
     reserved 1, 2; // For internal use
   }