Repository: bazelbuild/bazel
Issue: --execution_log_binary_file negation
Labels: type: bug
team-Remote-Exec

Using bazel 0.26.0rc10 and `--execution_log_binary_file`, you cannot negate the argument with `--execution_log_binary_file=` as you could with `experimental_execution_log_file`. I'm not sure if it was intentional behavior before but previously passing:

```
bazel build //... --experimental_execution_log_file=foo.log --experimental_execution_log_file=
```

Would not write an execution log file. This is useful for when you have a configuration that implies `experimental_execution_log_file`, but for some reason you don't want to write it just for this build.

Issue comments:
That's not an intended change (@agoulti). I think it should be possible to negate all our flags. WDYT @ishikhman?
I agree, we should keep the possibility to negate/disable all flags. It feels to me like a bug in a recent change that introduced a new set of flags.
Note that I just discovered that this is broken with `--experimental_execution_log_file` as well, which causes issues for us even without changing to the new flag.
I've submitted a fix for this https://github.com/bazelbuild/bazel/pull/8428

Pull request review comments:

Patch that resolved the issue:
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 dea75cf9938187..a2bb9cd30c96dd 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
@@ -73,7 +73,7 @@ private void initOutputs(CommandEnvironment env) throws IOException {
     FileSystem fileSystem = env.getRuntime().getFileSystem();
     Path workingDirectory = env.getWorkingDirectory();
 
-    if (executionOptions.executionLogBinaryFile != null) {
+    if (executionOptions.executionLogBinaryFile != null && !executionOptions.executionLogBinaryFile.isEmpty()) {
       outputStreams.addStream(
           new BinaryOutputStreamWrapper(
               workingDirectory
@@ -81,7 +81,7 @@ private void initOutputs(CommandEnvironment env) throws IOException {
                   .getOutputStream()));
     }
 
-    if (executionOptions.executionLogJsonFile != null) {
+    if (executionOptions.executionLogJsonFile != null && !executionOptions.executionLogJsonFile.isEmpty()) {
       outputStreams.addStream(
           new JsonOutputStreamWrapper(
               workingDirectory
@@ -90,7 +90,7 @@ private void initOutputs(CommandEnvironment env) throws IOException {
     }
 
     AsynchronousFileOutputStream outStream = null;
-    if (executionOptions.executionLogFile != null) {
+    if (executionOptions.executionLogFile != null && !executionOptions.executionLogFile.isEmpty()) {
       rawOutput = workingDirectory.getRelative(executionOptions.executionLogFile);
       outStream =
           new AsynchronousFileOutputStream(


Test patch:
diff --git a/src/test/shell/bazel/bazel_execlog_test.sh b/src/test/shell/bazel/bazel_execlog_test.sh
index 6dfbdc251048f8..89b9e8443559d0 100755
--- a/src/test/shell/bazel/bazel_execlog_test.sh
+++ b/src/test/shell/bazel/bazel_execlog_test.sh
@@ -119,4 +119,28 @@ EOF
   wc output || fail "no output produced"
 }
 
+function test_negating_flags() {
+  cat > BUILD <<'EOF'
+genrule(
+      name = "rule",
+      outs = ["out.txt"],
+      cmd = "echo hello > $(location out.txt)"
+)
+EOF
+  bazel build //:all --experimental_execution_log_file=output --experimental_execution_log_file= 2>&1 >> $TEST_log || fail "could not build"
+  if [[ -e output ]]; then
+    fail "file shouldn't exist"
+  fi
+
+  bazel build //:all --execution_log_json_file=output --execution_log_json_file= 2>&1 >> $TEST_log || fail "could not build"
+  if [[ -e output ]]; then
+    fail "file shouldn't exist"
+  fi
+
+  bazel build //:all --execution_log_binary_file=output --execution_log_binary_file= 2>&1 >> $TEST_log || fail "could not build"
+  if [[ -e output ]]; then
+    fail "file shouldn't exist"
+  fi
+}
+
 run_suite "execlog_tests"