Flags: warn for duplicate --bazelrc flag

If multiple --bazelrc flags are given, Bazel only
uses the first one.

This commit adds a warning to make that more
clear.

Rationale: #7489 (comment)

Fixes #7489

PiperOrigin-RevId: 262118102

src/main/cpp/blaze_util.cc

@@ -75,24 +75,66 @@ bool GetNullaryOption(const char *arg, const char *key) {
 }
 
 const char* SearchUnaryOption(const vector<string>& args,
-                              const char *key) {
+                              const char *key, bool warn_if_dupe) {
   if (args.empty()) {
     return NULL;
   }
 
+  const char* value = nullptr;
+  bool found_dupe = false;  // true if 'key' was found twice
   vector<string>::size_type i = 0;
+
+  // Examine the first N-1 arguments. (N-1 because we examine the i'th and
+  // i+1'th together, in case a flag is defined "--name value" style and not
+  // "--name=value" style.)
   for (; i < args.size() - 1; ++i) {
     if (args[i] == "--") {
-      return NULL;
+      // If the current argument is "--", all following args are target names.
+      // If 'key' was not found, 'value' is nullptr and we can return that.
+      // If 'key' was found exactly once, then 'value' has the value and again
+      // we can return that.
+      // If 'key' was found more than once then we could not have reached this
+      // line, because we would have broken out of the loop when 'key' was found
+      // the second time.
+      return value;
     }
     const char* result = GetUnaryOption(args[i].c_str(),
                                         args[i + 1].c_str(),
                                         key);
     if (result != NULL) {
-      return result;
+      // 'key' was found and 'result' has its value.
+      if (value) {
+        // 'key' was found once before, because 'value' is not empty.
+        found_dupe = true;
+        break;
+      } else {
+        // 'key' was not found before, so store the value in 'value'.
+        value = result;
+      }
+    }
+  }
+
+  if (value) {
+    // 'value' is not empty, so 'key' was found at least once in the first N-1
+    // arguments.
+    if (warn_if_dupe) {
+      if (!found_dupe) {
+        // We did not find a duplicate in the first N-1 arguments. Examine the
+        // last argument, it may be a duplicate.
+        found_dupe = (GetUnaryOption(args[i].c_str(), NULL, key) != nullptr);
+      }
+      if (found_dupe) {
+        BAZEL_LOG(WARNING) << key << " is given more than once, "
+                           << "only the first occurrence is used";
+      }
     }
+    return value;
+  } else {
+    // 'value' is empty, so 'key' was not yet found in the first N-1 arguments.
+    // If 'key' is in the last argument, we'll parse and return the value from
+    // that, and if it isn't, we'll return NULL.
+    return GetUnaryOption(args[i].c_str(), NULL, key);
   }
-  return GetUnaryOption(args[i].c_str(), NULL, key);
 }
 
 bool SearchNullaryOption(const vector<string>& args,

src/main/cpp/blaze_util.h

@@ -44,10 +44,12 @@ bool GetNullaryOption(const char *arg, const char *key);
 
 // Searches for 'key' in 'args' using GetUnaryOption. Arguments found after '--'
 // are omitted from the search.
+// When 'warn_if_dupe' is true, the method checks if 'key' is specified more
+// than once and prints a warning if so.
 // Returns the value of the 'key' flag iff it occurs in args.
 // Returns NULL otherwise.
 const char* SearchUnaryOption(const std::vector<std::string>& args,
-                              const char* key);
+                              const char* key, bool warn_if_dupe);
 
 // Searches for '--flag_name' and '--noflag_name' in 'args' using
 // GetNullaryOption. Arguments found after '--' are omitted from the search.

src/main/cpp/option_processor.cc

@@ -195,7 +195,8 @@ std::set<std::string> GetOldRcPaths(
     candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path};
   }
   string user_bazelrc_path = internal::FindLegacyUserBazelrc(
-      SearchUnaryOption(startup_args, "--bazelrc"), workspace);
+      SearchUnaryOption(startup_args, "--bazelrc", /* warn_if_dupe */ true),
+      workspace);
   if (!user_bazelrc_path.empty()) {
     candidate_bazelrc_paths.push_back(user_bazelrc_path);
   }
