From a89e22a5b7330064c2bbd3d862dd1100a99d0f21 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 11:46:48 -0800 Subject: [PATCH 01/40] nary --- src/main/cpp/blaze_util.cc | 43 ++++++++++++++++++++++++++++++++ src/main/cpp/blaze_util.h | 3 +++ src/main/cpp/option_processor.cc | 31 +++++++++++++---------- 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index d30c0d39647d8e..9736bced3cee86 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -74,6 +74,49 @@ bool GetNullaryOption(const char *arg, const char *key) { return true; } +std::vector SearchNaryOption(const vector& args, + const char *key, bool warn_if_dupe) { + vector values; + if (args.empty()) { + return values; + } + + const char* value = nullptr; + bool found_dupe = false; // true if 'key' was found twice + vector::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(); ++i) { + if (args[i] == "--") { + // 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 values; + } +// for (int j=0; j < args.size(); j ++) { +// BAZEL_LOG(WARNING) << "args " << args[j]; +// } + + const char* result = GetUnaryOption(args[i].c_str(), + args[i + 1].c_str(), + key); + std::cout<<"result: "<< *result<& args, const char *key, bool warn_if_dupe) { if (args.empty()) { diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index ed358d1c75028c..90fdd0e26ae4ae 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -53,6 +53,9 @@ bool GetNullaryOption(const char *arg, const char *key); const char* SearchUnaryOption(const std::vector& args, const char* key, bool warn_if_dupe); +std::vector SearchNaryOption(const std::vector& args, + 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. // Returns true if '--flag_name' is a flag in args and '--noflag_name' does not diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index dc3455f8815492..d167d94929c282 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include "src/main/cpp/blaze_util.h" #include "src/main/cpp/blaze_util_platform.h" @@ -41,6 +42,8 @@ extern char **environ; namespace blaze { +using std::cout; +using std::endl; using std::map; using std::set; using std::string; @@ -370,20 +373,22 @@ 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", + vector cmd_line_rc_files = SearchNaryOption(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); + for (int j=0 ; j < cmd_line_rc_files.size(); j++) { + std::cout << "rc " << *cmd_line_rc_files[j] << std::endl; + } + for (int i=0; i< cmd_line_rc_files.size(); i++) { + string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_files[i]); + // 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 From bda828c921b225489c061f21b08550e1364169aa Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 12:03:21 -0800 Subject: [PATCH 02/40] more --- src/main/cpp/option_processor.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index d167d94929c282..939c0f13017caf 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -204,11 +204,14 @@ std::set 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 cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc", /* warn_if_dupe */ true); + for (int i=0; i Date: Mon, 21 Dec 2020 12:04:57 -0800 Subject: [PATCH 03/40] no print --- src/main/cpp/blaze_util.cc | 6 ------ src/main/cpp/option_processor.cc | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 9736bced3cee86..e6e0fe5508a393 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -99,15 +99,9 @@ std::vector SearchNaryOption(const vector& args, // the second time. return values; } -// for (int j=0; j < args.size(); j ++) { -// BAZEL_LOG(WARNING) << "args " << args[j]; -// } - const char* result = GetUnaryOption(args[i].c_str(), args[i + 1].c_str(), key); - std::cout<<"result: "<< *result< cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc", /* warn_if_dupe */ true); - for (int j=0 ; j < cmd_line_rc_files.size(); j++) { - std::cout << "rc " << *cmd_line_rc_files[j] << std::endl; - } for (int i=0; i< cmd_line_rc_files.size(); i++) { string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_files[i]); // Unlike the previous 3 paths, where we ignore it if the file does not From 50d2b04b4f1ce751f685bfc634fec4b247147c58 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 12:06:21 -0800 Subject: [PATCH 04/40] no dedup --- src/main/cpp/blaze_util.cc | 2 +- src/main/cpp/blaze_util.h | 2 +- src/main/cpp/option_processor.cc | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index e6e0fe5508a393..a132000b8a2087 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -75,7 +75,7 @@ bool GetNullaryOption(const char *arg, const char *key) { } std::vector SearchNaryOption(const vector& args, - const char *key, bool warn_if_dupe) { + const char *key) { vector values; if (args.empty()) { return values; diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 90fdd0e26ae4ae..16c12c88e46f8f 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -54,7 +54,7 @@ const char* SearchUnaryOption(const std::vector& args, const char* key, bool warn_if_dupe); std::vector SearchNaryOption(const std::vector& args, - const char* key, bool warn_if_dupe); + const char* key); // 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 bb6a42968af5fb..b408336e3869e4 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -204,7 +204,7 @@ std::set GetOldRcPaths( internal::FindRcAlongsideBinary(cwd, path_to_binary); candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } - vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc", /* warn_if_dupe */ true); + vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); for (int i=0; i cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc", - /* warn_if_dupe */ true); + vector cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); for (int i=0; i< cmd_line_rc_files.size(); i++) { string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_files[i]); // Unlike the previous 3 paths, where we ignore it if the file does not From 13ae53852a07b5f8acb46ae6315c7345b7874f89 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 12:15:24 -0800 Subject: [PATCH 05/40] minor --- src/main/cpp/blaze_util.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index a132000b8a2087..80435df1812d42 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -81,13 +81,9 @@ std::vector SearchNaryOption(const vector& args, return values; } - const char* value = nullptr; bool found_dupe = false; // true if 'key' was found twice vector::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(); ++i) { if (args[i] == "--") { // If the current argument is "--", all following args are target names. From 421de4f14170db703571a4066f184806f8aa0896 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 15:38:48 -0800 Subject: [PATCH 06/40] cmt --- src/main/cpp/blaze_util.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 16c12c88e46f8f..52f15ae524fc38 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -53,6 +53,10 @@ bool GetNullaryOption(const char *arg, const char *key); const char* SearchUnaryOption(const std::vector& args, const char* key, bool warn_if_dupe); +// Searches for 'key' in 'args' using GetUnaryOption. Arguments found after '--' +// are omitted from the search. +// Returns the values of the 'key' flag iff it occurs in args. +// Returns empty vector otherwise. std::vector SearchNaryOption(const std::vector& args, const char* key); From 2f08fa8d61404c9452f6f8c06162269a02a45aae Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 16:43:51 -0800 Subject: [PATCH 07/40] initial tests --- src/main/cpp/blaze_util.cc | 3 ++- src/test/cpp/blaze_util_test.cc | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 80435df1812d42..9252837c5b740e 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -81,9 +81,9 @@ std::vector SearchNaryOption(const vector& args, return values; } - bool found_dupe = false; // true if 'key' was found twice vector::size_type i = 0; + for (; i < args.size(); ++i) { if (args[i] == "--") { // If the current argument is "--", all following args are target names. @@ -99,6 +99,7 @@ std::vector SearchNaryOption(const vector& args, args[i + 1].c_str(), key); if (result != NULL) { + std::cout << result << std::endl; // 'key' was found and 'result' has its value. values.push_back(result); } diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index dda7bb51388760..6c4efb3206eda1 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -30,6 +30,7 @@ namespace blaze { using std::string; using ::testing::HasSubstr; +using ::testing::ElementsAre; class BlazeUtilTest : public ::testing::Test { protected: @@ -234,4 +235,44 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) { } } +void assert_equal_vector_char_pointer(std::vector expected, std::vector actual) { + ASSERT_EQ(actual.size(), expected.size()) << "Vectors x and y are of unequal length"; + + for (int i = 0; i < actual.size(); ++i) { + ASSERT_STREQ(actual[i], expected[i]) << "Vectors x and y differ at index " << i; + } +} + +TEST_F(BlazeUtilTest, TestSearchNaryForEmpty) { + assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target"}, "")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryFlagNotPresent) { + assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals) { + assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag=value", "build", ":target"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals) { + assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag", "value", "build", ":target"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals) { + assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag", "value"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals) { + assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag=value"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithEquals) { + assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithoutEquals) { + assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target", "--", "--flag=value"}, "--flag")); +} + } // namespace blaze From b52aa5575503504cdc8b9320e2a21120844b0657 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 16:49:42 -0800 Subject: [PATCH 08/40] more --- src/test/cpp/blaze_util_test.cc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index 6c4efb3206eda1..c26853cb9af1c9 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -255,18 +255,38 @@ TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals) { assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag=value", "build", ":target"}, "--flag")); } +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals2) { + assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals3) { + assert_equal_vector_char_pointer({"value1", "value2", "value3"}, SearchNaryOption({"bazel", "--flag=value1", "--flag=value2", "--flag=value3", "build", ":target"}, "--flag")); +} + TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals) { assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag", "value", "build", ":target"}, "--flag")); } +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals2) { + assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"}, "--flag")); +} + TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals) { assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag", "value"}, "--flag")); } +TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals2) { + assert_equal_vector_char_pointer({"value1","value2"}, SearchNaryOption({"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"}, "--flag")); +} + TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals) { assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag=value"}, "--flag")); } +TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals2) { + assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "build", ":target", "--flag=value1", "--flag=value2"}, "--flag")); +} + TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithEquals) { assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag")); } From 6b8b39717f2d176e54f3230e530f945167975790 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 16:54:09 -0800 Subject: [PATCH 09/40] rm unused --- src/main/cpp/blaze_util.cc | 1 - src/main/cpp/option_processor.cc | 3 --- src/test/cpp/blaze_util_test.cc | 1 - 3 files changed, 5 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 9252837c5b740e..1766bee64a8d88 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -99,7 +99,6 @@ std::vector SearchNaryOption(const vector& args, args[i + 1].c_str(), key); if (result != NULL) { - std::cout << result << std::endl; // 'key' was found and 'result' has its value. values.push_back(result); } diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index b408336e3869e4..ec281455c90e47 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -24,7 +24,6 @@ #include #include #include -#include #include "src/main/cpp/blaze_util.h" #include "src/main/cpp/blaze_util_platform.h" @@ -42,8 +41,6 @@ extern char **environ; namespace blaze { -using std::cout; -using std::endl; using std::map; using std::set; using std::string; diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index c26853cb9af1c9..e041a0b08a4606 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -30,7 +30,6 @@ namespace blaze { using std::string; using ::testing::HasSubstr; -using ::testing::ElementsAre; class BlazeUtilTest : public ::testing::Test { protected: From d38eca0ea2fac3004f1444c75ba9e412f2a1e7f1 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 21 Dec 2020 17:04:05 -0800 Subject: [PATCH 10/40] rename --- src/test/cpp/blaze_util_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index e041a0b08a4606..e72f7aaa1b1639 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -235,10 +235,10 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) { } void assert_equal_vector_char_pointer(std::vector expected, std::vector actual) { - ASSERT_EQ(actual.size(), expected.size()) << "Vectors x and y are of unequal length"; + ASSERT_EQ(actual.size(), expected.size()) << "Vectors expected and actual are of unequal length"; for (int i = 0; i < actual.size(); ++i) { - ASSERT_STREQ(actual[i], expected[i]) << "Vectors x and y differ at index " << i; + ASSERT_STREQ(actual[i], expected[i]) << "Vectors expected and actual differ at index " << i; } } From fd7540b2a582999542b3ac6ee044f5e1e20e7504 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 10:26:58 -0800 Subject: [PATCH 11/40] size_type --- src/main/cpp/option_processor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index ec281455c90e47..96af04d9fe8f39 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -202,7 +202,7 @@ std::set GetOldRcPaths( candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); - for (int i=0; i::size_type i=0; i Date: Tue, 22 Dec 2020 10:29:59 -0800 Subject: [PATCH 12/40] fmt --- src/main/cpp/blaze_util.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 1766bee64a8d88..f76558df641eb6 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -81,10 +81,7 @@ std::vector SearchNaryOption(const vector& args, return values; } - vector::size_type i = 0; - - - for (; i < args.size(); ++i) { + for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { // If the current argument is "--", all following args are target names. // If 'key' was not found, 'value' is nullptr and we can return that. From e1b2a0715436d8aa84c77816bbbca2b6e09de63a Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 10:39:38 -0800 Subject: [PATCH 13/40] one more size_t --- src/main/cpp/option_processor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index 96af04d9fe8f39..e2729e7022a25b 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -374,7 +374,7 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles( // Get the command-line provided rc, passed as --bazelrc or nothing if the // flag is absent. vector cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); - for (int i=0; i< cmd_line_rc_files.size(); i++) { + for (vector::size_type i=0; i< cmd_line_rc_files.size(); i++) { string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_files[i]); // 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 From 3aa941e99f332d89533a3c88b1574904b3e88537 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 11:13:15 -0800 Subject: [PATCH 14/40] auto --- src/main/cpp/option_processor.cc | 10 ++++------ .../build/lib/bazel/BazelStartupOptionsModule.java | 4 +++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index e2729e7022a25b..8e271cd53300ac 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -202,10 +202,8 @@ std::set GetOldRcPaths( candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); - for (vector::size_type i=0; i cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); - for (vector::size_type i=0; i< cmd_line_rc_files.size(); i++) { - string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_files[i]); + 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. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 0ea737acf87ae0..546e89bc79633c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -33,7 +33,9 @@ public static final class Options extends OptionsBase { valueHelp = "", 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 chained together. " + + "E.g. --bazelrc=x.rc --bazelrc=y.rc so options in both RCs will be read. " + + "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.") From d72e7bc57d334cfe9d136a66da745f5af4171d55 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 13:18:40 -0800 Subject: [PATCH 15/40] potential index out of bound --- src/main/cpp/blaze_util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index f76558df641eb6..030f61f08ce1f9 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -93,7 +93,7 @@ std::vector SearchNaryOption(const vector& args, return values; } const char* result = GetUnaryOption(args[i].c_str(), - args[i + 1].c_str(), + args[std::min(i + 1, args.size() -1)].c_str(), key); if (result != NULL) { // 'key' was found and 'result' has its value. From 2888227df0aff3a4f33b324603584e077273b72e Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 13:19:30 -0800 Subject: [PATCH 16/40] nullptr --- src/main/cpp/blaze_util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 030f61f08ce1f9..07f126153a0890 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -95,7 +95,7 @@ std::vector SearchNaryOption(const vector& args, const char* result = GetUnaryOption(args[i].c_str(), args[std::min(i + 1, args.size() -1)].c_str(), key); - if (result != NULL) { + if (result != nullptr) { // 'key' was found and 'result' has its value. values.push_back(result); } From dda34c2117c369d11cbb71c67e5c5768e0904ec2 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 13:41:00 -0800 Subject: [PATCH 17/40] use vector --- src/main/cpp/blaze_util.cc | 8 ++------ src/main/cpp/blaze_util.h | 2 +- src/main/cpp/option_processor.cc | 18 ++++++++++++++---- src/test/cpp/blaze_util_test.cc | 4 ++-- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 07f126153a0890..ba112b475e11ef 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -74,13 +74,9 @@ bool GetNullaryOption(const char *arg, const char *key) { return true; } -std::vector SearchNaryOption(const vector& args, +std::vector SearchNaryOption(const vector& args, const char *key) { - vector values; - if (args.empty()) { - return values; - } - + vector values; for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { // If the current argument is "--", all following args are target names. diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 52f15ae524fc38..bda4a3893e3718 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -57,7 +57,7 @@ const char* SearchUnaryOption(const std::vector& args, // are omitted from the search. // Returns the values of the 'key' flag iff it occurs in args. // Returns empty vector otherwise. -std::vector SearchNaryOption(const std::vector& args, +std::vector SearchNaryOption(const std::vector& args, const char* key); // Searches for '--flag_name' and '--noflag_name' in 'args' using diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index 8e271cd53300ac..0ff5f2156f802c 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -201,13 +201,23 @@ std::set GetOldRcPaths( internal::FindRcAlongsideBinary(cwd, path_to_binary); candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } - vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); - for (auto& rc_file : cmd_line_rc_files) { - string user_bazelrc_path = internal::FindLegacyUserBazelrc(rc_file, workspace); + vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); + // 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. const std::vector deduped_existing_blazerc_paths = @@ -371,7 +381,7 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles( // Get the command-line provided rc, passed as --bazelrc or nothing if the // flag is absent. - vector cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); + vector cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); 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 diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index e72f7aaa1b1639..59928c6ef4c0d8 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -234,11 +234,11 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) { } } -void assert_equal_vector_char_pointer(std::vector expected, std::vector actual) { +void assert_equal_vector_char_pointer(std::vector expected, std::vector actual) { ASSERT_EQ(actual.size(), expected.size()) << "Vectors expected and actual are of unequal length"; for (int i = 0; i < actual.size(); ++i) { - ASSERT_STREQ(actual[i], expected[i]) << "Vectors expected and actual differ at index " << i; + ASSERT_EQ(actual[i], expected[i]) << "Vectors expected and actual differ at index " << i; } } From b39c1246a605ecbf168dbf29e43826caacb020a2 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 22 Dec 2020 13:45:29 -0800 Subject: [PATCH 18/40] algo --- src/main/cpp/blaze_util.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index ba112b475e11ef..0082ee543243b8 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -14,6 +14,7 @@ #include "src/main/cpp/blaze_util.h" +#include #include #include #include @@ -35,6 +36,7 @@ namespace blaze { using std::map; +using std::min; using std::string; using std::vector; From 71c781adaae79a1bdfe46ad2e8f3a768e97624a7 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 18 Jan 2021 13:59:30 -0800 Subject: [PATCH 19/40] address comments --- src/main/cpp/blaze_util.cc | 2 +- src/main/cpp/option_processor.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 0082ee543243b8..3272a1a476f0eb 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -82,7 +82,7 @@ std::vector SearchNaryOption(const vector& args, for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { // 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 not found, an empty vector is returned. // 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 diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index 0ff5f2156f802c..a3244c4daeb71b 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -202,8 +202,8 @@ std::set GetOldRcPaths( candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); - // Divide the cases where the vector is empty vs not, as `FindLegacyUserBazelrc` has a case - // for rc_file to be a nullptr. + // 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()) { From f690f2cf93dd053ef8576430f80aede851a198f0 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 18 Jan 2021 14:02:47 -0800 Subject: [PATCH 20/40] simplify --- src/main/cpp/blaze_util.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 3272a1a476f0eb..db1438b8f8eb11 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -82,12 +82,6 @@ std::vector SearchNaryOption(const vector& args, for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { // If the current argument is "--", all following args are target names. - // If 'key' was not found, an empty vector is returned. - // 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 values; } const char* result = GetUnaryOption(args[i].c_str(), From 420d88a147a24770bbd0a230b33e22ac55b0dee5 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 18 Jan 2021 18:20:46 -0800 Subject: [PATCH 21/40] integration tests for multirc --- .../shell/integration/startup_options_test.sh | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 63f1624866d41d..103e1d339abfd6 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -82,4 +82,30 @@ 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() { + tmpdir=$(cd $(mktemp -d) &> /dev/null && pwd -P) + echo "common --repository_cache=$tmpdir" > 1.rc + bazel --bazelrc=1.rc info &> $TEST_log || fail "Should success" + expect_log "repository_cache: $tmpdir" +} + +function test_multiple_bazelrc_later_overwrites_earlier() { + tmpdir1=$(cd $(mktemp -d) &> /dev/null && pwd -P) + tmpdir2=$(cd $(mktemp -d) &> /dev/null && pwd -P) + echo "common --repository_cache=$tmpdir1" > 1.rc + echo "common --repository_cache=$tmpdir2" > 2.rc + bazel --bazelrc=1.rc --bazelrc=2.rc info &> $TEST_log || fail "Should pass" + expect_log "repository_cache: $tmpdir2" +} + +function test_multiple_bazelrc_set_different_options() { + tmpdir=$(cd $(mktemp -d) &> /dev/null && pwd -P) + echo "common --repository_cache=$tmpdir" > 1.rc + echo "common --test_output=all" > 2.rc + bazel --bazelrc=1.rc --bazelrc=2.rc build --announce_rc &> $TEST_log || fail "Should pass" + expect_log "Inherited 'common' options: --repository_cache=$tmpdir" + expect_log "Inherited 'common' options: --test_output=all" +} + run_suite "${PRODUCT_NAME} startup options test" From eb1894b2451bbd994259df0400d0196da8718389 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 18 Jan 2021 18:25:51 -0800 Subject: [PATCH 22/40] rm --- src/test/shell/integration/startup_options_test.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 103e1d339abfd6..446964b0d421db 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -83,13 +83,6 @@ function test_autodetect_server_javabase() { } # Below are the regression tests for Issue #7489 -function test_multiple_bazelrc() { - tmpdir=$(cd $(mktemp -d) &> /dev/null && pwd -P) - echo "common --repository_cache=$tmpdir" > 1.rc - bazel --bazelrc=1.rc info &> $TEST_log || fail "Should success" - expect_log "repository_cache: $tmpdir" -} - function test_multiple_bazelrc_later_overwrites_earlier() { tmpdir1=$(cd $(mktemp -d) &> /dev/null && pwd -P) tmpdir2=$(cd $(mktemp -d) &> /dev/null && pwd -P) From dd78bd317f6956b4f5e8f444b1d13e7d686ff953 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 19 Jan 2021 14:54:24 -0800 Subject: [PATCH 23/40] improve --bazelrc help message on multivalue --- .../devtools/build/lib/bazel/BazelStartupOptionsModule.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 546e89bc79633c..5a119ec6cca9d1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -33,8 +33,10 @@ public static final class Options extends OptionsBase { valueHelp = "", help = "The location of the user .bazelrc file containing default values of " - + "Bazel options. This option can also be chained together. " - + "E.g. --bazelrc=x.rc --bazelrc=y.rc so options in both RCs will be read. " + + "Bazel options. This option can also be chained together.\n" + + "E.g. `--bazelrc=x.rc --bazelrc=y.rc` so options in both RCs will be read.\n" + + "Note: `--bazelrc x.rc y.rc` is illegal, and each bazelrc file needs to be " + + "accompanied by --bazelrc flag before it.\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 " From 01d15ee8e0f8294c97b2dc82c6de69d7e929894e Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 19 Jan 2021 15:44:19 -0800 Subject: [PATCH 24/40] address comment --- src/main/cpp/blaze_util.cc | 2 +- src/main/cpp/option_processor.cc | 3 +-- .../devtools/build/lib/bazel/BazelStartupOptionsModule.java | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index db1438b8f8eb11..dd27c838ec7e3b 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -81,7 +81,7 @@ std::vector SearchNaryOption(const vector& args, vector values; for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { - // If the current argument is "--", all following args are target names. + // "--" means all remaining args aren't options return values; } const char* result = GetUnaryOption(args[i].c_str(), diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index a3244c4daeb71b..dc5072a012840c 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -209,8 +209,7 @@ std::set GetOldRcPaths( if (!user_bazelrc_path.empty()) { candidate_bazelrc_paths.push_back(user_bazelrc_path); } - } - else { + } 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()) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 5a119ec6cca9d1..6372797b03f3bb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -33,7 +33,7 @@ public static final class Options extends OptionsBase { valueHelp = "", help = "The location of the user .bazelrc file containing default values of " - + "Bazel options. This option can also be chained together.\n" + + "Bazel options. This option can also be specified multiple times.\n" + "E.g. `--bazelrc=x.rc --bazelrc=y.rc` so options in both RCs will be read.\n" + "Note: `--bazelrc x.rc y.rc` is illegal, and each bazelrc file needs to be " + "accompanied by --bazelrc flag before it.\n" From d73606d397f9dc56bdff2c3bff65fb7f10a791ba Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 19 Jan 2021 16:06:54 -0800 Subject: [PATCH 25/40] rename --- src/main/cpp/blaze_util.cc | 4 ++-- src/main/cpp/blaze_util.h | 4 ++-- src/main/cpp/option_processor.cc | 4 ++-- src/test/cpp/blaze_util_test.cc | 26 +++++++++++++------------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index dd27c838ec7e3b..d1f144a92775cc 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -76,8 +76,8 @@ bool GetNullaryOption(const char *arg, const char *key) { return true; } -std::vector SearchNaryOption(const vector& args, - const char *key) { +std::vector GetAllUnaryOptionValues(const vector& args, + const char *key) { vector values; for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index bda4a3893e3718..1d4e12d21ed094 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -57,8 +57,8 @@ const char* SearchUnaryOption(const std::vector& args, // are omitted from the search. // Returns the values of the 'key' flag iff it occurs in args. // Returns empty vector otherwise. -std::vector SearchNaryOption(const std::vector& args, - const char* key); +std::vector GetAllUnaryOptionValues(const std::vector& args, + const char* key); // 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 dc5072a012840c..c27c97d7414e7d 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -201,7 +201,7 @@ std::set GetOldRcPaths( internal::FindRcAlongsideBinary(cwd, path_to_binary); candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } - vector cmd_line_rc_files = SearchNaryOption(startup_args, "--bazelrc"); + vector cmd_line_rc_files = GetAllUnaryOptionValues(startup_args, "--bazelrc"); // 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()){ @@ -380,7 +380,7 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles( // Get the command-line provided rc, passed as --bazelrc or nothing if the // flag is absent. - vector cmd_line_rc_files = SearchNaryOption(cmd_line->startup_args, "--bazelrc"); + vector cmd_line_rc_files = GetAllUnaryOptionValues(cmd_line->startup_args, "--bazelrc"); 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 diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index 59928c6ef4c0d8..63a7e7b244c809 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -243,55 +243,55 @@ void assert_equal_vector_char_pointer(std::vector expected, std::ve } TEST_F(BlazeUtilTest, TestSearchNaryForEmpty) { - assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target"}, "")); + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "")); } TEST_F(BlazeUtilTest, TestSearchNaryFlagNotPresent) { - assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals) { - assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag=value", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag=value", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals2) { - assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals3) { - assert_equal_vector_char_pointer({"value1", "value2", "value3"}, SearchNaryOption({"bazel", "--flag=value1", "--flag=value2", "--flag=value3", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({"value1", "value2", "value3"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "--flag=value3", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals) { - assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "--flag", "value", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag", "value", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals2) { - assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"}, "--flag")); + assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals) { - assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag", "value"}, "--flag")); + assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals2) { - assert_equal_vector_char_pointer({"value1","value2"}, SearchNaryOption({"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"}, "--flag")); + assert_equal_vector_char_pointer({"value1","value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals) { - assert_equal_vector_char_pointer({"value"}, SearchNaryOption({"bazel", "build", ":target", "--flag=value"}, "--flag")); + assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals2) { - assert_equal_vector_char_pointer({"value1", "value2"}, SearchNaryOption({"bazel", "build", ":target", "--flag=value1", "--flag=value2"}, "--flag")); + assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value1", "--flag=value2"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithEquals) { - assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag")); + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag")); } TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithoutEquals) { - assert_equal_vector_char_pointer({}, SearchNaryOption({"bazel", "build", ":target", "--", "--flag=value"}, "--flag")); + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--", "--flag=value"}, "--flag")); } } // namespace blaze From 5fa3ff44d0ffe183419ad83a69fa5d2c23b80088 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Wed, 20 Jan 2021 10:05:00 -0800 Subject: [PATCH 26/40] address comments --- src/main/cpp/blaze_util.cc | 10 +++++++++- src/test/cpp/blaze_util_test.cc | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index d1f144a92775cc..25b9810304e207 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -84,13 +84,21 @@ std::vector GetAllUnaryOptionValues(const vector& args, // "--" 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(), - args[std::min(i + 1, args.size() -1)].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=" string, in which case + // we need to advance the index to skip the next arg for later iterations. + if (result == next_arg) { + i++; + } } return values; diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index 63a7e7b244c809..e69f56c499ad2b 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -258,6 +258,22 @@ TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals2) { assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag")); } +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlag) { + assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1", "build", ":target"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnly) { + assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnlyMulti) { + assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "--flag", "value1"}, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnlyMultiWithEquals) { + assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag=--flag", "--flag", "value1"}, "--flag")); +} + TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals3) { assert_equal_vector_char_pointer({"value1", "value2", "value3"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "--flag=value3", "build", ":target"}, "--flag")); } From 509924268c1473d129f3bde1d31b9bbfffc4c8c4 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 25 Jan 2021 08:46:23 -0800 Subject: [PATCH 27/40] address comment --- .../devtools/build/lib/bazel/BazelStartupOptionsModule.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 6372797b03f3bb..92d5ede802c02b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -35,8 +35,6 @@ public static final class Options extends OptionsBase { "The location of the user .bazelrc file containing default values of " + "Bazel options. This option can also be specified multiple times.\n" + "E.g. `--bazelrc=x.rc --bazelrc=y.rc` so options in both RCs will be read.\n" - + "Note: `--bazelrc x.rc y.rc` is illegal, and each bazelrc file needs to be " - + "accompanied by --bazelrc flag before it.\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 " From 1d4ad3cdd5cfd82f195450e9e4b439a991df4ee1 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Tue, 26 Jan 2021 15:21:24 -0800 Subject: [PATCH 28/40] Use different options and PRODUCT_NAME --- .../shell/integration/startup_options_test.sh | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 446964b0d421db..0b3c2aa9a5bb90 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -84,20 +84,27 @@ function test_autodetect_server_javabase() { # Below are the regression tests for Issue #7489 function test_multiple_bazelrc_later_overwrites_earlier() { - tmpdir1=$(cd $(mktemp -d) &> /dev/null && pwd -P) - tmpdir2=$(cd $(mktemp -d) &> /dev/null && pwd -P) - echo "common --repository_cache=$tmpdir1" > 1.rc - echo "common --repository_cache=$tmpdir2" > 2.rc - bazel --bazelrc=1.rc --bazelrc=2.rc info &> $TEST_log || fail "Should pass" - expect_log "repository_cache: $tmpdir2" + plain_expected_output="INFO: Build completed successfully, 1 total action" + + echo "common --color=yes" > 1.rc + echo "common --color=no" > 2.rc + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" build &> $TEST_log || fail "Should pass" + expect_log "$plain_expected_output" + + echo "common --color=no" > 3.rc + echo "common --color=yes" > 4.rc + bazel "--${PRODUCT_NAME}rc=3.rc" "--${PRODUCT_NAME}rc=4.rc" build &> $TEST_log || fail "Should pass" + # The final value for color should be yes, therefore the colored output would not contain + # the plain string, but rather something like + # "^[[32mINFO:^[[0m Build completed successfully, 1 total action" + expect_not_log "$plain_expected_output" } function test_multiple_bazelrc_set_different_options() { - tmpdir=$(cd $(mktemp -d) &> /dev/null && pwd -P) - echo "common --repository_cache=$tmpdir" > 1.rc + echo "common --verbose_failures" > 1.rc echo "common --test_output=all" > 2.rc - bazel --bazelrc=1.rc --bazelrc=2.rc build --announce_rc &> $TEST_log || fail "Should pass" - expect_log "Inherited 'common' options: --repository_cache=$tmpdir" + 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" } From 9307713dedc10b030139be968d1a7f5f54cc18f7 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Sun, 31 Jan 2021 17:47:06 -0800 Subject: [PATCH 29/40] add doc string --- .../devtools/build/lib/bazel/BazelStartupOptionsModule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 92d5ede802c02b..03ddfbad8d532b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -38,7 +38,8 @@ public static final class Options extends OptionsBase { + "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 From 35a899e260db2e366d5d41fcc155401080b5ee80 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Thu, 4 Feb 2021 15:27:21 -0800 Subject: [PATCH 30/40] use info --- src/test/shell/integration/startup_options_test.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 0b3c2aa9a5bb90..3d84afe1413ecc 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -84,19 +84,19 @@ function test_autodetect_server_javabase() { # Below are the regression tests for Issue #7489 function test_multiple_bazelrc_later_overwrites_earlier() { - plain_expected_output="INFO: Build completed successfully, 1 total action" + plain_expected_output="INFO: Invocation ID" echo "common --color=yes" > 1.rc echo "common --color=no" > 2.rc - bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" build &> $TEST_log || fail "Should pass" + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" info &> $TEST_log || fail "Should pass" expect_log "$plain_expected_output" echo "common --color=no" > 3.rc echo "common --color=yes" > 4.rc - bazel "--${PRODUCT_NAME}rc=3.rc" "--${PRODUCT_NAME}rc=4.rc" build &> $TEST_log || fail "Should pass" + bazel "--${PRODUCT_NAME}rc=3.rc" "--${PRODUCT_NAME}rc=4.rc" info &> $TEST_log || fail "Should pass" # The final value for color should be yes, therefore the colored output would not contain # the plain string, but rather something like - # "^[[32mINFO:^[[0m Build completed successfully, 1 total action" + # "^[[32mINFO: ^[[0mInvocation ID: d636c789-7350-4283-92d5-240d44a4e291" expect_not_log "$plain_expected_output" } From ed75ed9ea4d10bb7b6c49e61573f86bc6c85da01 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Fri, 5 Feb 2021 17:06:37 -0800 Subject: [PATCH 31/40] fix tests --- .../shell/integration/startup_options_test.sh | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 3d84afe1413ecc..ddb14768e224fd 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -84,20 +84,18 @@ function test_autodetect_server_javabase() { # Below are the regression tests for Issue #7489 function test_multiple_bazelrc_later_overwrites_earlier() { - plain_expected_output="INFO: Invocation ID" + # Help message only visible with --help_verbosity=medium + help_message_in_description="--${PRODUCT_NAME}rc (a string; default: see description)" - echo "common --color=yes" > 1.rc - echo "common --color=no" > 2.rc - bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" info &> $TEST_log || fail "Should pass" - expect_log "$plain_expected_output" + 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 "common --color=no" > 3.rc - echo "common --color=yes" > 4.rc - bazel "--${PRODUCT_NAME}rc=3.rc" "--${PRODUCT_NAME}rc=4.rc" info &> $TEST_log || fail "Should pass" - # The final value for color should be yes, therefore the colored output would not contain - # the plain string, but rather something like - # "^[[32mINFO: ^[[0mInvocation ID: d636c789-7350-4283-92d5-240d44a4e291" - expect_not_log "$plain_expected_output" + 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() { From d7dc7e5b6936ae1d76d8a275ba24023f47d47609 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 1 Mar 2021 12:55:24 -0800 Subject: [PATCH 32/40] Ignore values after /dev/null (restart ci) --- src/main/cpp/blaze_util.cc | 19 ++++++++- src/main/cpp/blaze_util.h | 5 ++- src/main/cpp/option_processor.cc | 6 ++- .../lib/bazel/BazelStartupOptionsModule.java | 6 ++- src/test/cpp/blaze_util_test.cc | 42 +++++++++++-------- 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 25b9810304e207..4ed80902655b19 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -77,7 +77,8 @@ bool GetNullaryOption(const char *arg, const char *key) { } std::vector GetAllUnaryOptionValues(const vector& args, - const char *key) { + const char *key, + const char *ignore_after_value) { vector values; for (vector::size_type i = 0; i < args.size(); ++i) { if (args[i] == "--") { @@ -101,7 +102,21 @@ std::vector GetAllUnaryOptionValues(const vector& args, } } - return values; + if (ignore_after_value == nullptr) { + return values; + } + else { + vector new_values; + std::string ignore_after_value_str = std::string(ignore_after_value); + for (vector::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& args, diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 1d4e12d21ed094..8b28242ee9e6e3 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -55,10 +55,13 @@ const char* SearchUnaryOption(const std::vector& args, // 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 GetAllUnaryOptionValues(const std::vector& args, - const char* key); + 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. diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index c27c97d7414e7d..40cf990cbd4245 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -201,7 +201,8 @@ std::set GetOldRcPaths( internal::FindRcAlongsideBinary(cwd, path_to_binary); candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path}; } - vector cmd_line_rc_files = GetAllUnaryOptionValues(startup_args, "--bazelrc"); + vector 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()){ @@ -380,7 +381,8 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles( // Get the command-line provided rc, passed as --bazelrc or nothing if the // flag is absent. - vector cmd_line_rc_files = GetAllUnaryOptionValues(cmd_line->startup_args, "--bazelrc"); + vector 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 diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index 03ddfbad8d532b..b188a285504924 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -34,7 +34,11 @@ public static final class Options extends OptionsBase { help = "The location of the user .bazelrc file containing default values of " + "Bazel options. This option can also be specified multiple times.\n" - + "E.g. `--bazelrc=x.rc --bazelrc=y.rc` so options in both RCs will be read.\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 " diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index e69f56c499ad2b..866ba5068b324d 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -242,72 +242,80 @@ void assert_equal_vector_char_pointer(std::vector expected, std::ve } } -TEST_F(BlazeUtilTest, TestSearchNaryForEmpty) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryForEmpty) { assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "")); } -TEST_F(BlazeUtilTest, TestSearchNaryFlagNotPresent) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryFlagNotPresent) { assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithEquals) { assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag=value", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals2) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithEquals2) { assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlag) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlag) { assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnly) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnly) { assert_equal_vector_char_pointer({"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "value1"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnlyMulti) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnlyMulti) { assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag", "--flag", "value1"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithRepeatingFlagOptionsOnlyMultiWithEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithRepeatingFlagOptionsOnlyMultiWithEquals) { assert_equal_vector_char_pointer({"--flag", "value1"}, GetAllUnaryOptionValues({"bazel", "--flag=--flag", "--flag", "value1"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithEquals3) { +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, TestSearchNaryStartupOptionWithoutEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithoutEquals) { assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "--flag", "value", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryStartupOptionWithoutEquals2) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryStartupOptionWithoutEquals2) { assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals) { assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithEquals2) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals2) { assert_equal_vector_char_pointer({"value1","value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals) { assert_equal_vector_char_pointer({"value"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNaryCommandOptionWithoutEquals2) { +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals2) { assert_equal_vector_char_pointer({"value1", "value2"}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag=value1", "--flag=value2"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithEquals) { +TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithEquals) { assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({"bazel", "build", ":target", "--", "--flag", "value"}, "--flag")); } -TEST_F(BlazeUtilTest, TestSearchNarySkipsAfterDashDashWithoutEquals) { +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 From 658a9bd83ae8df7e05ccf7db79f9d386975680ec Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Fri, 5 Mar 2021 12:32:59 -0800 Subject: [PATCH 33/40] impl --- src/main/cpp/blaze_util.cc | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 4ed80902655b19..aaf1f6a3745677 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -93,7 +93,12 @@ std::vector GetAllUnaryOptionValues(const vector& args, if (result != nullptr) { // 'key' was found and 'result' has its value. values.push_back(result); + + if (ignore_after_value != nullptr && std::strcmp(result, ignore_after_value) == 0) { + break; + } } + // 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=" string, in which case // we need to advance the index to skip the next arg for later iterations. @@ -102,21 +107,7 @@ std::vector GetAllUnaryOptionValues(const vector& args, } } - if (ignore_after_value == nullptr) { - return values; - } - else { - vector new_values; - std::string ignore_after_value_str = std::string(ignore_after_value); - for (vector::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; - } + return values; } const char* SearchUnaryOption(const vector& args, From 23cdaf78674f3c046a2e9e572bb43ffb9c89973c Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 8 Mar 2021 16:49:28 -0800 Subject: [PATCH 34/40] help msg --- .../build/lib/bazel/BazelStartupOptionsModule.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index b188a285504924..dd2db2b831d569 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -34,15 +34,15 @@ public static final class Options extends OptionsBase { help = "The location of the user .bazelrc file containing default values of " + "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" + + "E.g. with " + + "`--bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc`,\n" + + " 1) x.rc and y.rc will be read.\n" + + " 2) /dev/null indicates that all further `--bazelrc`s will be ignored, " + + "so in this case z.rc will be ignored. This is useful " + + "to disable the search for a user rc file, e.g. in release builds.\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.\n" + + "directory. \n" + "Note: command line options will always supersede any option in bazelrc.") public String blazerc; From 2a5ef016d85bff9fd68dcd7cb7392996292cc6d0 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 8 Mar 2021 16:58:03 -0800 Subject: [PATCH 35/40] add integ test --- .../build/lib/bazel/BazelStartupOptionsModule.java | 2 +- src/test/shell/integration/startup_options_test.sh | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index dd2db2b831d569..cc126829118ff8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -42,7 +42,7 @@ public static final class Options extends OptionsBase { + "to disable the search for a user rc file, e.g. in release builds.\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. \n" + + "directory.\n" + "Note: command line options will always supersede any option in bazelrc.") public String blazerc; diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index ddb14768e224fd..844c05374cae1c 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -106,4 +106,16 @@ function test_multiple_bazelrc_set_different_options() { expect_log "Inherited 'common' options: --test_output=all" } +function test_bazelrc_after_devnull_ignored() { + echo "common --verbose_failures" > 1.rc + echo "common --test_output=all" > 2.rc + echo "common --definitely_invalid_config" > 3.rc + + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" "--${PRODUCT_NAME}rc=/dev/null" \ + "--${PRODUCT_NAME}rc=3.rc" build --announce_rc &> $TEST_log || fail "Should pass" + expect_log "Inherited 'common' options: --verbose_failures" + expect_log "Inherited 'common' options: --test_output=all" + expect_not_log "--definitely_invalid_config" +} + run_suite "${PRODUCT_NAME} startup options test" From cf7f5382ae2d116fbd9bdc9205ae39ba8dac803e Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Mon, 8 Mar 2021 17:08:46 -0800 Subject: [PATCH 36/40] strcmp --- src/main/cpp/blaze_util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index aaf1f6a3745677..1d28626e8a1a57 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -94,7 +94,7 @@ std::vector GetAllUnaryOptionValues(const vector& args, // 'key' was found and 'result' has its value. values.push_back(result); - if (ignore_after_value != nullptr && std::strcmp(result, ignore_after_value) == 0) { + if (ignore_after_value != nullptr && strcmp(result, ignore_after_value) == 0) { break; } } From f8f5b4bd838cf00ea37c9af54eafc01ef04bbf94 Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Wed, 10 Mar 2021 14:47:11 -0800 Subject: [PATCH 37/40] help msg tweaks and fix format (retrigger ci x2) --- .../lib/bazel/BazelStartupOptionsModule.java | 12 +- src/test/cpp/blaze_util_test.cc | 280 +++++++++++++++--- 2 files changed, 238 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index cc126829118ff8..fb67c4a01d6e9e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -33,13 +33,15 @@ public static final class Options extends OptionsBase { valueHelp = "", help = "The location of the user .bazelrc file containing default values of " - + "Bazel options. This option can also be specified multiple times.\n" + + "Bazel options. " + + "/dev/null indicates that all further `--bazelrc`s will be ignored, " + + "This is useful to disable the search for a user rc file, " + + "e.g. in release builds.\n" + + "This option can also be specified multiple times.\n" + "E.g. with " + "`--bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc`,\n" - + " 1) x.rc and y.rc will be read.\n" - + " 2) /dev/null indicates that all further `--bazelrc`s will be ignored, " - + "so in this case z.rc will be ignored. This is useful " - + "to disable the search for a user rc file, e.g. in release builds.\n" + + " 1) x.rc and y.rc are read.\n" + + " 2) z.rc is ignored due to the prior /dev/null.\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.\n" diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index 866ba5068b324d..6aaee3665dca18 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -234,88 +234,270 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) { } } -void assert_equal_vector_char_pointer(std::vector expected, std::vector actual) { - ASSERT_EQ(actual.size(), expected.size()) << "Vectors expected and actual are of unequal length"; +void assert_equal_vector_char_pointer(std::vector expected, + std::vector 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; + 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"}, "")); + 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")); + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({ + "bazel", + "build", + ":target" + }, "--flag")); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals) { + assert_equal_vector_char_pointer({ + "value" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag=value", + "build", + ":target" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals2) { + assert_equal_vector_char_pointer({ + "value1", + "value2" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag=value1", + "--flag=value2", + "build", + ":target" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithRepeatingFlag) { + assert_equal_vector_char_pointer({ + "--flag" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag", + "--flag", + "value1", + "build", + ":target" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithRepeatingFlagOptions) { + assert_equal_vector_char_pointer({ + "--flag" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag", + "--flag", + "value1" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionValuesWithEquals) { + assert_equal_vector_char_pointer({ + "--flag", + "value1" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag=--flag", + "--flag", + "value1" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals3) { + assert_equal_vector_char_pointer({ + "value1", + "value2", + "value3" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag=value1", + "--flag=value2", + "--flag=value3", + "build", + ":target" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithoutEquals) { + assert_equal_vector_char_pointer({ + "value" + }, + GetAllUnaryOptionValues({ + "bazel", + "--flag", + "value", + "build", + ":target" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithoutEquals2) { + 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")); + 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")); + 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")); + 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")); + 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")); + 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")); + 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")); + 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")); + assert_equal_vector_char_pointer({ + "/dev/null" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag", + "/dev/null", + "--flag", + "value2", + "--flag", + "value3" + }, "--flag", "/dev/null") + ); } } // namespace blaze From af82fc1796291708f19eecd34d694e72333efa2f Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Thu, 11 Mar 2021 09:45:31 -0800 Subject: [PATCH 38/40] grammar --- .../devtools/build/lib/bazel/BazelStartupOptionsModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java index fb67c4a01d6e9e..b87146f7a65648 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java @@ -35,7 +35,7 @@ public static final class Options extends OptionsBase { "The location of the user .bazelrc file containing default values of " + "Bazel options. " + "/dev/null indicates that all further `--bazelrc`s will be ignored, " - + "This is useful to disable the search for a user rc file, " + + "which is useful to disable the search for a user rc file, " + "e.g. in release builds.\n" + "This option can also be specified multiple times.\n" + "E.g. with " From c5350b677b5e8794b2a82d7acb636db2a011f93f Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Thu, 11 Mar 2021 16:15:05 -0800 Subject: [PATCH 39/40] use info --- src/test/shell/integration/startup_options_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 844c05374cae1c..b2cdd6d3d04146 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -101,7 +101,7 @@ function test_multiple_bazelrc_later_overwrites_earlier() { 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" + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" info --announce_rc &> $TEST_log || fail "Should pass" expect_log "Inherited 'common' options: --verbose_failures" expect_log "Inherited 'common' options: --test_output=all" } @@ -112,7 +112,7 @@ function test_bazelrc_after_devnull_ignored() { echo "common --definitely_invalid_config" > 3.rc bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" "--${PRODUCT_NAME}rc=/dev/null" \ - "--${PRODUCT_NAME}rc=3.rc" build --announce_rc &> $TEST_log || fail "Should pass" + "--${PRODUCT_NAME}rc=3.rc" info --announce_rc &> $TEST_log || fail "Should pass" expect_log "Inherited 'common' options: --verbose_failures" expect_log "Inherited 'common' options: --test_output=all" expect_not_log "--definitely_invalid_config" From c8d093a820a1ec434a7dcda420aa0ef877d351bc Mon Sep 17 00:00:00 2001 From: wisechengyi Date: Thu, 18 Mar 2021 13:59:55 -0700 Subject: [PATCH 40/40] doc update --- site/docs/guide.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/site/docs/guide.md b/site/docs/guide.md index 40bb606a34b527..91655157499895 100644 --- a/site/docs/guide.md +++ b/site/docs/guide.md @@ -776,8 +776,17 @@ before the command (`build`, `test`, etc). 4. **The user-specified RC file**, if specified with --bazelrc=file - This flag is optional. However, if the flag is specified, then the file must - exist. + This flag is optional but can also be specified multiple times. + + `/dev/null` indicates that all further `--bazelrc`s will be ignored, which + is useful to disable the search for a user rc file, e.g. in release builds. + + For example: + ``` + --bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc + ``` + 1. `x.rc` and `y.rc` are read. + 2. `z.rc` is ignored due to the prior `/dev/null`. In addition to this optional configuration file, Bazel looks for a global rc file. For more details, see the [global bazelrc section](#global_bazelrc).