From 90e116c2358e2d1e71818f7d39ddb609581ee211 Mon Sep 17 00:00:00 2001 From: ccalvarin Date: Tue, 15 May 2018 16:41:19 -0700 Subject: [PATCH] Add --ignore_all_rc_files startup options. This overrides --bazelrc and --[no]master_bazelrc regardless of order. Like --bazelrc and --[no]master_bazelrc, it cannot be mentioned in an rc file, this would be a contradiction. This flag is useful for testing, and for having a version-agnostic way to turn off all rc files, such as in the canonical command line reporting. Now that blazerc and bazelrc are separate, this is necessary. If explicit values for --bazelrc or --master_bazelrc are provided which are now ignored, Bazel will warn the user. #4502 Alternatives considered - We could avoid this flag but would need to have some well-documented, reusable list of the startup flags that effectively come to the same effect. This would be necessary in our integration tests and in the CommandLineEvent and other places where rc files need to be completely disabled for correctness. We decided that this startup option was more straightforward and usable for both users and Bazel devs: it shouldn't be used when more fine-grained control is needed, but provides warnings if users are likely to be confused by the outcome. RELNOTES: None. PiperOrigin-RevId: 196750704 --- src/main/cpp/bazel_startup_options.cc | 33 ++++++- src/main/cpp/bazel_startup_options.h | 6 ++ src/main/cpp/blaze.cc | 4 + src/main/cpp/option_processor.cc | 16 ++-- src/main/cpp/startup_options.cc | 38 +++++--- src/main/cpp/startup_options.h | 7 ++ .../runtime/BlazeServerStartupOptions.java | 10 +++ .../build/lib/runtime/CommandLineEvent.java | 7 +- src/test/cpp/bazel_startup_options_test.cc | 65 ++++++++++++++ src/test/cpp/rc_file_test.cc | 90 +++++++++++++++++++ src/test/cpp/startup_options_test.cc | 1 + src/test/cpp/test_util.cc | 21 +++++ src/test/cpp/test_util.h | 4 + .../lib/runtime/CommandLineEventTest.java | 18 ++-- 14 files changed, 286 insertions(+), 34 deletions(-) diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc index f3138f781b6a13..950647e55dac9e 100644 --- a/src/main/cpp/bazel_startup_options.cc +++ b/src/main/cpp/bazel_startup_options.cc @@ -16,13 +16,16 @@ #include #include "src/main/cpp/blaze_util.h" +#include "src/main/cpp/util/logging.h" #include "src/main/cpp/workspace_layout.h" namespace blaze { BazelStartupOptions::BazelStartupOptions( const WorkspaceLayout *workspace_layout) - : StartupOptions("Bazel", workspace_layout) { + : StartupOptions("Bazel", workspace_layout), + user_bazelrc_(""), + use_master_bazelrc_(true) { RegisterNullaryStartupFlag("master_bazelrc"); RegisterUnaryStartupFlag("bazelrc"); } @@ -38,12 +41,20 @@ blaze_exit_code::ExitCode BazelStartupOptions::ProcessArgExtra( *error = "Can't specify --bazelrc in the .bazelrc file."; return blaze_exit_code::BAD_ARGV; } - } else if (GetNullaryOption(arg, "--nomaster_bazelrc") || - GetNullaryOption(arg, "--master_bazelrc")) { + user_bazelrc_ = *value; + } else if (GetNullaryOption(arg, "--master_bazelrc")) { if (!rcfile.empty()) { - *error = "Can't specify --[no]master_bazelrc in .bazelrc file."; + *error = "Can't specify --master_bazelrc in .bazelrc file."; return blaze_exit_code::BAD_ARGV; } + use_master_bazelrc_ = true; + option_sources["blazerc"] = rcfile; + } else if (GetNullaryOption(arg, "--nomaster_bazelrc")) { + if (!rcfile.empty()) { + *error = "Can't specify --nomaster_bazelrc in .bazelrc file."; + return blaze_exit_code::BAD_ARGV; + } + use_master_bazelrc_ = false; option_sources["blazerc"] = rcfile; } else { *is_processed = false; @@ -54,4 +65,18 @@ blaze_exit_code::ExitCode BazelStartupOptions::ProcessArgExtra( return blaze_exit_code::SUCCESS; } +void BazelStartupOptions::MaybeLogStartupOptionWarnings() const { + if (ignore_all_rc_files) { + if (!user_bazelrc_.empty()) { + BAZEL_LOG(WARNING) << "Value of --bazelrc is ignored, since " + "--ignore_all_rc_files is on."; + } + if ((use_master_bazelrc_) && + option_sources.find("blazerc") != option_sources.end()) { + BAZEL_LOG(WARNING) << "Explicit value of --master_bazelrc is " + "ignored, since --ignore_all_rc_files is on."; + } + } +} + } // namespace blaze diff --git a/src/main/cpp/bazel_startup_options.h b/src/main/cpp/bazel_startup_options.h index 53ce6cb0210545..b27aaef613845d 100644 --- a/src/main/cpp/bazel_startup_options.h +++ b/src/main/cpp/bazel_startup_options.h @@ -29,6 +29,12 @@ class BazelStartupOptions : public StartupOptions { blaze_exit_code::ExitCode ProcessArgExtra( const char *arg, const char *next_arg, const std::string &rcfile, const char **value, bool *is_processed, std::string *error) override; + + void MaybeLogStartupOptionWarnings() const override; + + private: + std::string user_bazelrc_; + bool use_master_bazelrc_; }; } // namespace blaze diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 331c4c212bd80b..655406ae9493de 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -1473,6 +1473,10 @@ int Main(int argc, const char *argv[], WorkspaceLayout *workspace_layout, // If client_debug was false, this is ignored, so it's accurate. BAZEL_LOG(INFO) << "Debug logging requested, sending all client log " "statements to stderr"; + // TODO(b/79206210): Can't log this before SetDebugLog is called, since the + // warning might get swallowed. Once the bug is fixed, move this call to + // OptionProcessor::ParseOptions where the order of operations is more clear. + globals->options->MaybeLogStartupOptionWarnings(); blaze::CreateSecureOutputRoot(globals->options->output_user_root); diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index 6fa48fee2ad760..7df0af72030c71 100644 --- a/src/main/cpp/option_processor.cc +++ b/src/main/cpp/option_processor.cc @@ -292,15 +292,19 @@ blaze_exit_code::ExitCode OptionProcessor::ParseOptions( return blaze_exit_code::BAD_ARGV; } - // Read the rc files. This depends on the startup options in argv since these - // may contain rc-modifying options. For all other options, the precedence of + // Read the rc files, unless --ignore_all_rc_files was provided on the command + // line. This depends on the startup options in argv since these may contain + // other rc-modifying options. For all other options, the precedence of // options will be rc first, then command line options, though, despite this // exception. std::vector> rc_files; - const blaze_exit_code::ExitCode rc_parsing_exit_code = GetRcFiles( - workspace_layout_, workspace, cwd, cmd_line_.get(), &rc_files, error); - if (rc_parsing_exit_code != blaze_exit_code::SUCCESS) { - return rc_parsing_exit_code; + if (!SearchNullaryOption(cmd_line_->startup_args, "ignore_all_rc_files", + false)) { + const blaze_exit_code::ExitCode rc_parsing_exit_code = GetRcFiles( + workspace_layout_, workspace, cwd, cmd_line_.get(), &rc_files, error); + if (rc_parsing_exit_code != blaze_exit_code::SUCCESS) { + return rc_parsing_exit_code; + } } // Parse the startup options in the correct priority order. diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index f2cc25dea02480..5ccec1d635c03d 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -71,6 +71,7 @@ void StartupOptions::RegisterUnaryStartupFlag(const std::string &flag_name) { StartupOptions::StartupOptions(const string &product_name, const WorkspaceLayout *workspace_layout) : product_name(product_name), + ignore_all_rc_files(false), deep_execroot(true), block_for_lock(true), host_jvm_debug(false), @@ -125,12 +126,13 @@ StartupOptions::StartupOptions(const string &product_name, RegisterNullaryStartupFlag("block_for_lock"); RegisterNullaryStartupFlag("client_debug"); RegisterNullaryStartupFlag("deep_execroot"); + RegisterNullaryStartupFlag("expand_configs_in_place"); RegisterNullaryStartupFlag("experimental_oom_more_eagerly"); RegisterNullaryStartupFlag("fatal_event_bus_exceptions"); RegisterNullaryStartupFlag("host_jvm_debug"); + RegisterNullaryStartupFlag("ignore_all_rc_files"); RegisterNullaryStartupFlag("watchfs"); RegisterNullaryStartupFlag("write_command_log"); - RegisterNullaryStartupFlag("expand_configs_in_place"); RegisterUnaryStartupFlag("command_port"); RegisterUnaryStartupFlag("connect_timeout_secs"); RegisterUnaryStartupFlag("experimental_oom_more_eagerly_threshold"); @@ -225,6 +227,20 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( NULL) { host_jvm_args.push_back(value); option_sources["host_jvm_args"] = rcfile; // NB: This is incorrect + } else if (GetNullaryOption(arg, "--ignore_all_rc_files")) { + if (!rcfile.empty()) { + *error = "Can't specify --ignore_all_rc_files in an rc file."; + return blaze_exit_code::BAD_ARGV; + } + ignore_all_rc_files = true; + option_sources["ignore_all_rc_files"] = rcfile; + } else if (GetNullaryOption(arg, "--noignore_all_rc_files")) { + if (!rcfile.empty()) { + *error = "Can't specify --noignore_all_rc_files in an rc file."; + return blaze_exit_code::BAD_ARGV; + } + ignore_all_rc_files = false; + option_sources["ignore_all_rc_files"] = rcfile; } else if (GetNullaryOption(arg, "--batch")) { batch = true; option_sources["batch"] = rcfile; @@ -243,8 +259,8 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( } else if (GetNullaryOption(arg, "--nofatal_event_bus_exceptions")) { fatal_event_bus_exceptions = false; option_sources["fatal_event_bus_exceptions"] = rcfile; - } else if ((value = GetUnaryOption(arg, next_arg, - "--io_nice_level")) != NULL) { + } else if ((value = GetUnaryOption(arg, next_arg, "--io_nice_level")) != + NULL) { if (!blaze_util::safe_strto32(value, &io_nice_level) || io_nice_level > 7) { blaze_util::StringPrintf(error, @@ -253,8 +269,8 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( return blaze_exit_code::BAD_ARGV; } option_sources["io_nice_level"] = rcfile; - } else if ((value = GetUnaryOption(arg, next_arg, - "--max_idle_secs")) != NULL) { + } else if ((value = GetUnaryOption(arg, next_arg, "--max_idle_secs")) != + NULL) { if (!blaze_util::safe_strto32(value, &max_idle_secs) || max_idle_secs < 0) { blaze_util::StringPrintf(error, @@ -305,8 +321,8 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( } else if (GetNullaryOption(arg, "--noexpand_configs_in_place")) { expand_configs_in_place = false; option_sources["expand_configs_in_place"] = rcfile; - } else if ((value = GetUnaryOption( - arg, next_arg, "--connect_timeout_secs")) != NULL) { + } else if ((value = GetUnaryOption(arg, next_arg, + "--connect_timeout_secs")) != NULL) { if (!blaze_util::safe_strto32(value, &connect_timeout_secs) || connect_timeout_secs < 1 || connect_timeout_secs > 120) { blaze_util::StringPrintf(error, @@ -316,8 +332,8 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( return blaze_exit_code::BAD_ARGV; } option_sources["connect_timeout_secs"] = rcfile; - } else if ((value = GetUnaryOption( - arg, next_arg, "--command_port")) != NULL) { + } else if ((value = GetUnaryOption(arg, next_arg, "--command_port")) != + NULL) { if (!blaze_util::safe_strto32(value, &command_port) || command_port < 0 || command_port > 65535) { blaze_util::StringPrintf(error, @@ -327,8 +343,8 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( return blaze_exit_code::BAD_ARGV; } option_sources["command_port"] = rcfile; - } else if ((value = GetUnaryOption(arg, next_arg, "--invocation_policy")) - != NULL) { + } else if ((value = GetUnaryOption(arg, next_arg, "--invocation_policy")) != + NULL) { if (invocation_policy == NULL) { invocation_policy = value; option_sources["invocation_policy"] = rcfile; diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 242a383fa30e1b..f6ffd25b6ec8a5 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -133,6 +133,10 @@ class StartupOptions { const char *arg, const char *next_arg, const std::string &rcfile, const char **value, bool *is_processed, std::string *error) = 0; + // Once startup options have been parsed, warn the user if certain options + // might combine in surprising ways. + virtual void MaybeLogStartupOptionWarnings() const = 0; + // Returns the absolute path to the user's local JDK install, to be used as // the default target javabase and as a fall-back host_javabase. This is not // the embedded JDK. @@ -213,6 +217,9 @@ class StartupOptions { // output_base. std::string output_user_root; + // Override more finegrained rc file flags and ignore them all. + bool ignore_all_rc_files; + // Whether to put the execroot at $OUTPUT_BASE/$WORKSPACE_NAME (if false) or // $OUTPUT_BASE/execroot/$WORKSPACE_NAME (if true). bool deep_execroot; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index a5b81be80bc12e..90684c5946841e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -289,6 +289,16 @@ public String getTypeDescription() { ) public boolean batchCpuScheduling; + @Option( + name = "ignore_all_rc_files", + defaultValue = "false", // NOTE: purely decorative, rc files are read by the client. + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.CHANGES_INPUTS}, + help = + "Disables all rc files, regardless of the values of other rc-modifying flags, even if " + + "these flags come later in the list of startup options.") + public boolean ignoreAllRcFiles; + @Option( name = "blazerc", defaultValue = "null", // NOTE: purely decorative, rc files are read by the client. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java index afa68079ed7c26..1bf99be2c40d55 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java @@ -319,7 +319,7 @@ private CommandLineSection getCanonicalStartupOptions() { // Create the fake ones to prevent reapplication of the original rc file contents. OptionsParser fakeOptions = OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); try { - fakeOptions.parse("--nomaster_blazerc", "--blazerc=/dev/null"); + fakeOptions.parse("--ignore_all_rc_files"); } catch (OptionsParsingException e) { // Unless someone changes the definition of these flags, this is impossible. throw new IllegalStateException(e); @@ -336,8 +336,11 @@ private CommandLineSection getCanonicalStartupOptions() { .filter( option -> { String optionName = option.getOptionName(); - return !optionName.equals("blazerc") + return !optionName.equals("ignore_all_rc_files") + && !optionName.equals("blazerc") && !optionName.equals("master_blazerc") + && !optionName.equals("bazelrc") + && !optionName.equals("master_bazelrc") && !optionName.equals("invocation_policy"); }) .collect(Collectors.toList())) diff --git a/src/test/cpp/bazel_startup_options_test.cc b/src/test/cpp/bazel_startup_options_test.cc index 2ac4c7f61b431a..686f43178dc0ca 100644 --- a/src/test/cpp/bazel_startup_options_test.cc +++ b/src/test/cpp/bazel_startup_options_test.cc @@ -86,6 +86,7 @@ TEST_F(BazelStartupOptionsTest, ValidStartupFlags) { ExpectIsNullaryOption(options, "experimental_oom_more_eagerly"); ExpectIsNullaryOption(options, "fatal_event_bus_exceptions"); ExpectIsNullaryOption(options, "host_jvm_debug"); + ExpectIsNullaryOption(options, "ignore_all_rc_files"); ExpectIsNullaryOption(options, "master_bazelrc"); ExpectIsNullaryOption(options, "watchfs"); ExpectIsNullaryOption(options, "write_command_log"); @@ -111,4 +112,68 @@ TEST_F(BazelStartupOptionsTest, BlazercFlagsAreNotAccepted) { EXPECT_FALSE(startup_options_->IsUnary("--blazerc")); } +TEST_F(BazelStartupOptionsTest, IgnoredBazelrcFlagWarns) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), {"--bazelrc=somefile", "--ignore_all_rc_files"}, + "WARNING: Value of --bazelrc is ignored, since --ignore_all_rc_files is " + "on.\n"); +} + +TEST_F(BazelStartupOptionsTest, IgnoredBazelrcFlagWarnsWhenAfterIgnore) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), {"--ignore_all_rc_files", "--bazelrc=somefile"}, + "WARNING: Value of --bazelrc is ignored, since --ignore_all_rc_files is " + "on.\n"); +} + +TEST_F(BazelStartupOptionsTest, IgnoredMasterBazelrcFlagWarns) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), {"--master_bazelrc", "--ignore_all_rc_files"}, + "WARNING: Explicit value of --master_bazelrc is ignored, " + "since --ignore_all_rc_files is on.\n"); +} + +TEST_F(BazelStartupOptionsTest, IgnoredMasterBazelrcFlagWarnsAfterIgnore) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), {"--ignore_all_rc_files", "--master_bazelrc"}, + "WARNING: Explicit value of --master_bazelrc is ignored, " + "since --ignore_all_rc_files is on.\n"); +} + +TEST_F(BazelStartupOptionsTest, MultipleIgnoredRcFlagsWarnOnceEach) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), + {"--master_bazelrc", "--bazelrc=somefile", "--ignore_all_rc_files", + "--bazelrc=thefinalfile", "--master_bazelrc"}, + "WARNING: Value of --bazelrc is ignored, " + "since --ignore_all_rc_files is on.\n" + "WARNING: Explicit value of --master_bazelrc is ignored, " + "since --ignore_all_rc_files is on.\n"); +} + +TEST_F(BazelStartupOptionsTest, IgnoredNoMasterBazelrcDoesNotWarn) { + // Warning for nomaster would feel pretty spammy - it's redundant, but the + // behavior is as one would expect, so warning is unnecessary. + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), {"--ignore_all_rc_files", "--nomaster_bazelrc"}, + ""); +} + +TEST_F(BazelStartupOptionsTest, IgnoreOptionDoesNotWarnOnItsOwn) { + ParseStartupOptionsAndExpectWarning(startup_options_.get(), + {"--ignore_all_rc_files"}, ""); +} + +TEST_F(BazelStartupOptionsTest, NonIgnoredOptionDoesNotWarn) { + ParseStartupOptionsAndExpectWarning(startup_options_.get(), + {"--bazelrc=somefile"}, ""); +} + +TEST_F(BazelStartupOptionsTest, FinalValueOfIgnoreIsUsedForWarning) { + ParseStartupOptionsAndExpectWarning( + startup_options_.get(), + {"--ignore_all_rc_files", "--master_bazelrc", "--noignore_all_rc_files"}, + ""); +} + } // namespace blaze diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc index 70b3f931ed9925..5900db1cd104d5 100644 --- a/src/test/cpp/rc_file_test.cc +++ b/src/test/cpp/rc_file_test.cc @@ -229,6 +229,96 @@ TEST_F(GetRcFileTest, GetRcFilesReadsUserRcInWorkspace) { using ParseOptionsTest = RcFileTest; +TEST_F(ParseOptionsTest, IgnoreAllRcFilesIgnoresAllMasterAndUserRcFiles) { + // Put fake options in different expected rc files, to check that none of them + // are read. + std::string user_workspace_rc; + ASSERT_TRUE( + SetUpUserRcFileInWorkspace("startup --userfoo", &user_workspace_rc)); + std::string workspace_rc; + ASSERT_TRUE(SetUpMasterRcFileInWorkspace("startup --workspacemasterfoo", + &workspace_rc)); + std::string binary_rc; + ASSERT_TRUE(SetUpMasterRcFileAlongsideBinary("startup --binarymasterfoo", + &binary_rc)); + + const std::vector args = {binary_path_, "--ignore_all_rc_files", + "build"}; + // Expect no error due to the incorrect options, as non of them should have + // been loaded. + std::string error; + EXPECT_EQ(blaze_exit_code::SUCCESS, + option_processor_->ParseOptions(args, workspace_, cwd_, &error)); + ASSERT_EQ("", error); + + // Check that the startup options' provenance message contains nothing + testing::internal::CaptureStderr(); + option_processor_->PrintStartupOptionsProvenanceMessage(); + const std::string& output = testing::internal::GetCapturedStderr(); + + EXPECT_EQ(output, ""); +} + +TEST_F(ParseOptionsTest, LaterIgnoreRcFileValueWins) { + std::string workspace_rc; + ASSERT_TRUE(SetUpMasterRcFileInWorkspace("startup --workspacemasterfoo", + &workspace_rc)); + + const std::vector args = {binary_path_, "--ignore_all_rc_files", + "--noignore_all_rc_files", "build"}; + std::string error; + EXPECT_EQ(blaze_exit_code::BAD_ARGV, + option_processor_->ParseOptions(args, workspace_, cwd_, &error)); + ASSERT_EQ( + "Unknown startup option: '--workspacemasterfoo'.\n For more info, run " + "'bazel help startup_options'.", + error); + + // Check that the startup options' provenance message contains the provenance + // of the incorrect option. + testing::internal::CaptureStderr(); + option_processor_->PrintStartupOptionsProvenanceMessage(); + const std::string& output = testing::internal::GetCapturedStderr(); + + EXPECT_THAT(output, + MatchesRegex("INFO: Reading 'startup' options from .*bazel.rc: " + "--workspacemasterfoo\n")); +} + +TEST_F(ParseOptionsTest, IgnoreAllRcFilesIgnoresCommandLineRcFileToo) { + // Put fake options in different expected rc files, to check that none of them + // are read. + std::string workspace_rc; + ASSERT_TRUE(SetUpMasterRcFileInWorkspace("startup --workspacemasterfoo", + &workspace_rc)); + std::string binary_rc; + ASSERT_TRUE(SetUpMasterRcFileAlongsideBinary("startup --binarymasterfoo", + &binary_rc)); + const std::string cmdline_rc_path = + blaze_util::JoinPath(workspace_, "mybazelrc"); + ASSERT_TRUE( + blaze_util::MakeDirectories(blaze_util::Dirname(cmdline_rc_path), 0755)); + ASSERT_TRUE( + blaze_util::WriteFile("startup --userfoo", cmdline_rc_path, 0755)); + + const std::vector args = {binary_path_, "--ignore_all_rc_files", + "--bazelrc=" + cmdline_rc_path, + "build"}; + // Expect no error due to the incorrect options, as non of them should have + // been loaded. + std::string error; + EXPECT_EQ(blaze_exit_code::SUCCESS, + option_processor_->ParseOptions(args, workspace_, cwd_, &error)); + ASSERT_EQ("", error); + + // Check that the startup options' provenance message contains nothing + testing::internal::CaptureStderr(); + option_processor_->PrintStartupOptionsProvenanceMessage(); + const std::string& output = testing::internal::GetCapturedStderr(); + + EXPECT_EQ(output, ""); +} + TEST_F(ParseOptionsTest, CommandLineBazelrcHasUnknownOption) { const std::string cmdline_rc_path = blaze_util::JoinPath(workspace_, "mybazelrc"); diff --git a/src/test/cpp/startup_options_test.cc b/src/test/cpp/startup_options_test.cc index ecc7ac17024c1c..9a0b01c31113b0 100644 --- a/src/test/cpp/startup_options_test.cc +++ b/src/test/cpp/startup_options_test.cc @@ -34,6 +34,7 @@ class FakeStartupOptions : public StartupOptions { *is_processed = false; return blaze_exit_code::SUCCESS; } + void MaybeLogStartupOptionWarnings() const override {} }; class StartupOptionsTest : public ::testing::Test { diff --git a/src/test/cpp/test_util.cc b/src/test/cpp/test_util.cc index b50f74d79c0d79..f56fd8798b3932 100644 --- a/src/test/cpp/test_util.cc +++ b/src/test/cpp/test_util.cc @@ -50,4 +50,25 @@ void ExpectIsUnaryOption(const StartupOptions* options, EXPECT_FALSE(options->IsNullary("--no" + flag_name)); } +void ParseStartupOptionsAndExpectWarning( + StartupOptions* startup_options, + const std::vector& options_to_parse, + const std::string& expected_warning) { + std::vector flags; + for (std::string option : options_to_parse) { + flags.push_back(RcStartupFlag("", option)); + } + + std::string error; + EXPECT_EQ(blaze_exit_code::SUCCESS, + startup_options->ProcessArgs(flags, &error)); + ASSERT_EQ("", error); + + testing::internal::CaptureStderr(); + startup_options->MaybeLogStartupOptionWarnings(); + const std::string& output = testing::internal::GetCapturedStderr(); + + EXPECT_EQ(expected_warning, output); +} + } // namespace blaze diff --git a/src/test/cpp/test_util.h b/src/test/cpp/test_util.h index e571f6b134e708..6c23c2ee15ecde 100644 --- a/src/test/cpp/test_util.h +++ b/src/test/cpp/test_util.h @@ -23,6 +23,10 @@ void ExpectIsNullaryOption(const StartupOptions* options, const std::string& flag_name); void ExpectIsUnaryOption(const StartupOptions* options, const std::string& flag_name); +void ParseStartupOptionsAndExpectWarning( + StartupOptions* startup_options, + const std::vector& options_to_parse, + const std::string& expected_warning); } // namespace blaze diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java index 6c7f2929f1ea0d..71ad57531440c3 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java @@ -93,11 +93,9 @@ public void testMostlyEmpty_CanonicalCommandLine() { checkCommandLineSectionLabels(line); assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); - assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) - .isEqualTo("--nomaster_blazerc"); - assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) - .isEqualTo("--blazerc=/dev/null"); + .isEqualTo("--ignore_all_rc_files"); assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); @@ -201,11 +199,9 @@ public void testBlazercs_CanonicalCommandLine() throws OptionsParsingException { // Expect the provided rc-related startup options are removed and replaced with the // rc-prevention options. assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); - assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) - .isEqualTo("--nomaster_blazerc"); - assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) - .isEqualTo("--blazerc=/dev/null"); + .isEqualTo("--ignore_all_rc_files"); assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); @@ -281,7 +277,7 @@ public void testOptionsAtVariousPriorities_CanonicalCommandLine() throws Options checkCommandLineSectionLabels(line); assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); - assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); // In the canonical line, expect the options in priority order. assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(4); @@ -347,7 +343,7 @@ public void testExpansionOption_CanonicalCommandLine() throws OptionsParsingExce checkCommandLineSectionLabels(line); assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); - assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(4); @@ -419,7 +415,7 @@ public void testOptionWithImplicitRequirement_CanonicalCommandLine() // Unlike expansion flags, implicit requirements are not listed separately. assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); - assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(1); assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm())