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: https://github.com/bazelbuild/bazel/issues/7489#issuecomment-515952490
Fixes https://github.com/bazelbuild/bazel/issues/7489
PiperOrigin-RevId: 262118102
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index ce48149..ca25bba 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -75,24 +75,66 @@
}
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;
+ }
}
}
- return GetUnaryOption(args[i].c_str(), NULL, key);
+
+ 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);
+ }
}
bool SearchNullaryOption(const vector<string>& args,
diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h
index b0b5538..123ead9 100644
--- a/src/main/cpp/blaze_util.h
+++ b/src/main/cpp/blaze_util.h
@@ -44,10 +44,12 @@
// 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.
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 41c6db8..8f8f156 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -195,7 +195,8 @@
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 @@
// 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
diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc
index 03b43c0..dda7bb51 100644
--- a/src/test/cpp/blaze_util_test.cc
+++ b/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, TestSearchUnaryEmptyCase) {
- ASSERT_STREQ(nullptr, SearchUnaryOption({}, "--flag"));
+ ASSERT_STREQ(nullptr, SearchUnaryOption({}, "--flag", false));
}
TEST_F(BlazeUtilTest, TestSearchNullaryForEmpty) {
@@ -165,50 +167,71 @@
}
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
