Skip to content

Commit

Permalink
Forbid simultaneously requesting both a JSON and binary execution log.
Browse files Browse the repository at this point in the history
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 bazelbuild#18643.

PiperOrigin-RevId: 582994996
Change-Id: I8e0bc228c73a0da7f3e7b830ade2cab6c14319ff
  • Loading branch information
tjgq authored and copybara-github committed Nov 16, 2023
1 parent 51bddee commit 4b7c808
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 4b7c808

Please sign in to comment.