Skip to content

Commit

Permalink
Pass StartupOptions around as a const ref as much as possible
Browse files Browse the repository at this point in the history
Nice for simplification and consistency, as well as following the style guide.

ComputeBaseDirectories still takes a StartupOptions* since it mutates it - I'll
clean this up in another change...

I left BlazeServer's ctor as is since I want to just lift the fields that it
needs off of StartupOptions. However, I want to do this when I can guarantee in
the code that StartupOptions will no longer be mutated - this'll probably come
in the same change as touching up ComputeBaseDirectories...

RELNOTES: None
PiperOrigin-RevId: 251940659
  • Loading branch information
michajlo authored and Copybara-Service committed Jun 6, 2019
1 parent 50fa3ec commit 12c7942
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 106 deletions.
180 changes: 90 additions & 90 deletions src/main/cpp/blaze.cc

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/main/cpp/blaze_util_darwin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ string GetSystemJavabase() {
}

int ConfigureDaemonProcess(posix_spawnattr_t *attrp,
const StartupOptions *options) {
return posix_spawnattr_set_qos_class_np(attrp, options->macos_qos_class);
const StartupOptions &options) {
return posix_spawnattr_set_qos_class_np(attrp, options.macos_qos_class);
}

void WriteSystemSpecificProcessIdentifier(
Expand Down
2 changes: 1 addition & 1 deletion src/main/cpp/blaze_util_freebsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ string GetSystemJavabase() {
}

int ConfigureDaemonProcess(posix_spawnattr_t *attrp,
const StartupOptions *options) {
const StartupOptions &options) {
// No interesting platform-specific details to configure on this platform.
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/cpp/blaze_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ static bool GetStartTime(const string& pid, string* start_time) {
}

int ConfigureDaemonProcess(posix_spawnattr_t* attrp,
const StartupOptions* options) {
const StartupOptions &options) {
// No interesting platform-specific details to configure on this platform.
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/cpp/blaze_util_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ int ExecuteDaemon(const std::string& exe,
const bool daemon_output_append,
const std::string& binaries_dir,
const std::string& server_dir,
const StartupOptions* options,
const StartupOptions &options,
BlazeServerStartup** server_startup);

// A character used to separate paths in a list.
Expand Down
4 changes: 2 additions & 2 deletions src/main/cpp/blaze_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ bool SocketBlazeServerStartup::IsStillAlive() {
// Returns zero on success or -1 on error, in which case errno is set to the
// corresponding error details.
int ConfigureDaemonProcess(posix_spawnattr_t* attrp,
const StartupOptions* options);
const StartupOptions &options);

void WriteSystemSpecificProcessIdentifier(
const string& server_dir, pid_t server_pid);
Expand All @@ -351,7 +351,7 @@ int ExecuteDaemon(const string& exe,
const bool daemon_output_append,
const string& binaries_dir,
const string& server_dir,
const StartupOptions* options,
const StartupOptions &options,
BlazeServerStartup** server_startup) {
const string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile);
const string daemonize = blaze_util::JoinPath(binaries_dir, "daemonize");
Expand Down
2 changes: 1 addition & 1 deletion src/main/cpp/blaze_util_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ int ExecuteDaemon(const string& exe,
const bool daemon_out_append,
const string& binaries_dir,
const string& server_dir,
const StartupOptions* options,
const StartupOptions &options,
BlazeServerStartup** server_startup) {
wstring wdaemon_output;
string error;
Expand Down
10 changes: 5 additions & 5 deletions src/main/cpp/startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
NULL) {
// TODO(bazel-team): Consider examining the javabase and re-execing in case
// of architecture mismatch.
server_javabase_ = blaze::AbsolutePathFromFlag(value);
explicit_server_javabase_ = blaze::AbsolutePathFromFlag(value);
option_sources["server_javabase"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--host_jvm_args")) !=
NULL) {
Expand Down Expand Up @@ -492,10 +492,10 @@ string StartupOptions::GetEmbeddedJavabase() const {
return "";
}

string StartupOptions::GetServerJavabase() {
string StartupOptions::GetServerJavabase() const {
// 1) Allow overriding the server_javabase via --server_javabase.
if (!server_javabase_.empty()) {
return server_javabase_;
if (!explicit_server_javabase_.empty()) {
return explicit_server_javabase_;
}
if (default_server_javabase_.empty()) {
string bundled_jre_path = GetEmbeddedJavabase();
Expand All @@ -517,7 +517,7 @@ string StartupOptions::GetServerJavabase() {
}

string StartupOptions::GetExplicitServerJavabase() const {
return server_javabase_;
return explicit_server_javabase_;
}

string StartupOptions::GetJvm() {
Expand Down
10 changes: 7 additions & 3 deletions src/main/cpp/startup_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class StartupOptions {

// Returns the GetHostJavabase. This should be called after parsing
// the --server_javabase option.
std::string GetServerJavabase();
std::string GetServerJavabase() const;

// Returns the explicit value of the --server_javabase startup option or the
// empty string if it was not specified on the command line.
Expand Down Expand Up @@ -331,8 +331,12 @@ class StartupOptions {
bool *is_space_separated,
std::string *error);

std::string server_javabase_;
std::string default_server_javabase_;
// The server javabase as provided on the commandline.
std::string explicit_server_javabase_;

// The server javabase to be used (computed lazily).
mutable std::string default_server_javabase_;

// Contains the collection of startup flags that Bazel accepts.
std::set<std::unique_ptr<StartupFlag>> valid_startup_flags;
};
Expand Down

0 comments on commit 12c7942

Please sign in to comment.