@@ -363,7 +364,8 @@ 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");
+      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

src/test/cpp/blaze_util_test.cc

@@ -23,11 +23,13 @@
 #include "src/main/cpp/blaze_util.h"
 #include "src/main/cpp/blaze_util_platform.h"
 #include "src/main/cpp/util/file.h"
+#include "googlemock/include/gmock/gmock.h"
 #include "googletest/include/gtest/gtest.h"
 
 namespace blaze {
 
 using std::string;
+using ::testing::HasSubstr;
 
 class BlazeUtilTest : public ::testing::Test {
  protected:
@@ -113,7 +115,7 @@ TEST_F(BlazeUtilTest, TestSearchNullaryEmptyCase) {
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryEmptyCase) {
-  ASSERT_STREQ(nullptr, SearchUnaryOption({}, "--flag"));
+  ASSERT_STREQ(nullptr, SearchUnaryOption({}, "--flag", false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchNullaryForEmpty) {
@@ -165,50 +167,71 @@ TEST_F(BlazeUtilTest, TestSearchNullaryLastFlagWins) {
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryForEmpty) {
-  ASSERT_STREQ(nullptr, SearchUnaryOption({"bazel", "build", ":target"}, ""));
+  ASSERT_STREQ(nullptr, SearchUnaryOption({"bazel", "build", ":target"}, "",
+                                          false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryFlagNotPresent) {
   ASSERT_STREQ(nullptr,
-               SearchUnaryOption({"bazel", "build", ":target"}, "--flag"));
+               SearchUnaryOption({"bazel", "build", ":target"}, "--flag",
+                                 false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryStartupOptionWithEquals) {
   ASSERT_STREQ("value",
                SearchUnaryOption({"bazel", "--flag=value", "build", ":target"},
-                                 "--flag"));
+                                 "--flag", false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryStartupOptionWithoutEquals) {
   ASSERT_STREQ("value",
                SearchUnaryOption(
-                   {"bazel", "--flag", "value", "build", ":target"}, "--flag"));
+                   {"bazel", "--flag", "value", "build", ":target"}, "--flag",
+                   false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWithEquals) {
   ASSERT_STREQ("value",
                SearchUnaryOption(
-                   {"bazel", "build", ":target", "--flag", "value"}, "--flag"));
+                   {"bazel", "build", ":target", "--flag", "value"}, "--flag",
+                   false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWithoutEquals) {
   ASSERT_STREQ("value",
                SearchUnaryOption(
-                   {"bazel", "build", ":target", "--flag=value"}, "--flag"));
+                   {"bazel", "build", ":target", "--flag=value"}, "--flag",
+                   false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnarySkipsAfterDashDashWithEquals) {
   ASSERT_STREQ(nullptr,
                SearchUnaryOption(
                    {"bazel", "build", ":target", "--", "--flag", "value"},
-                   "--flag"));
+                   "--flag", false));
 }
 
 TEST_F(BlazeUtilTest, TestSearchUnarySkipsAfterDashDashWithoutEquals) {
   ASSERT_STREQ(nullptr,
                SearchUnaryOption(
                    {"bazel", "build", ":target", "--", "--flag=value"},
-                   "--flag"));
+                   "--flag", false));
+}
+
+TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) {
+  testing::internal::CaptureStderr();
+  for (int i = 0; i < 2; ++i) {
+    bool warn_if_dupe = (i == 0);
+    ASSERT_STREQ("v1",
+                 SearchUnaryOption(
+                     {"foo", "--flag", "v1", "bar", "--flag=v2"}, "--flag",
+                     warn_if_dupe));
+
+    if (warn_if_dupe) {
+      std::string stderr_output = testing::internal::GetCapturedStderr();
+      EXPECT_THAT(stderr_output, HasSubstr("--flag is given more than once"));
+    }
+  }
 }
 
 }  // namespace blaze