Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client, refactor: simplify nullary flag handling #9121

Closed
wants to merge 3 commits into from
Closed

Client, refactor: simplify nullary flag handling #9121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7acf89
Client, refactor: simplify nullary flag handling
laszlocsomor Aug 8, 2019
f71ca18
Update comments.
laszlocsomor Aug 8, 2019
a298194
Support special handlers for nullary flags
laszlocsomor Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 4 additions & 60 deletions src/main/cpp/bazel_startup_options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ BazelStartupOptions::BazelStartupOptions(
use_workspace_rc(true),
use_home_rc(true),
use_master_bazelrc_(true) {
RegisterNullaryStartupFlag("home_rc");
RegisterNullaryStartupFlag("master_bazelrc");
RegisterNullaryStartupFlag("system_rc");
RegisterNullaryStartupFlag("workspace_rc");
RegisterNullaryStartupFlagNoRc("home_rc", &use_home_rc);
RegisterNullaryStartupFlagNoRc("master_bazelrc", &use_master_bazelrc_);
RegisterNullaryStartupFlagNoRc("system_rc", &use_system_rc);
RegisterNullaryStartupFlagNoRc("workspace_rc", &use_workspace_rc);
RegisterUnaryStartupFlag("bazelrc");
}

Expand All @@ -48,62 +48,6 @@ blaze_exit_code::ExitCode BazelStartupOptions::ProcessArgExtra(
return blaze_exit_code::BAD_ARGV;
}
user_bazelrc_ = *value;
} else if (GetNullaryOption(arg, "--system_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --system_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_system_rc = true;
option_sources["system_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--nosystem_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --nosystem_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_system_rc = false;
option_sources["system_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--workspace_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --workspace_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_workspace_rc = true;
option_sources["workspace_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--noworkspace_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --noworkspace_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_workspace_rc = false;
option_sources["workspace_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--home_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --home_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_home_rc = true;
option_sources["home_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--nohome_rc")) {
if (!rcfile.empty()) {
*error = "Can't specify --nohome_rc in .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
use_home_rc = false;
option_sources["home_rc"] = rcfile;
} else if (GetNullaryOption(arg, "--master_bazelrc")) {
if (!rcfile.empty()) {
*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;
return blaze_exit_code::SUCCESS;
Expand Down
12 changes: 3 additions & 9 deletions src/main/cpp/blaze.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ class BlazeServer final {
const bool block_for_lock,
const string &output_base,
const string &server_jvm_out);
~BlazeServer();

// Acquire a lock for the server running in this output base. Returns the
// number of milliseconds spent waiting for the lock.
Expand Down Expand Up @@ -302,7 +301,7 @@ class BlazeServer final {

// Pipe that the main thread sends actions to and the cancel thread receives
// actions from.
blaze_util::IPipe *pipe_;
std::unique_ptr<blaze_util::IPipe> pipe_;

bool TryConnect(CommandServer::Stub *client);
void CancelThread();
Expand Down Expand Up @@ -1606,18 +1605,13 @@ BlazeServer::BlazeServer(
output_base_(output_base) {
gpr_set_log_function(null_grpc_log_function);

pipe_ = blaze_util::CreatePipe();
if (pipe_ == NULL) {
pipe_.reset(blaze_util::CreatePipe());
if (!pipe_.get()) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "Couldn't create pipe: " << GetLastErrorString();
}
}

BlazeServer::~BlazeServer() {
delete pipe_;
pipe_ = NULL;
}

bool BlazeServer::TryConnect(
CommandServer::Stub *client) {
grpc::ClientContext context;
Expand Down
196 changes: 70 additions & 126 deletions src/main/cpp/startup_options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,24 @@ namespace blaze {
using std::string;
using std::vector;

void StartupOptions::RegisterNullaryStartupFlag(const std::string &flag_name) {
valid_nullary_startup_flags_.insert(std::string("--") + flag_name);
valid_nullary_startup_flags_.insert(std::string("--no") + flag_name);
void StartupOptions::RegisterNullaryStartupFlag(const std::string &flag_name,
bool* flag_value) {
all_nullary_startup_flags_[std::string("--") + flag_name] = flag_value;
all_nullary_startup_flags_[std::string("--no") + flag_name] = flag_value;
}

void StartupOptions::RegisterNullaryStartupFlagNoRc(
const std::string& flag_name, bool* flag_value) {
RegisterNullaryStartupFlag(flag_name, flag_value);
no_rc_nullary_startup_flags_.insert(std::string("--") + flag_name);
no_rc_nullary_startup_flags_.insert(std::string("--no") + flag_name);
}

void StartupOptions::RegisterSpecialNullaryStartupFlag(
const std::string &flag_name, SpecialNullaryFlagHandler handler) {
RegisterNullaryStartupFlag(flag_name, nullptr);
special_nullary_startup_flags_[std::string("--") + flag_name] = handler;
special_nullary_startup_flags_[std::string("--no") + flag_name] = handler;
}

void StartupOptions::RegisterUnaryStartupFlag(const std::string &flag_name) {
Expand Down Expand Up @@ -105,22 +120,28 @@ StartupOptions::StartupOptions(const string &product_name,
// IMPORTANT: Before modifying the statements below please contact a Bazel
// core team member that knows the internal procedure for adding/deprecating
// startup flags.
RegisterNullaryStartupFlag("batch");
RegisterNullaryStartupFlag("batch_cpu_scheduling");
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("idle_server_tasks");
RegisterNullaryStartupFlag("incompatible_enable_execution_transition");
RegisterNullaryStartupFlag("shutdown_on_low_sys_mem");
RegisterNullaryStartupFlag("ignore_all_rc_files");
RegisterNullaryStartupFlag("unlimit_coredumps");
RegisterNullaryStartupFlag("watchfs");
RegisterNullaryStartupFlag("write_command_log");
RegisterNullaryStartupFlag("batch", &batch);
RegisterNullaryStartupFlag("batch_cpu_scheduling", &batch_cpu_scheduling);
RegisterNullaryStartupFlag("block_for_lock", &block_for_lock);
RegisterNullaryStartupFlag("client_debug", &client_debug);
RegisterNullaryStartupFlag("deep_execroot", &deep_execroot);
RegisterNullaryStartupFlag("expand_configs_in_place",
&expand_configs_in_place);
RegisterNullaryStartupFlag("experimental_oom_more_eagerly",
&oom_more_eagerly);
RegisterNullaryStartupFlag("fatal_event_bus_exceptions",
&fatal_event_bus_exceptions);
RegisterNullaryStartupFlag("host_jvm_debug", &host_jvm_debug);
RegisterNullaryStartupFlag("idle_server_tasks", &idle_server_tasks);
RegisterNullaryStartupFlag("incompatible_enable_execution_transition",
&incompatible_enable_execution_transition);
RegisterNullaryStartupFlag("shutdown_on_low_sys_mem",
&shutdown_on_low_sys_mem);
RegisterNullaryStartupFlagNoRc("ignore_all_rc_files",
&ignore_all_rc_files);
RegisterNullaryStartupFlag("unlimit_coredumps", &unlimit_coredumps);
RegisterNullaryStartupFlag("watchfs", &watchfs);
RegisterNullaryStartupFlag("write_command_log", &write_command_log);
RegisterUnaryStartupFlag("command_port");
RegisterUnaryStartupFlag("connect_timeout_secs");
RegisterUnaryStartupFlag("digest_function");
Expand Down Expand Up @@ -160,12 +181,12 @@ bool StartupOptions::IsUnary(const string &arg) const {
bool StartupOptions::IsNullary(const string &arg) const {
std::string::size_type i = arg.find_first_of('=');
if (i == std::string::npos) {
return valid_nullary_startup_flags_.find(arg) !=
valid_nullary_startup_flags_.end();
return all_nullary_startup_flags_.find(arg) !=
all_nullary_startup_flags_.end();
} else {
std::string f = arg.substr(0, i);
if (valid_nullary_startup_flags_.find(f) !=
valid_nullary_startup_flags_.end()) {
if (all_nullary_startup_flags_.find(f) !=
all_nullary_startup_flags_.end()) {
BAZEL_DIE(blaze_exit_code::BAD_ARGV)
<< "In argument '" << arg << "': option '" << f
<< "' does not take a value.";
Expand All @@ -192,6 +213,32 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
const char* next_arg = next_argstr.empty() ? NULL : next_argstr.c_str();
const char* value = NULL;

if (IsNullary(argstr)) {
// 'enabled' is true if 'argstr' is "--foo", and false if it's "--nofoo".
bool enabled = (argstr.compare(0, 4, "--no") != 0);
if (no_rc_nullary_startup_flags_.find(argstr) !=
no_rc_nullary_startup_flags_.end()) {
// no_rc_nullary_startup_flags_ are forbidden in .bazelrc files.
if (!rcfile.empty()) {
*error = std::string("Can't specify ") + argstr +
" in the .bazelrc file.";
return blaze_exit_code::BAD_ARGV;
}
}
if (special_nullary_startup_flags_.find(argstr) !=
special_nullary_startup_flags_.end()) {
special_nullary_startup_flags_[argstr](enabled);
} else {
// 'argstr' is either "--foo" or "--nofoo", and the map entry is the
// pointer to the bool storing the flag's value.
*all_nullary_startup_flags_[argstr] = enabled;
}
// Use the key "foo" for 'argstr' of "--foo" / "--nofoo".
option_sources[argstr.substr(enabled ? 2 : 4)] = rcfile;
*is_space_separated = false;
return blaze_exit_code::SUCCESS;
}

if ((value = GetUnaryOption(arg, next_arg, "--output_base")) != NULL) {
output_base = blaze::AbsolutePathFromFlag(value);
option_sources["output_base"] = rcfile;
Expand All @@ -207,21 +254,6 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
"--server_jvm_out")) != NULL) {
server_jvm_out = blaze::AbsolutePathFromFlag(value);
option_sources["server_jvm_out"] = rcfile;
} else if (GetNullaryOption(arg, "--deep_execroot")) {
deep_execroot = true;
option_sources["deep_execroot"] = rcfile;
} else if (GetNullaryOption(arg, "--nodeep_execroot")) {
deep_execroot = false;
option_sources["deep_execroot"] = rcfile;
} else if (GetNullaryOption(arg, "--block_for_lock")) {
block_for_lock = true;
option_sources["block_for_lock"] = rcfile;
} else if (GetNullaryOption(arg, "--noblock_for_lock")) {
block_for_lock = false;
option_sources["block_for_lock"] = rcfile;
} else if (GetNullaryOption(arg, "--host_jvm_debug")) {
host_jvm_debug = true;
option_sources["host_jvm_debug"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--host_jvm_profile")) !=
NULL) {
host_jvm_profile = value;
Expand All @@ -236,38 +268,6 @@ 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;
} else if (GetNullaryOption(arg, "--nobatch")) {
batch = false;
option_sources["batch"] = rcfile;
} else if (GetNullaryOption(arg, "--batch_cpu_scheduling")) {
batch_cpu_scheduling = true;
option_sources["batch_cpu_scheduling"] = rcfile;
} else if (GetNullaryOption(arg, "--nobatch_cpu_scheduling")) {
batch_cpu_scheduling = false;
option_sources["batch_cpu_scheduling"] = rcfile;
} else if (GetNullaryOption(arg, "--fatal_event_bus_exceptions")) {
fatal_event_bus_exceptions = true;
option_sources["fatal_event_bus_exceptions"] = rcfile;
} 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) {
if (!blaze_util::safe_strto32(value, &io_nice_level) ||
Expand Down Expand Up @@ -317,18 +317,6 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
return blaze_exit_code::BAD_ARGV;
}
option_sources["macos_qos_class"] = rcfile;
} else if (GetNullaryOption(arg, "--shutdown_on_low_sys_mem")) {
shutdown_on_low_sys_mem = true;
option_sources["shutdown_on_low_sys_mem"] = rcfile;
} else if (GetNullaryOption(arg, "--noshutdown_on_low_sys_mem")) {
shutdown_on_low_sys_mem = false;
option_sources["shutdown_on_low_sys_mem"] = rcfile;
} else if (GetNullaryOption(arg, "--experimental_oom_more_eagerly")) {
oom_more_eagerly = true;
option_sources["experimental_oom_more_eagerly"] = rcfile;
} else if (GetNullaryOption(arg, "--noexperimental_oom_more_eagerly")) {
oom_more_eagerly = false;
option_sources["experimental_oom_more_eagerly"] = rcfile;
} else if ((value = GetUnaryOption(
arg, next_arg,
"--experimental_oom_more_eagerly_threshold")) != NULL) {
Expand All @@ -342,36 +330,6 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
return blaze_exit_code::BAD_ARGV;
}
option_sources["experimental_oom_more_eagerly_threshold"] = rcfile;
} else if (GetNullaryOption(arg, "--write_command_log")) {
write_command_log = true;
option_sources["write_command_log"] = rcfile;
} else if (GetNullaryOption(arg, "--nowrite_command_log")) {
write_command_log = false;
option_sources["write_command_log"] = rcfile;
} else if (GetNullaryOption(arg, "--watchfs")) {
watchfs = true;
option_sources["watchfs"] = rcfile;
} else if (GetNullaryOption(arg, "--nowatchfs")) {
watchfs = false;
option_sources["watchfs"] = rcfile;
} else if (GetNullaryOption(arg, "--client_debug")) {
client_debug = true;
option_sources["client_debug"] = rcfile;
} else if (GetNullaryOption(arg, "--noclient_debug")) {
client_debug = false;
option_sources["client_debug"] = rcfile;
} else if (GetNullaryOption(arg, "--expand_configs_in_place")) {
expand_configs_in_place = true;
option_sources["expand_configs_in_place"] = rcfile;
} else if (GetNullaryOption(arg, "--noexpand_configs_in_place")) {
expand_configs_in_place = false;
option_sources["expand_configs_in_place"] = rcfile;
} else if (GetNullaryOption(arg, "--idle_server_tasks")) {
idle_server_tasks = true;
option_sources["idle_server_tasks"] = rcfile;
} else if (GetNullaryOption(arg, "--noidle_server_tasks")) {
idle_server_tasks = false;
option_sources["idle_server_tasks"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--connect_timeout_secs")) != NULL) {
if (!blaze_util::safe_strto32(value, &connect_timeout_secs) ||
Expand Down Expand Up @@ -409,20 +367,6 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
"multiple times.";
return blaze_exit_code::BAD_ARGV;
}
} else if (GetNullaryOption(arg, "--unlimit_coredumps")) {
unlimit_coredumps = true;
option_sources["unlimit_coredumps"] = rcfile;
} else if (GetNullaryOption(arg, "--nounlimit_coredumps")) {
unlimit_coredumps = false;
option_sources["unlimit_coredumps"] = rcfile;
} else if (GetNullaryOption(arg,
"--incompatible_enable_execution_transition")) {
incompatible_enable_execution_transition = true;
option_sources["incompatible_enable_execution_transition"] = rcfile;
} else if (GetNullaryOption(arg,
"--noincompatible_enable_execution_transition")) {
incompatible_enable_execution_transition = false;
option_sources["incompatible_enable_execution_transition"] = rcfile;
} else {
bool extra_argument_processed;
blaze_exit_code::ExitCode process_extra_arg_exit_code = ProcessArgExtra(
Expand Down
Loading