From 7f3ffbca55749b3720041fd9ab5b0b0962937039 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Wed, 26 Oct 2016 15:31:38 +0000 Subject: "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 --- .../build/lib/runtime/BlazeCommandDispatcher.java | 19 ++++++++++--------- .../google/devtools/build/lib/runtime/Command.java | 8 +++++++- .../build/lib/runtime/commands/CleanCommand.java | 21 +++++++++++---------- 3 files changed, 28 insertions(+), 20 deletions(-) (limited to 'src/main') 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; @@ -88,6 +87,13 @@ public @interface Command { */ boolean binaryStdErr() default false; + /** + * Returns true if this command may want to write to the command.log. + * + *

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 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. -- cgit v1.2.3