Split BlazeModule.getCommandOptions into two; unify all implementations.

Several modules now explicitly add common command options. Of the remaining
ones, most add options to the build command, except one, which adds options
to query. They now all use the canonical implementation.

Also updated the documentation to clarify what this method actually does.

--
MOS_MIGRATED_REVID=125560058

10 files changed, 46 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
index dece6e0cf0..4704497d4f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -270,7 +270,7 @@ public class BazelWorkspaceStatusModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return command.builds()
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(WorkspaceStatusAction.Options.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java b/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
index 79c753f7d3..192dc60b40 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
@@ -106,7 +106,7 @@ public class DashModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return (command.name().equals("build") || command.name().equals("test"))
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(DashOptions.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
index a0c142e096..0e6b8accc2 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -153,7 +153,7 @@ public class BazelRulesModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return command.builds()
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(BazelExecutionOptions.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 88e7f86e58..ff772217b4 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -88,7 +88,7 @@ public final class RemoteModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return command.builds()
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(RemoteOptions.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
index 722baea810..8e42ad8f41 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.runtime;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
@@ -59,8 +60,14 @@ public class BlazeCommandUtils {
     return ImmutableList.copyOf(options);
   }
 
-  public static ImmutableList<Class<? extends OptionsBase>> getCommonOptions() {
-    return COMMON_COMMAND_OPTIONS;
+  public static ImmutableSet<Class<? extends OptionsBase>> getCommonOptions(
+      Iterable<BlazeModule> modules) {
+    ImmutableSet.Builder<Class<? extends OptionsBase>> builder = ImmutableSet.builder();
+    builder.addAll(COMMON_COMMAND_OPTIONS);
+    for (BlazeModule blazeModule : modules) {
+      builder.addAll(blazeModule.getCommonCommandOptions());
+    }
+    return builder.build();
   }
 
   /**
@@ -80,7 +87,7 @@ public class BlazeCommandUtils {
     }
 
     Set<Class<? extends OptionsBase>> options = new HashSet<>();
-    options.addAll(COMMON_COMMAND_OPTIONS);
+    options.addAll(getCommonOptions(modules));
     Collections.addAll(options, commandAnnotation.options());
 
     if (commandAnnotation.usesConfigurationOptions()) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index b6e5d45637..79d0c935c8 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -245,16 +245,41 @@ public abstract class BlazeModule {
   }
 
   /**
-   * Returns the extra options this module contributes to a specific command.
+   * Returns extra options this module contributes to a specific command. Note that option
+   * inheritance applies: if this method returns a non-empty list, then the returned options are
+   * added to every command that depends on this command.
    *
-   * <p>This method will be called at the beginning of each command (after #beforeCommand).
+   * <p>This method may be called at any time, and the returned value may be cached. Implementations
+   * must be thread-safe and never return different lists for the same command object. Typical
+   * implementations look like this:
+   * <pre>
+   * return "build".equals(command.name())
+   *     ? ImmutableList.<Class<? extends OptionsBase>>of(MyOptions.class)
+   *     : ImmutableList.<Class<? extends OptionsBase>>of();
+   * </pre>
+   * Note that this example adds options to all commands that inherit from the build command.
+   *
+   * <p>This method is also used to generate command-line documentation; in order to avoid
+   * duplicated options descriptions, this method should never return the same options class for two
+   * different commands if one of them inherits the other.
+   *
+   * <p>If you want to add options to all commands, override {@link #getCommonCommandOptions}
+   * instead.
+   *
+   * @param command the command
    */
-  @SuppressWarnings("unused")
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
     return ImmutableList.of();
   }
 
   /**
+   * Returns extra options this module contributes to all commands.
+   */
+  public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
+    return ImmutableList.of();
+  }
+
+  /**
    * Returns a map of option categories to descriptive strings. This is used by {@code HelpCommand}
    * to show a more readable list of flags.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java
index ba199923a1..79875d8b7e 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java
@@ -355,7 +355,7 @@ public final class HelpCommand implements BlazeCommand {
       result.append("\n");
 
       result.append("<h2><a name=\"common_options\">Options Common to all Commands</a></h2>\n");
-      appendOptionsHtml(result, BlazeCommandUtils.getCommonOptions());
+      appendOptionsHtml(result, BlazeCommandUtils.getCommonOptions(runtime.getBlazeModules()));
       result.append("\n");
 
       for (Map.Entry<String, BlazeCommand> e : commandsByName.entrySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
index 7187937a74..04f6169715 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
@@ -85,7 +85,7 @@ public class SandboxModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return command.builds()
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(SandboxOptions.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java b/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
index a41a3baa38..00b5ad370c 100644
--- a/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
+++ b/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.ssd;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.cache.DigestUtils;
 import com.google.devtools.build.lib.runtime.BlazeModule;
-import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
 
@@ -26,7 +25,7 @@ import com.google.devtools.common.options.OptionsProvider;
  */
 public final class SsdModule extends BlazeModule {
   @Override
-  public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
+  public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
     return ImmutableList.<Class<? extends OptionsBase>>of(SsdOptions.class);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
index 63bade2086..b69879778f 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
@@ -45,7 +45,7 @@ public class WorkerModule extends BlazeModule {
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return command.builds()
+    return "build".equals(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(WorkerOptions.class)
         : ImmutableList.<Class<? extends OptionsBase>>of();
   }