Repository: bazelbuild/bazel
Issue: bazel clean: don't suggest to use --async, if it is not supported on current platform
Labels: type: bug
P3
category: misc > misc

On Bazel@HEAD, on Windows:

```
  $ bazel clean
  INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
```

Now trying to pass `--async`, as suggested:

```
  $ bazel clean --async
  INFO: --async cannot be used on non-Linux platforms, falling back to synchronous clean
  INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
```

Issue comments:
Sounds like a nice improvement.

@akira-baruah do you want to send your fix over as a pull request? :)
@philwo Thanks - I made PR #4236, but haven't tested it on Windows yet.

Pull request review comments:

Patch that resolved the issue:
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 dfe0dd3bb1f697..6c6446214cc086 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
@@ -126,7 +126,8 @@ public ExitCode exec(CommandEnvironment env, OptionsProvider options)
     // MacOS and FreeBSD support setsid(2) but don't have /usr/bin/setsid, so if we wanted to
     // support --expunge_async on these platforms, we'd have to write a wrapper that calls setsid(2)
     // and exec(2).
-    if (async && OS.getCurrent() != OS.LINUX) {
+    boolean asyncSupport = OS.getCurrent() == OS.LINUX;
+    if (async && !asyncSupport) {
       String fallbackName = cleanOptions.expunge ? "--expunge" : "synchronous clean";
       env.getReporter()
           .handle(
@@ -138,7 +139,7 @@ public ExitCode exec(CommandEnvironment env, OptionsProvider options)
     }
 
     String cleanBanner =
-        async
+        (async || !asyncSupport)
             ? "Starting clean."
             : "Starting clean (this may take a while). "
                 + "Consider using --async if the clean takes more than several minutes.";


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index ba38e5698eb4bf..78d1569b98e80c 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1029,7 +1029,10 @@ java_test(
 
 java_test(
     name = "runtime-tests",
-    srcs = glob(["runtime/*.java"]),
+    srcs = glob([
+        "runtime/*.java",
+        "runtime/commands/*.java"
+    ]),
     test_class = "com.google.devtools.build.lib.AllTests",
     deps = [
         ":foundations_testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/commands/CleanCommandTest.java b/src/test/java/com/google/devtools/build/lib/runtime/commands/CleanCommandTest.java
new file mode 100644
index 00000000000000..98216924a9e552
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/runtime/commands/CleanCommandTest.java
@@ -0,0 +1,106 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.runtime.commands;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.runtime.BlazeCommand;
+import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher;
+import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode;
+import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.runtime.BlazeServerStartupOptions;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.io.RecordingOutErr;
+import com.google.devtools.common.options.OptionsParser;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests {@link CleanCommand}.
+ */
+@RunWith(JUnit4.class)
+public class CleanCommandTest {
+
+  private final RecordingOutErr outErr = new RecordingOutErr();
+  private Scratch scratch = new Scratch();
+  private BlazeRuntime runtime;
+  private BlazeCommand command;
+  private BlazeCommandDispatcher dispatcher;
+
+  @Before
+  public final void initializeRuntime() throws Exception {
+    String productName = TestConstants.PRODUCT_NAME;
+    ServerDirectories serverDirectories =
+        new ServerDirectories(scratch.dir("install"), scratch.dir("output"));
+    this.runtime =
+        new BlazeRuntime.Builder()
+            .setFileSystem(scratch.getFileSystem())
+            .setProductName(productName)
+            .setServerDirectories(serverDirectories)
+            .setStartupOptionsProvider(
+                OptionsParser.newOptionsParser(BlazeServerStartupOptions.class))
+            .addBlazeModule(
+                new BlazeModule() {
+                  @Override
+                  public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
+                    // We must add these options so that the defaults package can be created.
+                    builder.addConfigurationOptions(BuildConfiguration.Options.class);
+                    // The tools repository is needed for createGlobals
+                    builder.setToolsRepository(TestConstants.TOOLS_REPOSITORY);
+                  }
+                })
+            .build();
+    BlazeDirectories directories =
+        new BlazeDirectories(serverDirectories, scratch.dir("workspace"), productName);
+    this.runtime.initWorkspace(directories, /* binTools= */ null);
+    this.command = new CleanCommand();
+    this.dispatcher = new BlazeCommandDispatcher(this.runtime, this.command);
+  }
+
+  @Test
+  public void testCleanWithAsyncDoesNotSuggestAsync() throws Exception {
+    List<String> commandLine = Lists.newArrayList("clean", "--async");
+    this.dispatcher.exec(commandLine, LockingMode.ERROR_OUT, "test", outErr);
+    String output = outErr.toString();
+    String suggestion = "Consider using --async";
+    assertWithMessage("clean --async command shouldn't suggest using --async")
+        .that(output).doesNotContain(suggestion);
+  }
+
+  @Test
+  public void testCleanSuggestsAsyncOnLinuxPlatformsOnly() throws Exception {
+    List<String> commandLine = Lists.newArrayList("clean");
+    this.dispatcher.exec(commandLine, LockingMode.ERROR_OUT, "test", outErr);
+    String output = outErr.toString();
+    String suggestion = "Consider using --async";
+    if (OS.getCurrent() == OS.LINUX) {
+      assertWithMessage("clean command should suggest using --async on Linux platforms")
+          .that(output).contains(suggestion);
+    } else {
+      assertWithMessage("clean command shouldn't suggest using --async on non-Linux platforms")
+          .that(output).doesNotContain(suggestion);
+    }
+  }
+}