Make specific options win over common ones

When specifying which command names to parse options for, given the
command annotation, ensure that "common" comes first, so that it can
be overridden by more specific options.

--
Change-Id: I20cd90c098e993580c32297d1e36c8a6c939873a
Reviewed-on: https://bazel-review.googlesource.com/#/c/3091
MOS_MIGRATED_REVID=117821257

3 files changed, 146 insertions, 4 deletions

diff --git a/BUILD b/BUILD
index 3852863b59..295515d1e6 100644
--- a/BUILD
+++ b/BUILD
@@ -22,9 +22,9 @@ filegroup(
         ["**"],
         exclude = [
             "bazel-*/**",  # convenience symlinks
-            "out/**"       # IntelliJ with setup-intellij.sh
-            "output/**",   # output of compile.sh
-            ".*/**",       # mainly git
+            "out/**" +  # IntelliJ with setup-intellij.sh
+            "output/**",  # output of compile.sh
+            ".*/**",  # mainly git
         ],
     ) + [
         "//examples:srcs",
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 a426f47fd5..38021d518a 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
@@ -489,7 +489,7 @@ public class BlazeCommandDispatcher {
     List<String> result = new ArrayList<>();
     getCommandNamesToParseHelper(commandAnnotation, result);
     result.add("common");
-    // TODO(bazel-team): This statement is a NO-OP: Lists.reverse(result);
+    result = Lists.reverse(result);
     return result;
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherRcoptionsTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherRcoptionsTest.java
new file mode 100644
index 0000000000..000f79d42b
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherRcoptionsTest.java
@@ -0,0 +1,142 @@
+// Copyright 2016 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;
+
+import static org.junit.Assert.assertEquals;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory;
+import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
+import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.ShutdownBlazeServerException;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.util.io.RecordingOutErr;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsProvider;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+
+import java.util.List;
+
+/**
+ * Tests the handling of rc-options in {@link BlazeCommandDispatcher}.
+ */
+@RunWith(JUnit4.class)
+public class BlazeCommandDispatcherRcoptionsTest {
+
+  /**
+   * Example options to be used by the tests.
+   */
+  public static class FooOptions extends OptionsBase {
+    @Option(name = "numoption", defaultValue = "0")
+    public int numOption;
+  }
+
+  @Command(
+    name = "reportnum",
+    options = {FooOptions.class},
+    shortDescription = "",
+    help = ""
+  )
+  private static class ReportNumCommand implements BlazeCommand {
+
+    @Override
+    public ExitCode exec(CommandEnvironment env, OptionsProvider options)
+        throws ShutdownBlazeServerException {
+      FooOptions fooOptions = options.getOptions(FooOptions.class);
+      env.getReporter().getOutErr().printOut("" + fooOptions.numOption);
+      return ExitCode.SUCCESS;
+    }
+
+    @Override
+    public void editOptions(CommandEnvironment env, OptionsParser optionsParser) {}
+  }
+
+  private final Scratch scratch = new Scratch();
+  private final RecordingOutErr outErr = new RecordingOutErr();
+  private final ReportNumCommand reportNum = new ReportNumCommand();
+  private BlazeRuntime runtime;
+
+  @Before
+  public final void initializeRuntime() throws Exception {
+    BlazeDirectories directories =
+        new BlazeDirectories(
+            scratch.dir("install_base"), scratch.dir("output_base"), scratch.dir("pkg"));
+    this.runtime =
+        new BlazeRuntime.Builder()
+            .setDirectories(directories)
+            .setStartupOptionsProvider(
+                OptionsParser.newOptionsParser(BlazeServerStartupOptions.class))
+            .setConfigurationFactory(
+                new ConfigurationFactory(Mockito.mock(ConfigurationCollectionFactory.class)))
+            .build();
+  }
+
+  @Test
+  public void testCommonUsed() throws Exception {
+    List<String> blazercOpts =
+        ImmutableList.of(
+            "--rc_source=/home/jrluser/.blazerc", "--default_override=0:common=--numoption=99");
+
+    BlazeCommandDispatcher dispatch = new BlazeCommandDispatcher(runtime, reportNum);
+    List<String> cmdLine = Lists.newArrayList("reportnum");
+    cmdLine.addAll(blazercOpts);
+
+    dispatch.exec(cmdLine, outErr);
+    String out = outErr.outAsLatin1();
+    assertEquals("Common options should be used", "99", out);
+  }
+
+  @Test
+  public void testSpecificOptionsWin() throws Exception {
+    List<String> blazercOpts =
+        ImmutableList.of(
+            "--rc_source=/home/jrluser/.blazerc",
+            "--default_override=0:reportnum=--numoption=42",
+            "--default_override=0:common=--numoption=99");
+
+    BlazeCommandDispatcher dispatch = new BlazeCommandDispatcher(runtime, reportNum);
+    List<String> cmdLine = Lists.newArrayList("reportnum");
+    cmdLine.addAll(blazercOpts);
+
+    dispatch.exec(cmdLine, outErr);
+    String out = outErr.outAsLatin1();
+    assertEquals("Specific options should dominate common options", "42", out);
+  }
+
+  @Test
+  public void testSpecificOptionsWinOtherOrder() throws Exception {
+    List<String> blazercOpts =
+        ImmutableList.of(
+            "--rc_source=/home/jrluser/.blazerc",
+            "--default_override=0:common=--numoption=99",
+            "--default_override=0:reportnum=--numoption=42");
+
+    BlazeCommandDispatcher dispatch = new BlazeCommandDispatcher(runtime, reportNum);
+    List<String> cmdLine = Lists.newArrayList("reportnum");
+    cmdLine.addAll(blazercOpts);
+
+    dispatch.exec(cmdLine, outErr);
+    String out = outErr.outAsLatin1();
+    assertEquals("Specific options should dominate common options", "42", out);
+  }
+}