Support multiple --bazelrc on command line

src/main/cpp/blaze_util.cc

@@ -14,6 +14,7 @@
 
 #include "src/main/cpp/blaze_util.h"
 
+#include <algorithm>
 #include <fcntl.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -35,6 +36,7 @@
 namespace blaze {
 
 using std::map;
+using std::min;
 using std::string;
 using std::vector;
 
@@ -74,6 +76,49 @@ bool GetNullaryOption(const char *arg, const char *key) {
   return true;
 }
 
+std::vector<std::string> GetAllUnaryOptionValues(const vector<string>& args,
+                                                 const char *key,
+                                                 const char *ignore_after_value) {
+  vector<std::string> values;
+  for (vector<string>::size_type i = 0; i < args.size(); ++i) {
+    if (args[i] == "--") {
+      // "--" means all remaining args aren't options
+      return values;
+    }
+
+    const char* next_arg = args[std::min(i + 1, args.size() - 1)].c_str();
+    const char* result = GetUnaryOption(args[i].c_str(),
+                                        next_arg,
+                                        key);
+    if (result != nullptr) {
+      // 'key' was found and 'result' has its value.
+      values.push_back(result);
+    }
+    // This is a pointer comparison, so equality means that the result must be from the next arg
+    // instead of happening to match the value from "--key=<value>" string, in which case
+    // we need to advance the index to skip the next arg for later iterations.
+    if (result == next_arg) {
+      i++;
+    }
+  }
+
+  if (ignore_after_value == nullptr) {
+    return values;
+  }
+  else {
+    vector<std::string> new_values;
+    std::string ignore_after_value_str = std::string(ignore_after_value);
+    for (vector<string>::size_type i = 0; i < values.size(); ++i) {
+      std::string curr_val = values[i];
+      new_values.push_back(curr_val);
+      if (curr_val == ignore_after_value_str) {
+        break;
+      }
+    }
+    return new_values;
+  }
+}
+
 const char* SearchUnaryOption(const vector<string>& args,
                               const char *key, bool warn_if_dupe) {
   if (args.empty()) {

src/main/cpp/blaze_util.h

@@ -53,6 +53,16 @@ bool GetNullaryOption(const char *arg, const char *key);
 const char* SearchUnaryOption(const std::vector<std::string>& args,
                               const char* key, bool warn_if_dupe);
 
+// Searches for 'key' in 'args' using GetUnaryOption. Arguments found after '--'
+// are omitted from the search.
+// If 'ignore_after_value' is not nullptr, all values matching the 'key' following
+// 'ignore_after_value' will be ignored.
+// Returns the values of the 'key' flag iff it occurs in args.
+// Returns empty vector otherwise.
+std::vector<std::string> GetAllUnaryOptionValues(const std::vector<std::string>& args,
+                                                 const char* key,
+                                                 const char* ignore_after_value=nullptr);
+
 // Searches for '--flag_name' and '--noflag_name' in 'args' using
 // GetNullaryOption. Arguments found after '--' are omitted from the search.
 // Returns true if '--flag_name' is a flag in args and '--noflag_name' does not

src/main/cpp/option_processor.cc

@@ -201,11 +201,22 @@ std::set<std::string> GetOldRcPaths(
         internal::FindRcAlongsideBinary(cwd, path_to_binary);
     candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path};
   }
-  string user_bazelrc_path = internal::FindLegacyUserBazelrc(
-      SearchUnaryOption(startup_args, "--bazelrc", /* warn_if_dupe */ true),
-      workspace);
-  if (!user_bazelrc_path.empty()) {
-    candidate_bazelrc_paths.push_back(user_bazelrc_path);
+  vector<std::string> cmd_line_rc_files =
+      GetAllUnaryOptionValues(startup_args, "--bazelrc", "/dev/null");
+  // Divide the cases where the vector is empty vs not, as `FindLegacyUserBazelrc`
+  // has a case for rc_file to be a nullptr.
+  if (cmd_line_rc_files.empty()){
+    string user_bazelrc_path = internal::FindLegacyUserBazelrc(nullptr, workspace);
+    if (!user_bazelrc_path.empty()) {
+      candidate_bazelrc_paths.push_back(user_bazelrc_path);
+    }
+  } else {
+    for (auto& rc_file : cmd_line_rc_files) {
+      string user_bazelrc_path = internal::FindLegacyUserBazelrc(rc_file.c_str(), workspace);
+      if (!user_bazelrc_path.empty()) {
+        candidate_bazelrc_paths.push_back(user_bazelrc_path);
+      }
+    }
   }
   // DedupeBlazercPaths returns paths whose canonical path could be computed,
   // therefore these paths must exist.
@@ -370,20 +381,19 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles(
 
   // Get the command-line provided rc, passed as --bazelrc or nothing if the
   // flag is absent.
-  const char* cmd_line_rc_file =
-      SearchUnaryOption(cmd_line->startup_args, "--bazelrc",
-                        /* warn_if_dupe */ true);
-  if (cmd_line_rc_file != nullptr) {
-    string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_file);
-    // Unlike the previous 3 paths, where we ignore it if the file does not
-    // exist or is unreadable, since this path is explicitly passed, this is an
-    // error. Check this condition here.
-    if (!blaze_util::CanReadFile(absolute_cmd_line_rc)) {
-      BAZEL_LOG(ERROR) << "Error: Unable to read .bazelrc file '"
-                       << absolute_cmd_line_rc << "'.";
-      return blaze_exit_code::BAD_ARGV;
-    }
-    rc_files.push_back(absolute_cmd_line_rc);
+  vector<std::string> cmd_line_rc_files =
+      GetAllUnaryOptionValues(cmd_line->startup_args, "--bazelrc", "/dev/null");
+  for (auto& rc_file : cmd_line_rc_files) {
+        string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(rc_file);
+        // Unlike the previous 3 paths, where we ignore it if the file does not
+        // exist or is unreadable, since this path is explicitly passed, this is an
+        // error. Check this condition here.
+        if (!blaze_util::CanReadFile(absolute_cmd_line_rc)) {
+          BAZEL_LOG(ERROR) << "Error: Unable to read .bazelrc file '"
+                           << absolute_cmd_line_rc << "'.";
+          return blaze_exit_code::BAD_ARGV;
+        }
+        rc_files.push_back(absolute_cmd_line_rc);
   }
 
   // Log which files we're looking for before removing duplicates and

src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java

@@ -33,10 +33,17 @@ public static final class Options extends OptionsBase {
         valueHelp = "<path>",
         help =
             "The location of the user .bazelrc file containing default values of "
-                + "Bazel options. If unspecified, Bazel uses the first .bazelrc file it finds in "
+                + "Bazel options. This option can also be specified multiple times.\n"
+                + "E.g. with `--bazelrc=x.rc --bazelrc=y.rc`, options in both RCs will be read.\n"
+                + "--bazelrc values that come after `/dev/null` will be ignored to maintain "
+                + "backward compatibility.\nE.g. with "
+                + "`--bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc`, "
+                + "only x.rc and y.rc will be ready, and z.rc will be ignored.\n"
+                + "If unspecified, Bazel uses the first .bazelrc file it finds in "
                 + "the following two locations: the workspace directory, then the user's home "
                 + "directory. Use /dev/null to disable the search for a user rc file, e.g. in "
-                + "release builds.")
+                + "release builds.\n"
+                + "Note: command line options will always supersede any option in bazelrc.")
     public String blazerc;
 
     // TODO(b/36168162): Remove this after the transition period is ower. This now only serves to

src/test/cpp/blaze_util_test.cc

@@ -234,4 +234,88 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) {
   }
 }
 
+void assert_equal_vector_char_pointer(std::vector<std::string> expected, std::vector<std::string> actual) {
+  ASSERT_EQ(actual.size(), expected.size()) << "Vectors expected and actual are of unequal length";
+
+  for (int i = 0; i < actual.size(); ++i) {
+    ASSERT_EQ(actual[i], expected[i]) << "Vectors expected and actual differ at index " << i;
+  }
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryForEmpty) {
+  assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, ""));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryFlagNotPresent) {
+  assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithEquals) {
+  assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag=value", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithEquals2) {
+  assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlag) {
+  assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnly) {
+  assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnlyMulti) {
+  assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "--flag", "value1"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnlyMultiWithEquals) {
+  assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag=--flag", "--flag", "value1"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithEquals3) {
+  assert_equal_vector_char_pointer({"value1", "value2", "value3"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "--flag=value3", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithoutEquals) {
+  assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag", "value", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithoutEquals2) {
+  assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals) {
+  assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals2) {
+  assert_equal_vector_char_pointer({"value1","value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals) {
+  assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals2) {
+  assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value1", "--flag=value2"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithEquals) {
+  assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithoutEquals) {
+  assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--", "--flag=value"}, "--flag"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfter) {
+  assert_equal_vector_char_pointer({"value1","/dev/null"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value1", "--flag", "/dev/null",  "--flag", "value3"}, "--flag", "/dev/null"));
+}
+
+TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfterDevNull) {
+  assert_equal_vector_char_pointer({"/dev/null"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "/dev/null", "--flag", "value2",  "--flag", "value3"}, "--flag", "/dev/null"));
+}
+
 }  // namespace blaze

src/test/shell/integration/startup_options_test.sh

@@ -82,4 +82,28 @@ function test_autodetect_server_javabase() {
   bazel --noautodetect_server_javabase version &> $TEST_log || fail "Should pass"
 }
 
+# Below are the regression tests for Issue #7489
+function test_multiple_bazelrc_later_overwrites_earlier() {
+  # Help message only visible with --help_verbosity=medium
+  help_message_in_description="--${PRODUCT_NAME}rc (a string; default: see description)"
+
+  echo "help --help_verbosity=short" > 1.rc
+  echo "help --help_verbosity=medium" > 2.rc
+  bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass"
+  expect_log "$help_message_in_description"
+
+  echo "help --help_verbosity=medium" > 1.rc
+  echo "help --help_verbosity=short" > 2.rc
+  bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass"
+  expect_not_log "$help_message_in_description"
+}
+
+function test_multiple_bazelrc_set_different_options() {
+  echo "common --verbose_failures" > 1.rc
+  echo "common --test_output=all" > 2.rc
+  bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" build --announce_rc &> $TEST_log || fail "Should pass"
+  expect_log "Inherited 'common' options: --verbose_failures"
+  expect_log "Inherited 'common' options: --test_output=all"
+}
+
 run_suite "${PRODUCT_NAME} startup options test"