Fix clean command to not use boolean options

Using boolean options with expansions on them meant they were still used if in the --no(flag) case.

RELNOTES: Clean command no longer uses boolean values for --async, --expunge, and --expunge_async options.
PiperOrigin-RevId: 157465859

1 files changed, 32 insertions, 31 deletions

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 8dd171b48a..c007e5f61d 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
@@ -63,7 +63,7 @@ public final class CleanCommand implements BlazeCommand {
 
     @Option(
       name = "expunge",
-      defaultValue = "false",
+      defaultValue = "null",
       category = "clean",
       expansion = "--clean_style=expunge",
       help =
@@ -71,11 +71,11 @@ public final class CleanCommand implements BlazeCommand {
               + "instance, which includes all %{product}-created temporary and build output "
               + "files, and stops the %{product} server if it is running."
     )
-    public boolean expunge;
+    public Void expunge;
 
     @Option(
       name = "expunge_async",
-      defaultValue = "false",
+      defaultValue = "null",
       category = "clean",
       expansion = "--clean_style=expunge_async",
       help =
@@ -85,11 +85,11 @@ public final class CleanCommand implements BlazeCommand {
               + "this command completes, it will be safe to execute new commands in the same "
               + "client, even though the deletion may continue in the background."
     )
-    public boolean expunge_async;
+    public Void expungeAsync;
 
     @Option(
       name = "async",
-      defaultValue = "false",
+      defaultValue = "null",
       category = "clean",
       expansion = "--clean_style=async",
       help =
@@ -99,7 +99,7 @@ public final class CleanCommand implements BlazeCommand {
               + "commands in the same client, even though the deletion may continue in the "
               + "background."
     )
-    public boolean async;
+    public Void async;
   }
 
   /**
@@ -123,29 +123,24 @@ public final class CleanCommand implements BlazeCommand {
   public ExitCode exec(CommandEnvironment env, OptionsProvider options)
       throws ShutdownBlazeServerException {
     Options cleanOptions = options.getOptions(Options.class);
-    cleanOptions.expunge_async = cleanOptions.cleanStyle.equals("expunge_async");
-    cleanOptions.expunge = cleanOptions.cleanStyle.equals("expunge");
-    cleanOptions.async = cleanOptions.cleanStyle.equals("async");
+    boolean expungeAsync = cleanOptions.cleanStyle.equals("expunge_async");
+    boolean expunge = cleanOptions.cleanStyle.equals("expunge");
+    boolean async = cleanOptions.cleanStyle.equals("async");
 
     env.getEventBus().post(new NoBuildEvent());
 
-    if (!cleanOptions.expunge
-        && !cleanOptions.expunge_async
-        && !cleanOptions.async
-        && !cleanOptions.cleanStyle.isEmpty()) {
+    if (!expunge && !expungeAsync && !async && !cleanOptions.cleanStyle.isEmpty()) {
       env.getReporter().handle(Event.error(
           null, "Invalid clean_style value '" + cleanOptions.cleanStyle + "'"));
       return ExitCode.COMMAND_LINE_ERROR;
     }
 
-    String asyncName =
-        (cleanOptions.expunge || cleanOptions.expunge_async) ? "--expunge_async" : "--async";
+    String asyncName = (expunge || expungeAsync) ? "--expunge_async" : "--async";
 
     // TODO(dmarting): Deactivate expunge_async on non-Linux platform until we completely fix it
     // for non-Linux platforms (https://github.com/bazelbuild/bazel/issues/1906).
-    if ((cleanOptions.expunge_async || cleanOptions.async) && OS.getCurrent() != OS.LINUX) {
-      boolean expunge = cleanOptions.expunge_async;
-      String fallbackName = expunge ? "--expunge" : "synchronous clean";
+    if ((expungeAsync || async) && OS.getCurrent() != OS.LINUX) {
+      String fallbackName = expungeAsync ? "--expunge" : "synchronous clean";
       env.getReporter()
           .handle(
               Event.info(
@@ -153,14 +148,14 @@ public final class CleanCommand implements BlazeCommand {
                   asyncName
                       + " cannot be used on non-Linux platforms, falling back to "
                       + fallbackName));
-      cleanOptions.expunge_async = false;
-      cleanOptions.expunge = expunge;
-      cleanOptions.async = false;
+      expunge = expungeAsync;
+      expungeAsync = false;
+      async = false;
       cleanOptions.cleanStyle = expunge ? "expunge" : "";
     }
 
     String cleanBanner =
-        (cleanOptions.expunge_async || cleanOptions.async)
+        (expungeAsync || async)
             ? "Starting clean."
             : "Starting clean (this may take a while). "
                 + "Consider using "
@@ -173,7 +168,7 @@ public final class CleanCommand implements BlazeCommand {
     try {
       String symlinkPrefix = options.getOptions(BuildRequest.BuildRequestOptions.class)
           .getSymlinkPrefix(env.getRuntime().getProductName());
-      actuallyClean(env, env.getOutputBase(), cleanOptions, symlinkPrefix);
+      actuallyClean(env, env.getOutputBase(), expunge, expungeAsync, async, symlinkPrefix);
       return ExitCode.SUCCESS;
     } catch (IOException e) {
       env.getReporter().handle(Event.error(e.getMessage()));
@@ -216,14 +211,20 @@ public final class CleanCommand implements BlazeCommand {
         .execute();
   }
 
-  private void actuallyClean(CommandEnvironment env,
-      Path outputBase, Options cleanOptions, String symlinkPrefix) throws IOException,
-      ShutdownBlazeServerException, CommandException, ExecException, InterruptedException {
+  private void actuallyClean(
+      CommandEnvironment env,
+      Path outputBase,
+      boolean expunge,
+      boolean expungeAsync,
+      boolean async,
+      String symlinkPrefix)
+      throws IOException, ShutdownBlazeServerException, CommandException, ExecException,
+          InterruptedException {
     String workspaceDirectory = env.getWorkspace().getBaseName();
     if (env.getOutputService() != null) {
       env.getOutputService().clean();
     }
-    if (cleanOptions.expunge) {
+    if (expunge) {
       LOG.info("Expunging...");
       env.getRuntime().prepareForAbruptShutdown();
       // Delete the big subdirectories with the important content first--this
@@ -233,7 +234,7 @@ public final class CleanCommand implements BlazeCommand {
       // all significant files will be gone by then.
       FileSystemUtils.deleteTreesBelow(outputBase);
       FileSystemUtils.deleteTree(outputBase);
-    } else if (cleanOptions.expunge_async) {
+    } else if (expungeAsync) {
       LOG.info("Expunging asynchronously...");
       env.getRuntime().prepareForAbruptShutdown();
       asyncClean(env, outputBase, "Output base");
@@ -245,8 +246,8 @@ public final class CleanCommand implements BlazeCommand {
       for (String directory : new String[] {workspaceDirectory, "execroot"}) {
         Path child = outputBase.getRelative(directory);
         if (child.exists()) {
-          LOG.finest("Cleaning " + child + (cleanOptions.async ? " asynchronously..." : ""));
-          if (cleanOptions.async) {
+          LOG.finest("Cleaning " + child + (async ? " asynchronously..." : ""));
+          if (async) {
             asyncClean(env, child, "Output tree");
           } else {
             FileSystemUtils.deleteTreesBelow(child);
@@ -259,7 +260,7 @@ public final class CleanCommand implements BlazeCommand {
         workspaceDirectory, env.getWorkspace(), env.getReporter(),
         symlinkPrefix, env.getRuntime().getProductName());
     // shutdown on expunge cleans
-    if (cleanOptions.expunge || cleanOptions.expunge_async) {
+    if (expunge || expungeAsync) {
       throw new ShutdownBlazeServerException(0);
     }
     System.gc();