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). diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index d30c0d39647d8e..1d28626e8a1a57 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; @@ -74,6 +76,40 @@ bool GetNullaryOption(const char *arg, const char *key) { return true; } +std::vector GetAllUnaryOptionValues(const vector& args, + const char *key, + const char *ignore_after_value) { + vector values; + for (vector::size_type i = 0; i < args.size(); ++i) { + if (args[i] == "--") { + // "--" means all remaining args aren't options + return values; + } + + const char* next_arg = args[std::min(i + 1, args.size() - 1)].c_str(); + const char* result = GetUnaryOption(args[i].c_str(), + next_arg, + key); + if (result != nullptr) { + // 'key' was found and 'result' has its value. + values.push_back(result); + + if (ignore_after_value != nullptr && 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. + if (result == next_arg) { + i++; + } + } + + return values; +} + const char* SearchUnaryOption(const vector& 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..8b28242ee9e6e3 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -53,6 +53,16 @@ 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. +// 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* ignore_after_value=nullptr); + // Searches for '--flag_name' and '--noflag_name' in 'args' using // GetNullaryOption. Arguments found after '--' are omitted from the search. // Returns true if '--flag_name' is a flag in args and '--noflag_name' does not diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index dc3455f8815492..40cf990cbd4245 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -201,11 +201,22 @@ 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 = + GetAllUnaryOptionValues(startup_args, "--bazelrc", "/dev/null"); + // Divide the cases where the vector is empty vs not, as `FindLegacyUserBazelrc` + // has a case for rc_file to be a nullptr. + if (cmd_line_rc_files.empty()){ + string user_bazelrc_path = internal::FindLegacyUserBazelrc(nullptr, workspace); + if (!user_bazelrc_path.empty()) { + candidate_bazelrc_paths.push_back(user_bazelrc_path); + } + } else { + for (auto& rc_file : cmd_line_rc_files) { + string user_bazelrc_path = internal::FindLegacyUserBazelrc(rc_file.c_str(), workspace); + if (!user_bazelrc_path.empty()) { + candidate_bazelrc_paths.push_back(user_bazelrc_path); + } + } } // DedupeBlazercPaths returns paths whose canonical path could be computed, // therefore these paths must exist. @@ -370,20 +381,19 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles( // Get the command-line provided rc, passed as --bazelrc or nothing if the // flag is absent. - const char* cmd_line_rc_file = - SearchUnaryOption(cmd_line->startup_args, "--bazelrc", - /* warn_if_dupe */ true); - if (cmd_line_rc_file != nullptr) { - string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_file); - // Unlike the previous 3 paths, where we ignore it if the file does not - // exist or is unreadable, since this path is explicitly passed, this is an - // error. Check this condition here. - if (!blaze_util::CanReadFile(absolute_cmd_line_rc)) { - BAZEL_LOG(ERROR) << "Error: Unable to read .bazelrc file '" - << absolute_cmd_line_rc << "'."; - return blaze_exit_code::BAD_ARGV; - } - rc_files.push_back(absolute_cmd_line_rc); + vector cmd_line_rc_files = + GetAllUnaryOptionValues(cmd_line->startup_args, "--bazelrc", "/dev/null"); + for (auto& rc_file : cmd_line_rc_files) { + string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(rc_file); + // Unlike the previous 3 paths, where we ignore it if the file does not + // exist or is unreadable, since this path is explicitly passed, this is an + // error. Check this condition here. + if (!blaze_util::CanReadFile(absolute_cmd_line_rc)) { + BAZEL_LOG(ERROR) << "Error: Unable to read .bazelrc file '" + << absolute_cmd_line_rc << "'."; + return blaze_exit_code::BAD_ARGV; + } + rc_files.push_back(absolute_cmd_line_rc); } // Log which files we're looking for before removing duplicates and 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..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 @@ -33,10 +33,19 @@ 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. " + + "/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.\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 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. Use /dev/null to disable the search for a user rc file, e.g. in " - + "release builds.") + + "directory.\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 diff --git a/src/test/cpp/blaze_util_test.cc b/src/test/cpp/blaze_util_test.cc index dda7bb51388760..6aaee3665dca18 100644 --- a/src/test/cpp/blaze_util_test.cc +++ b/src/test/cpp/blaze_util_test.cc @@ -234,4 +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"; + + for (int i = 0; i < actual.size(); ++i) { + ASSERT_EQ(actual[i], expected[i]) + << "Vectors expected and actual differ at index " << i; + } +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryForEmpty) { + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({ + "bazel", + "build", + ":target" + }, "")); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryFlagNotPresent) { + assert_equal_vector_char_pointer({}, GetAllUnaryOptionValues({ + "bazel", + "build", + ":target" + }, "--flag")); +} + +TEST_F(BlazeUtilTest, 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") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals2) { + assert_equal_vector_char_pointer({ + "value1", + "value2" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag", + "value1", + "--flag", + "value2" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals) { + assert_equal_vector_char_pointer({ + "value" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag=value" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals2) { + assert_equal_vector_char_pointer({ + "value1", + "value2" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag=value1", + "--flag=value2" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithEquals) { + assert_equal_vector_char_pointer({}, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--", + "--flag", + "value" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithoutEquals) { + assert_equal_vector_char_pointer({}, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--", + "--flag=value" + }, "--flag") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfter) { + assert_equal_vector_char_pointer({ + "value1", + "/dev/null" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag", + "value1", + "--flag", + "/dev/null", + "--flag", + "value3" + }, "--flag", "/dev/null") + ); +} + +TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfterDevNull) { + assert_equal_vector_char_pointer({ + "/dev/null" + }, + GetAllUnaryOptionValues({ + "bazel", + "build", + ":target", + "--flag", + "/dev/null", + "--flag", + "value2", + "--flag", + "value3" + }, "--flag", "/dev/null") + ); +} + } // namespace blaze diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 63f1624866d41d..b2cdd6d3d04146 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -82,4 +82,40 @@ function test_autodetect_server_javabase() { bazel --noautodetect_server_javabase version &> $TEST_log || fail "Should pass" } +# Below are the regression tests for Issue #7489 +function test_multiple_bazelrc_later_overwrites_earlier() { + # Help message only visible with --help_verbosity=medium + help_message_in_description="--${PRODUCT_NAME}rc (a string; default: see description)" + + echo "help --help_verbosity=short" > 1.rc + echo "help --help_verbosity=medium" > 2.rc + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass" + expect_log "$help_message_in_description" + + echo "help --help_verbosity=medium" > 1.rc + echo "help --help_verbosity=short" > 2.rc + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass" + expect_not_log "$help_message_in_description" +} + +function test_multiple_bazelrc_set_different_options() { + echo "common --verbose_failures" > 1.rc + echo "common --test_output=all" > 2.rc + bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" info --announce_rc &> $TEST_log || fail "Should pass" + expect_log "Inherited 'common' options: --verbose_failures" + 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" 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" +} + run_suite "${PRODUCT_NAME} startup options test"