Skip to content

Commit

Permalink
Add --ignore_all_rc_files startup options.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cvcal authored and Copybara-Service committed May 15, 2018
1 parent 0015d18 commit 90e116c
Show file tree
Hide file tree
Showing 14 changed files with 286 additions and 34 deletions.
33 changes: 29 additions & 4 deletions src/main/cpp/bazel_startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
#include <cassert>

#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");
}
Expand All @@ -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;
Expand All @@ -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
6 changes: 6 additions & 0 deletions src/main/cpp/bazel_startup_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/main/cpp/blaze.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
16 changes: 10 additions & 6 deletions src/main/cpp/option_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::unique_ptr<RcFile>> 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.
Expand Down
38 changes: 27 additions & 11 deletions src/main/cpp/startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions src/main/cpp/startup_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()))
Expand Down
65 changes: 65 additions & 0 deletions src/test/cpp/bazel_startup_options_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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
Loading

0 comments on commit 90e116c

Please sign in to comment.