Create the StartupFlag class and use it instead of plain list of strings.
Additionally, add a warning note for developers who wish to delete startup
options: they first need to deprecate the flag and once it's a no-op
for a sufficient amount of time then they can delete it from the list
of valid options.
PiperOrigin-RevId: 160546248
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 46f0df4..21e0815 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -32,9 +32,44 @@
using std::string;
using std::vector;
+StartupFlag::~StartupFlag() {}
+
+bool UnaryStartupFlag::NeedsParameter() const {
+ return true;
+}
+
+bool UnaryStartupFlag::IsValid(const std::string &arg) const {
+ // The second argument of GetUnaryOption is not relevant to determine
+ // whether the option is unary or not, hence we set it to the empty string
+ // by default.
+ //
+ // TODO(lpino): Improve GetUnaryOption to only require the arg and the
+ // option we are looking for.
+ return GetUnaryOption(arg.c_str(), "", ("--" + name_).c_str()) != NULL;
+}
+
+bool NullaryStartupFlag::NeedsParameter() const {
+ return false;
+}
+
+bool NullaryStartupFlag::IsValid(const std::string &arg) const {
+ return GetNullaryOption(arg.c_str(), ("--" + name_).c_str()) ||
+ GetNullaryOption(arg.c_str(), ("--no" + name_).c_str());
+}
+
StartupOptions::StartupOptions(const WorkspaceLayout* workspace_layout)
: StartupOptions("Bazel", workspace_layout) {}
+void StartupOptions::RegisterNullaryStartupFlag(const std::string &flag_name) {
+ valid_startup_flags.insert(std::unique_ptr<NullaryStartupFlag>(
+ new NullaryStartupFlag(flag_name)));
+}
+
+void StartupOptions::RegisterUnaryStartupFlag(const std::string &flag_name) {
+ valid_startup_flags.insert(std::unique_ptr<UnaryStartupFlag>(
+ new UnaryStartupFlag(flag_name)));
+}
+
StartupOptions::StartupOptions(const string &product_name,
const WorkspaceLayout *workspace_layout)
: product_name(product_name),
@@ -76,24 +111,37 @@
output_root, "_" + product_name_lower + "_" + GetUserName());
// 3 hours (but only 15 seconds if used within a test)
max_idle_secs = testing ? 15 : (3 * 3600);
- nullary_options = {"deep_execroot",
- "block_for_lock",
- "host_jvm_debug",
- "master_blazerc",
- "master_bazelrc",
- "batch",
- "batch_cpu_scheduling",
- "allow_configurable_attributes",
- "fatal_event_bus_exceptions",
- "experimental_oom_more_eagerly",
- "write_command_log",
- "watchfs",
- "client_debug"};
- unary_options = {"output_base", "install_base",
- "output_user_root", "host_jvm_profile", "host_javabase",
- "host_jvm_args", "bazelrc", "blazerc", "io_nice_level",
- "max_idle_secs", "experimental_oom_more_eagerly_threshold",
- "command_port", "invocation_policy", "connect_timeout_secs"};
+
+ // IMPORTANT: Before modifying the statements below please contact a Bazel
+ // core team member that knows the internal procedure for adding/deprecating
+ // startup flags.
+ RegisterNullaryStartupFlag("allow_configurable_attributes");
+ RegisterNullaryStartupFlag("batch");
+ RegisterNullaryStartupFlag("batch_cpu_scheduling");
+ RegisterNullaryStartupFlag("block_for_lock");
+ RegisterNullaryStartupFlag("client_debug");
+ RegisterNullaryStartupFlag("deep_execroot");
+ RegisterNullaryStartupFlag("experimental_oom_more_eagerly");
+ RegisterNullaryStartupFlag("fatal_event_bus_exceptions");
+ RegisterNullaryStartupFlag("host_jvm_debug");
+ RegisterNullaryStartupFlag("master_bazelrc");
+ RegisterNullaryStartupFlag("master_blazerc");
+ RegisterNullaryStartupFlag("watchfs");
+ RegisterNullaryStartupFlag("write_command_log");
+ RegisterUnaryStartupFlag("bazelrc");
+ RegisterUnaryStartupFlag("blazerc");
+ RegisterUnaryStartupFlag("command_port");
+ RegisterUnaryStartupFlag("connect_timeout_secs");
+ RegisterUnaryStartupFlag("experimental_oom_more_eagerly_threshold");
+ RegisterUnaryStartupFlag("host_javabase");
+ RegisterUnaryStartupFlag("host_jvm_args");
+ RegisterUnaryStartupFlag("host_jvm_profile");
+ RegisterUnaryStartupFlag("invocation_policy");
+ RegisterUnaryStartupFlag("io_nice_level");
+ RegisterUnaryStartupFlag("install_base");
+ RegisterUnaryStartupFlag("max_idle_secs");
+ RegisterUnaryStartupFlag("output_base");
+ RegisterUnaryStartupFlag("output_user_root");
}
StartupOptions::~StartupOptions() {}
@@ -105,9 +153,8 @@
}
bool StartupOptions::IsNullary(const string& arg) const {
- for (string option : nullary_options) {
- if (GetNullaryOption(arg.c_str(), ("--" + option).c_str()) ||
- GetNullaryOption(arg.c_str(), ("--no" + option).c_str())) {
+ for (const auto& flag : valid_startup_flags) {
+ if (!flag->NeedsParameter() && flag->IsValid(arg)) {
return true;
}
}
@@ -115,14 +162,8 @@
}
bool StartupOptions::IsUnary(const string& arg) const {
- for (string option : unary_options) {
- // The second argument of GetUnaryOption is not relevant to determine
- // whether the option is unary or not, hence we set it to the empty string
- // by default.
- //
- // TODO(lpino): Improve GetUnaryOption to only require the arg and the
- // option we are looking for.
- if (GetUnaryOption(arg.c_str(), "", ("--" + option).c_str()) != NULL) {
+ for (const auto& flag : valid_startup_flags) {
+ if (flag->NeedsParameter() && flag->IsValid(arg)) {
return true;
}
}
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index 607810d..4b6d2b9 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -16,6 +16,7 @@
#include <map>
#include <memory>
+#include <set>
#include <string>
#include <vector>
@@ -25,6 +26,40 @@
class WorkspaceLayout;
+// Represents a single startup flag (or startup option).
+class StartupFlag {
+ public:
+ virtual ~StartupFlag() = 0;
+ virtual bool NeedsParameter() const = 0;
+ virtual bool IsValid(const std::string& arg) const = 0;
+};
+
+// A startup flag that doesn't expect a value.
+// For instance, NullaryStartupFlag("master_bazelrc") is used to represent
+// "--master_bazelrc" and "--nomaster_bazelrc".
+class NullaryStartupFlag : public StartupFlag {
+ public:
+ NullaryStartupFlag(const std::string& name) : name_(name) {}
+ bool IsValid(const std::string& arg) const override;
+ bool NeedsParameter() const override;
+
+ private:
+ const std::string name_;
+};
+
+// A startup flag that expects a value.
+// For instance, UnaryStartupFlag("bazelrc") is used to represent
+// "--bazelrc=foo" or "--bazelrc foo".
+class UnaryStartupFlag : public StartupFlag {
+ public:
+ UnaryStartupFlag(const std::string& name) : name_(name) {}
+ bool IsValid(const std::string& arg) const override;
+ bool NeedsParameter() const override;
+
+ private:
+ const std::string name_;
+};
+
// This class holds the parsed startup options for Blaze.
// These options and their defaults must be kept in sync with those in
// src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java.
@@ -221,15 +256,15 @@
explicit StartupOptions(const std::string &product_name,
const WorkspaceLayout* workspace_layout);
- // Holds the valid nullary startup options.
- std::vector<std::string> nullary_options;
+ void RegisterUnaryStartupFlag(const std::string& flag_name);
- // Holds the valid unary startup options.
- std::vector<std::string> unary_options;
+ void RegisterNullaryStartupFlag(const std::string& flag_name);
private:
std::string host_javabase;
std::string default_host_javabase;
+ // Contains the collection of startup flags that Bazel accepts.
+ std::set<std::unique_ptr<StartupFlag>> valid_startup_flags;
#if defined(COMPILER_MSVC) || defined(__CYGWIN__)
static std::string WindowsUnixRoot(const std::string &bazel_sh);
diff --git a/src/test/cpp/startup_options_test.cc b/src/test/cpp/startup_options_test.cc
index 72cdf1b..c8c59d0 100644
--- a/src/test/cpp/startup_options_test.cc
+++ b/src/test/cpp/startup_options_test.cc
@@ -45,6 +45,34 @@
startup_options_.reset(new StartupOptions(workspace_layout_.get()));
}
+ void SuccessfulIsNullaryTest(const std::string& flag_name) const {
+ EXPECT_TRUE(startup_options_->IsNullary("--" + flag_name));
+ EXPECT_TRUE(startup_options_->IsNullary("--no" + flag_name));
+
+ EXPECT_FALSE(startup_options_->IsNullary("--" + flag_name + "__invalid"));
+
+ EXPECT_DEATH(startup_options_->IsNullary("--" + flag_name + "=foo"),
+ ("In argument '--" + flag_name + "=foo': option "
+ "'--" + flag_name + "' does not take a value").c_str());
+
+ EXPECT_DEATH(startup_options_->IsNullary("--no" + flag_name + "=foo"),
+ ("In argument '--no" + flag_name + "=foo': option "
+ "'--no" + flag_name + "' does not take a value").c_str());
+
+ EXPECT_FALSE(startup_options_->IsUnary("--" + flag_name));
+ EXPECT_FALSE(startup_options_->IsUnary("--no" + flag_name));
+ }
+
+ void SuccessfulIsUnaryTest(const std::string& flag_name) const {
+ EXPECT_TRUE(startup_options_->IsUnary("--" + flag_name));
+ EXPECT_TRUE(startup_options_->IsUnary("--" + flag_name + "="));
+ EXPECT_TRUE(startup_options_->IsUnary("--" + flag_name + "=foo"));
+
+ EXPECT_FALSE(startup_options_->IsUnary("--" + flag_name + "__invalid"));
+ EXPECT_FALSE(startup_options_->IsNullary("--" + flag_name));
+ EXPECT_FALSE(startup_options_->IsNullary("--no" + flag_name));
+ }
+
private:
std::unique_ptr<WorkspaceLayout> workspace_layout_;
@@ -92,16 +120,44 @@
ASSERT_EQ("/tmp", startup_options_->output_root);
}
-TEST_F(StartupOptionsTest, IsNullaryTest) {
- EXPECT_TRUE(startup_options_->IsNullary("--master_bazelrc"));
- EXPECT_TRUE(startup_options_->IsNullary("--nomaster_bazelrc"));
+TEST_F(StartupOptionsTest, EmptyFlagsAreInvalidTest) {
EXPECT_FALSE(startup_options_->IsNullary(""));
EXPECT_FALSE(startup_options_->IsNullary("--"));
- EXPECT_FALSE(startup_options_->IsNullary("--master_bazelrcascasc"));
- string error_msg = std::string("In argument '--master_bazelrc=foo': option ")
- + std::string("'--master_bazelrc' does not take a value");
- EXPECT_DEATH(startup_options_->IsNullary("--master_bazelrc=foo"),
- error_msg.c_str());
+ EXPECT_FALSE(startup_options_->IsUnary(""));
+ EXPECT_FALSE(startup_options_->IsUnary("--"));
+}
+
+TEST_F(StartupOptionsTest, ValidStartupFlagsTest) {
+ // IMPORTANT: Before modifying this test, please contact a Bazel core team
+ // member that knows the Google-internal procedure for adding/deprecating
+ // startup flags.
+ SuccessfulIsNullaryTest("allow_configurable_attributes");
+ SuccessfulIsNullaryTest("batch");
+ SuccessfulIsNullaryTest("batch_cpu_scheduling");
+ SuccessfulIsNullaryTest("block_for_lock");
+ SuccessfulIsNullaryTest("client_debug");
+ SuccessfulIsNullaryTest("deep_execroot");
+ SuccessfulIsNullaryTest("experimental_oom_more_eagerly");
+ SuccessfulIsNullaryTest("fatal_event_bus_exceptions");
+ SuccessfulIsNullaryTest("host_jvm_debug");
+ SuccessfulIsNullaryTest("master_bazelrc");
+ SuccessfulIsNullaryTest("master_blazerc");
+ SuccessfulIsNullaryTest("watchfs");
+ SuccessfulIsNullaryTest("write_command_log");
+ SuccessfulIsUnaryTest("bazelrc");
+ SuccessfulIsUnaryTest("blazerc");
+ SuccessfulIsUnaryTest("command_port");
+ SuccessfulIsUnaryTest("connect_timeout_secs");
+ SuccessfulIsUnaryTest("experimental_oom_more_eagerly_threshold");
+ SuccessfulIsUnaryTest("host_javabase");
+ SuccessfulIsUnaryTest("host_jvm_args");
+ SuccessfulIsUnaryTest("host_jvm_profile");
+ SuccessfulIsUnaryTest("invocation_policy");
+ SuccessfulIsUnaryTest("io_nice_level");
+ SuccessfulIsUnaryTest("install_base");
+ SuccessfulIsUnaryTest("max_idle_secs");
+ SuccessfulIsUnaryTest("output_base");
+ SuccessfulIsUnaryTest("output_user_root");
}
TEST_F(StartupOptionsTest, IsUnaryTest) {
