"bazel clean": prevent creation of command.log

Open files cannot be deleted on Windows, thus
`bazel clean --expunge` fails when it attemps to
delete the `command.log` that stdout/stderr is
tee'd into, and so does BlazeCommandDispatcher
when it attemps to delete the `command.log` just
before dispatching to the command implementation
(not just `clean` but any command).

This change:
- closes `command.log` before we attempt to
  delete it
- marks CleanCommand (through the Command
  annotation) as one that should not write to the
  command.log, thus we don't create a new instance
  of the file

This change allows `bazel clean --expunge` to
delete everything in the output base, with the
exception of `jvm.log`. Unfortunately that file is
opened by the C++ bazel client process, so we have
to close it there prior to sending the clean to
the bazel server.

See https://github.com/bazelbuild/bazel/issues/1586

--
MOS_MIGRATED_REVID=137278553

3 files changed, 28 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 7d7f300bd9..87adbd7374 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -370,20 +370,21 @@ public class BlazeCommandDispatcher {
       return exitCausingException.getExitCode().getNumericExitCode();
     }
 
-    if (env.getRuntime().writeCommandLog()) {
-      try {
-        Path commandLog = getCommandLogPath(env.getOutputBase());
+    try {
+      Path commandLog = getCommandLogPath(env.getOutputBase());
 
-        // Unlink old command log from previous build, if present, so scripts
-        // reading it don't conflate it with the command log we're about to write.
-        commandLog.delete();
+      // Unlink old command log from previous build, if present, so scripts
+      // reading it don't conflate it with the command log we're about to write.
+      closeSilently(logOutputStream);
+      logOutputStream = null;
+      commandLog.delete();
 
+      if (env.getRuntime().writeCommandLog() && commandAnnotation.writeCommandLog()) {
         logOutputStream = commandLog.getOutputStream();
         outErr = tee(outErr, OutErr.create(logOutputStream, logOutputStream));
-      } catch (IOException ioException) {
-        LoggingUtil.logToRemote(
-            Level.WARNING, "Unable to delete or open command.log", ioException);
       }
+    } catch (IOException ioException) {
+      LoggingUtil.logToRemote(Level.WARNING, "Unable to delete or open command.log", ioException);
     }
 
     ExitCode result = checkCwdInWorkspace(env, commandAnnotation, commandName, outErr);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/Command.java b/src/main/java/com/google/devtools/build/lib/runtime/Command.java
index c415f0a4cf..4db562790c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/Command.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/Command.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.runtime;
 
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
-
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -89,6 +88,13 @@ public @interface Command {
   boolean binaryStdErr() default false;
 
   /**
+   * Returns true if this command may want to write to the command.log.
+   *
+   * <p>The clean command would typically set this to false so it can delete the command.log.
+   */
+  boolean writeCommandLog() default true;
+
+  /**
    * The help message for this command.  If the value starts with "resource:",
    * the remainder is interpreted as the name of a text file resource (in the
    * .jar file that provides the Command implementation class).
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
index 3b1c68fae7..961400edb1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -38,16 +38,17 @@ import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
 import java.util.logging.Logger;
 
-/**
- * Implements 'blaze clean'.
- */
-@Command(name = "clean",
-         builds = true,  // Does not, but people expect build options to be there
-         options = { CleanCommand.Options.class },
-         help = "resource:clean.txt",
-         shortDescription = "Removes output files and optionally stops the server.",
-         // TODO(bazel-team): Remove this - we inherit a huge number of unused options.
-         inherits = { BuildCommand.class })
+/** Implements 'blaze clean'. */
+@Command(
+  name = "clean",
+  builds = true, // Does not, but people expect build options to be there
+  writeCommandLog = false, // Do not create a command.log, otherwise we couldn't delete it.
+  options = {CleanCommand.Options.class},
+  help = "resource:clean.txt",
+  shortDescription = "Removes output files and optionally stops the server.",
+  // TODO(bazel-team): Remove this - we inherit a huge number of unused options.
+  inherits = {BuildCommand.class}
+)
 public final class CleanCommand implements BlazeCommand {
   /**
    * An interface for special options for the clean command.