From 242acd6c07ccfe380bddf8d5a719c1cdfd6fdfb9 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Tue, 27 Aug 2019 05:38:36 -0700 Subject: [PATCH] Client: convert output_base to Path Benefits of a Path abstraction instead of raw strings: - type safety - no need to convert paths on Windows all the time - no need to check if paths are absolute Closes #9245. Change-Id: I33438503e692c27a31975b5e496da11ebfe300f3 PiperOrigin-RevId: 265663938 --- src/main/cpp/bazel_startup_options.cc | 4 +- src/main/cpp/blaze.cc | 136 ++++++++++++++------------ src/main/cpp/blaze_util.cc | 2 +- src/main/cpp/blaze_util.h | 4 +- src/main/cpp/blaze_util_darwin.cc | 26 ++--- src/main/cpp/blaze_util_freebsd.cc | 14 +-- src/main/cpp/blaze_util_linux.cc | 17 ++-- src/main/cpp/blaze_util_platform.h | 28 +++--- src/main/cpp/blaze_util_posix.cc | 76 +++++++------- src/main/cpp/blaze_util_windows.cc | 117 +++++++++------------- src/main/cpp/server_process_info.cc | 14 +-- src/main/cpp/server_process_info.h | 8 +- src/main/cpp/startup_options.cc | 22 +++-- src/main/cpp/startup_options.h | 5 +- src/main/cpp/util/file_platform.h | 5 +- src/main/cpp/util/file_posix.cc | 12 ++- src/main/cpp/util/file_windows.cc | 70 ++++--------- src/main/cpp/util/path_platform.h | 10 +- src/main/cpp/util/path_posix.cc | 10 ++ src/main/cpp/util/path_windows.cc | 14 +++ 20 files changed, 308 insertions(+), 286 deletions(-) diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc index 4866fcea488aac..3d702e3891a85c 100644 --- a/src/main/cpp/bazel_startup_options.cc +++ b/src/main/cpp/bazel_startup_options.cc @@ -87,13 +87,13 @@ void BazelStartupOptions::MaybeLogStartupOptionWarnings() const { << "Output user root \"" << output_user_root << "\" contains a space. This will probably break the build. " "You should set a different --output_user_root."; - } else if (output_base.find_first_of(' ') != std::string::npos) { + } else if (output_base.Contains(' ')) { // output_base is computed from output_user_root by default. // If output_user_root was bad, don't check output_base: while output_base // may also be bad, we already warned about output_user_root so there's no // point in another warning. BAZEL_LOG(WARNING) - << "Output base \"" << output_base + << "Output base \"" << output_base.AsPrintablePath() << "\" contains a space. This will probably break the build. " "You should not set --output_base and let Bazel use the default, or " "set --output_base to a path without space."; diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index fbaabecb9a4ccb..a514ef8b5f9327 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -237,12 +237,9 @@ struct LoggingInfo { class BlazeServer final { public: - BlazeServer( - const int connect_timeout_secs, - const bool batch, - const bool block_for_lock, - const string &output_base, - const string &server_jvm_out); + BlazeServer(const int connect_timeout_secs, const bool batch, + const bool block_for_lock, const blaze_util::Path &output_base, + const blaze_util::Path &server_jvm_out); // Acquire a lock for the server running in this output base. Returns the // number of milliseconds spent waiting for the lock. @@ -309,7 +306,7 @@ class BlazeServer final { const int connect_timeout_secs_; const bool batch_; const bool block_for_lock_; - const string output_base_; + const blaze_util::Path output_base_; }; //////////////////////////////////////////////////////////////////////// @@ -370,7 +367,7 @@ static vector GetServerExeArgs( result.push_back("-XX:+HeapDumpOnOutOfMemoryError"); result.push_back("-XX:HeapDumpPath=" + - blaze_util::PathAsJvmFlag(startup_options.output_base)); + startup_options.output_base.AsJvmArgument()); // TODO(b/109998449): only assume JDK >= 9 for embedded JDKs if (!startup_options.GetEmbeddedJavabase().empty()) { @@ -470,13 +467,14 @@ static vector GetServerExeArgs( blaze_util::ConvertPath(startup_options.install_base)); result.push_back("--install_md5=" + install_md5); result.push_back("--output_base=" + - blaze_util::ConvertPath(startup_options.output_base)); + startup_options.output_base.AsCommandLineArgument()); result.push_back("--workspace_directory=" + blaze_util::ConvertPath(workspace)); result.push_back("--default_system_javabase=" + GetSystemJavabase()); - if (!startup_options.server_jvm_out.empty()) { - result.push_back("--server_jvm_out=" + startup_options.server_jvm_out); + if (!startup_options.server_jvm_out.IsEmpty()) { + result.push_back("--server_jvm_out=" + + startup_options.server_jvm_out.AsCommandLineArgument()); } if (startup_options.deep_execroot) { @@ -742,18 +740,24 @@ static void RunBatchMode( } } -static void WriteFileToStderrOrDie(const char *file_name) { - FILE *fp = fopen(file_name, "r"); +static void WriteFileToStderrOrDie(const blaze_util::Path &path) { +#if defined(_WIN32) || defined(__CYGWIN__) + FILE *fp = _wfopen(path.AsNativePath().c_str(), L"r"); +#else + FILE *fp = fopen(path.AsNativePath().c_str(), "r"); +#endif + if (fp == NULL) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "opening " << file_name << " failed: " << GetLastErrorString(); + << "opening " << path.AsPrintablePath() + << " failed: " << GetLastErrorString(); } char buffer[255]; int num_read; while ((num_read = fread(buffer, 1, sizeof buffer, fp)) > 0) { if (ferror(fp)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "failed to read from '" << file_name + << "failed to read from '" << path.AsPrintablePath() << "': " << GetLastErrorString(); } fwrite(buffer, 1, num_read, stderr); @@ -816,12 +820,14 @@ static void ConnectOrDie( if (server->ProcessInfo().jvm_log_file_append_) { // Don't dump the log if we were appending - the user should know where // to find it, and who knows how much content they may have accumulated. - BAZEL_LOG(USER) << "Server crashed during startup. See " - << server->ProcessInfo().jvm_log_file_; + BAZEL_LOG(USER) + << "Server crashed during startup. See " + << server->ProcessInfo().jvm_log_file_.AsPrintablePath(); } else { - BAZEL_LOG(USER) << "Server crashed during startup. Now printing " - << server->ProcessInfo().jvm_log_file_; - WriteFileToStderrOrDie(server->ProcessInfo().jvm_log_file_.c_str()); + BAZEL_LOG(USER) + << "Server crashed during startup. Now printing " + << server->ProcessInfo().jvm_log_file_.AsPrintablePath(); + WriteFileToStderrOrDie(server->ProcessInfo().jvm_log_file_); } exit(blaze_exit_code::INTERNAL_ERROR); } @@ -1126,8 +1132,8 @@ static void KillRunningServerIfDifferentStartupOptions( return; } - string cmdline_path = - blaze_util::JoinPath(startup_options.output_base, "server/cmdline"); + blaze_util::Path cmdline_path = + startup_options.output_base.GetRelative("server/cmdline"); string old_joined_arguments; // No, /proc/$PID/cmdline does not work, because it is limited to 4K. Even @@ -1162,8 +1168,8 @@ static void EnsureCorrectRunningVersion( // target dirs don't match, or if the symlink was not present, then kill any // running servers. Lastly, symlink to our installation so others know which // installation is running. - const string installation_path = - blaze_util::JoinPath(startup_options.output_base, "install"); + const blaze_util::Path installation_path = + startup_options.output_base.GetRelative("install"); string prev_installation; bool ok = blaze_util::ReadDirectorySymlink(installation_path, &prev_installation); @@ -1180,9 +1186,10 @@ static void EnsureCorrectRunningVersion( blaze_util::UnlinkPath(installation_path); if (!SymlinkDirectories(startup_options.install_base, installation_path)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "failed to create installation symlink '" << installation_path - << "': " << GetLastErrorString(); + << "failed to create installation symlink '" + << installation_path.AsPrintablePath() << "': " << err; } // Update the mtime of the install base so that cleanup tools can @@ -1310,37 +1317,42 @@ static void UpdateConfiguration( install_md5); } - if (startup_options->output_base.empty()) { - startup_options->output_base = blaze::GetHashedBaseDir( - startup_options->output_user_root, workspace); + if (startup_options->output_base.IsEmpty()) { + startup_options->output_base = blaze_util::Path( + blaze::GetHashedBaseDir(startup_options->output_user_root, workspace)); } - const char *output_base = startup_options->output_base.c_str(); if (!blaze_util::PathExists(startup_options->output_base)) { if (!blaze_util::MakeDirectories(startup_options->output_base, 0777)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "Output base directory '" << output_base - << "' could not be created: " << GetLastErrorString(); + << "Output base directory '" + << startup_options->output_base.AsPrintablePath() + << "' could not be created: " << err; } } else { if (!blaze_util::IsDirectory(startup_options->output_base)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "Output base directory '" << output_base + << "Output base directory '" + << startup_options->output_base.AsPrintablePath() << "' could not be created. It exists but is not a directory."; } } if (!blaze_util::CanAccessDirectory(startup_options->output_base)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "Output base directory '" << output_base + << "Output base directory '" + << startup_options->output_base.AsPrintablePath() << "' must be readable and writable."; } - ExcludePathFromBackup(output_base); + ExcludePathFromBackup(startup_options->output_base); - startup_options->output_base = blaze_util::MakeCanonical(output_base); - if (startup_options->output_base.empty()) { + startup_options->output_base = startup_options->output_base.Canonicalize(); + if (startup_options->output_base.IsEmpty()) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "blaze_util::MakeCanonical('" << output_base - << "') failed: " << GetLastErrorString(); + << "blaze_util::MakeCanonical('" + << startup_options->output_base.AsPrintablePath() + << "') failed: " << err; } } @@ -1420,10 +1432,10 @@ static string CheckAndGetBinaryPath(const string &cwd, const string &argv0) { } } -static int GetExitCodeForAbruptExit(const string &output_base) { +static int GetExitCodeForAbruptExit(const blaze_util::Path &output_base) { BAZEL_LOG(INFO) << "Looking for a custom exit-code."; - std::string filename = blaze_util::JoinPath( - output_base, "exit_code_to_use_on_abrupt_exit"); + blaze_util::Path filename = + output_base.GetRelative("exit_code_to_use_on_abrupt_exit"); std::string content; if (!blaze_util::ReadFile(filename, &content)) { BAZEL_LOG(INFO) << "Unable to read the custom exit-code file. " @@ -1572,7 +1584,8 @@ int Main(int argc, const char *argv[], WorkspaceLayout *workspace_layout, UnlimitCoredumps(); } - blaze::CreateSecureOutputRoot(startup_options->output_user_root); + blaze::CreateSecureOutputRoot( + blaze_util::Path(startup_options->output_user_root)); // Only start a server when in a workspace because otherwise we won't do more // than emit a help message. @@ -1610,18 +1623,16 @@ static bool ProtoStringEqual(const StringTypeA &cookieA, return memcmp(cookieA.c_str(), cookieB.c_str(), cookie_length) == 0; } -BlazeServer::BlazeServer( - const int connect_timeout_secs, - const bool batch, - const bool block_for_lock, - const string &output_base, - const string &server_jvm_out) - : connected_(false), - process_info_(output_base, server_jvm_out), - connect_timeout_secs_(connect_timeout_secs), - batch_(batch), - block_for_lock_(block_for_lock), - output_base_(output_base) { +BlazeServer::BlazeServer(const int connect_timeout_secs, const bool batch, + const bool block_for_lock, + const blaze_util::Path &output_base, + const blaze_util::Path &server_jvm_out) + : connected_(false), + process_info_(output_base, server_jvm_out), + connect_timeout_secs_(connect_timeout_secs), + batch_(batch), + block_for_lock_(block_for_lock), + output_base_(output_base) { gpr_set_log_function(null_grpc_log_function); pipe_.reset(blaze_util::CreatePipe()); @@ -1658,14 +1669,13 @@ bool BlazeServer::TryConnect( bool BlazeServer::Connect() { assert(!connected_); - std::string server_dir = blaze_util::JoinPath(output_base_, "server"); + blaze_util::Path server_dir = output_base_.GetRelative("server"); std::string port; std::string ipv4_prefix = "127.0.0.1:"; std::string ipv6_prefix_1 = "[0:0:0:0:0:0:0:1]:"; std::string ipv6_prefix_2 = "[::1]:"; - if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "command_port"), - &port)) { + if (!blaze_util::ReadFile(server_dir.GetRelative("command_port"), &port)) { return false; } @@ -1676,12 +1686,12 @@ bool BlazeServer::Connect() { return false; } - if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "request_cookie"), + if (!blaze_util::ReadFile(server_dir.GetRelative("request_cookie"), &request_cookie_)) { return false; } - if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "response_cookie"), + if (!blaze_util::ReadFile(server_dir.GetRelative("response_cookie"), &response_cookie_)) { return false; } @@ -1864,7 +1874,7 @@ void BlazeServer::KillRunningServer() { << "Shutdown request failed, server still alive: (error code: " << status.error_code() << ", error message: '" << status.error_message() << "', log file: '" - << process_info_.jvm_log_file_ << "')"; + << process_info_.jvm_log_file_.AsPrintablePath() << "')"; } KillServerProcess(process_info_.server_pid_, output_base_); } @@ -2002,12 +2012,12 @@ unsigned int BlazeServer::Communicate( BAZEL_LOG(USER) << "\nServer terminated abruptly (error code: " << status.error_code() << ", error message: '" << status.error_message() << "', log file: '" - << process_info_.jvm_log_file_ << "')\n"; + << process_info_.jvm_log_file_.AsPrintablePath() << "')\n"; return GetExitCodeForAbruptExit(output_base_); } else if (!finished) { BAZEL_LOG(USER) << "\nServer finished RPC without an explicit exit code (log file: '" - << process_info_.jvm_log_file_ << "')\n"; + << process_info_.jvm_log_file_.AsPrintablePath() << "')\n"; return GetExitCodeForAbruptExit(output_base_); } else if (final_response.has_exec_request()) { const command_server::ExecRequest& request = final_response.exec_request(); diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index ca25bba43ca6da..2c3f7a208cb681 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -175,7 +175,7 @@ void LogWait(unsigned int elapsed_seconds, unsigned int wait_seconds) { elapsed_seconds, wait_seconds); } -bool AwaitServerProcessTermination(int pid, const string& output_base, +bool AwaitServerProcessTermination(int pid, const blaze_util::Path& output_base, unsigned int wait_seconds) { uint64_t st = GetMillisecondsMonotonic(); const unsigned int first_seconds = 5; diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 123ead96e4c125..f91ee50d90575d 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -26,6 +26,8 @@ #include #include +#include "src/main/cpp/util/path.h" + namespace blaze { extern const char kServerPidFile[]; @@ -74,7 +76,7 @@ std::string AbsolutePathFromFlag(const std::string& value); // wait_seconds elapses or the server process terminates. Returns true if a // check sees that the server process terminated. Logs to stderr after 5, 10, // and 30 seconds if the wait lasts that long. -bool AwaitServerProcessTermination(int pid, const std::string& output_base, +bool AwaitServerProcessTermination(int pid, const blaze_util::Path& output_base, unsigned int wait_seconds); // The number of seconds the client will wait for the server process to diff --git a/src/main/cpp/blaze_util_darwin.cc b/src/main/cpp/blaze_util_darwin.cc index fbeb0a16daf1d5..bcb3bcddde0086 100644 --- a/src/main/cpp/blaze_util_darwin.cc +++ b/src/main/cpp/blaze_util_darwin.cc @@ -99,11 +99,12 @@ string GetOutputRoot() { return "/var/tmp"; } -void WarnFilesystemType(const string& output_base) { +void WarnFilesystemType(const blaze_util::Path &output_base) { // Check to see if we are on a non-local drive. CFScopedReleaser cf_url(CFURLCreateFromFileSystemRepresentation( - kCFAllocatorDefault, reinterpret_cast(output_base.c_str()), - output_base.length(), true)); + kCFAllocatorDefault, + reinterpret_cast(output_base.AsNativePath().c_str()), + output_base.AsNativePath().length(), true)); CFBooleanRef cf_local = NULL; CFErrorRef cf_error = NULL; if (!cf_url.isValid() || @@ -111,13 +112,13 @@ void WarnFilesystemType(const string& output_base) { &cf_local, &cf_error)) { CFScopedReleaser cf_error_releaser(cf_error); BAZEL_LOG(WARNING) << "couldn't get file system type information for '" - << output_base + << output_base.AsPrintablePath() << "': " << DescriptionFromCFError(cf_error_releaser); return; } CFScopedReleaser cf_local_releaser(cf_local); if (!CFBooleanGetValue(cf_local_releaser)) { - BAZEL_LOG(WARNING) << "Output base '" << output_base + BAZEL_LOG(WARNING) << "Output base '" << output_base.AsPrintablePath() << "' is on a non-local drive. This may lead to " "surprising failures and undetermined behavior."; } @@ -204,7 +205,7 @@ int ConfigureDaemonProcess(posix_spawnattr_t *attrp, void WriteSystemSpecificProcessIdentifier(const blaze_util::Path &server_dir, pid_t server_pid) {} -bool VerifyServerProcess(int pid, const string &output_base) { +bool VerifyServerProcess(int pid, const blaze_util::Path &output_base) { // TODO(lberki): This only checks for the process's existence, not whether // its start time matches. Therefore this might accidentally kill an // unrelated process if the server died and the PID got reused. @@ -213,19 +214,22 @@ bool VerifyServerProcess(int pid, const string &output_base) { // Sets a flag on path to exclude the path from Apple's automatic backup service // (Time Machine) -void ExcludePathFromBackup(const string &path) { +void ExcludePathFromBackup(const blaze_util::Path &path) { CFScopedReleaser cf_url(CFURLCreateFromFileSystemRepresentation( - kCFAllocatorDefault, reinterpret_cast(path.c_str()), - path.length(), true)); + kCFAllocatorDefault, + reinterpret_cast(path.AsNativePath().c_str()), + path.AsNativePath().length(), true)); if (!cf_url.isValid()) { - BAZEL_LOG(WARNING) << "unable to exclude '" << path << "' from backups"; + BAZEL_LOG(WARNING) << "unable to exclude '" << path.AsPrintablePath() + << "' from backups"; return; } CFErrorRef cf_error = NULL; if (!CFURLSetResourcePropertyForKey(cf_url, kCFURLIsExcludedFromBackupKey, kCFBooleanTrue, &cf_error)) { CFScopedReleaser cf_error_releaser(cf_error); - BAZEL_LOG(WARNING) << "unable to exclude '" << path << "' from backups: " + BAZEL_LOG(WARNING) << "unable to exclude '" << path.AsPrintablePath() + << "' from backups: " << DescriptionFromCFError(cf_error_releaser); return; } diff --git a/src/main/cpp/blaze_util_freebsd.cc b/src/main/cpp/blaze_util_freebsd.cc index b731d38217218c..067dfae9bacc0b 100644 --- a/src/main/cpp/blaze_util_freebsd.cc +++ b/src/main/cpp/blaze_util_freebsd.cc @@ -56,16 +56,17 @@ string GetOutputRoot() { } } -void WarnFilesystemType(const string &output_base) { +void WarnFilesystemType(const blaze_util::Path &output_base) { struct statfs buf = {}; - if (statfs(output_base.c_str(), &buf) < 0) { + if (statfs(output_base.AsNativePath().c_str(), &buf) < 0) { BAZEL_LOG(WARNING) << "couldn't get file system type information for '" - << output_base << "': " << strerror(errno); + << output_base.AsPrintablePath() + << "': " << strerror(errno); return; } if (strcmp(buf.f_fstypename, "nfs") == 0) { - BAZEL_LOG(WARNING) << "Output base '" << output_base + BAZEL_LOG(WARNING) << "Output base '" << output_base.AsPrintablePath() << "' is on NFS. This may lead to surprising failures " "and undetermined behavior."; } @@ -164,7 +165,7 @@ int ConfigureDaemonProcess(posix_spawnattr_t *attrp, void WriteSystemSpecificProcessIdentifier(const blaze_util::Path &server_dir, pid_t server_pid) {} -bool VerifyServerProcess(int pid, const string &output_base) { +bool VerifyServerProcess(int pid, const blaze_util::Path &output_base) { // TODO(lberki): This only checks for the process's existence, not whether // its start time matches. Therefore this might accidentally kill an // unrelated process if the server died and the PID got reused. @@ -172,8 +173,7 @@ bool VerifyServerProcess(int pid, const string &output_base) { } // Not supported. -void ExcludePathFromBackup(const string &path) { -} +void ExcludePathFromBackup(const blaze_util::Path &path) {} int32_t GetExplicitSystemLimit(const int resource) { return -1; diff --git a/src/main/cpp/blaze_util_linux.cc b/src/main/cpp/blaze_util_linux.cc index b23898e2c7b70f..91d65ead0f2856 100644 --- a/src/main/cpp/blaze_util_linux.cc +++ b/src/main/cpp/blaze_util_linux.cc @@ -66,16 +66,17 @@ string GetOutputRoot() { return "/tmp"; } -void WarnFilesystemType(const string& output_base) { +void WarnFilesystemType(const blaze_util::Path &output_base) { struct statfs buf = {}; - if (statfs(output_base.c_str(), &buf) < 0) { + if (statfs(output_base.AsNativePath().c_str(), &buf) < 0) { BAZEL_LOG(WARNING) << "couldn't get file system type information for '" - << output_base << "': " << strerror(errno); + << output_base.AsPrintablePath() + << "': " << strerror(errno); return; } if (buf.f_type == NFS_SUPER_MAGIC) { - BAZEL_LOG(WARNING) << "Output base '" << output_base + BAZEL_LOG(WARNING) << "Output base '" << output_base.AsPrintablePath() << "' is on NFS. This may lead to surprising failures " "and undetermined behavior."; } @@ -232,7 +233,7 @@ void WriteSystemSpecificProcessIdentifier(const blaze_util::Path &server_dir, // On Linux we use a combination of PID and start time to identify the server // process. That is supposed to be unique unless one can start more processes // than there are PIDs available within a single jiffy. -bool VerifyServerProcess(int pid, const string& output_base) { +bool VerifyServerProcess(int pid, const blaze_util::Path &output_base) { string start_time; if (!GetStartTime(ToString(pid), &start_time)) { // Cannot read PID file from /proc . Process died meantime, all is good. No @@ -242,8 +243,7 @@ bool VerifyServerProcess(int pid, const string& output_base) { string recorded_start_time; bool file_present = blaze_util::ReadFile( - blaze_util::JoinPath(output_base, "server/server.starttime"), - &recorded_start_time); + output_base.GetRelative("server/server.starttime"), &recorded_start_time); // If start time file got deleted, but PID file didn't, assume that this is an // old Blaze process that doesn't know how to write start time files yet. @@ -251,8 +251,7 @@ bool VerifyServerProcess(int pid, const string& output_base) { } // Not supported. -void ExcludePathFromBackup(const string &path) { -} +void ExcludePathFromBackup(const blaze_util::Path &path) {} int32_t GetExplicitSystemLimit(const int resource) { return -1; diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index d00130677610c2..1474bce785d9f0 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -82,18 +82,19 @@ class SignalHandler { return server_process_info_; } const std::string& GetProductName() const { return product_name_; } - const std::string& GetOutputBase() const { return output_base_; } + const blaze_util::Path& GetOutputBase() const { return output_base_; } void CancelServer() { cancel_server_(); } - void Install( - const std::string &product_name, const std::string &output_base, - const ServerProcessInfo* server_process_info, Callback cancel_server); + void Install(const std::string& product_name, + const blaze_util::Path& output_base, + const ServerProcessInfo* server_process_info, + Callback cancel_server); ATTRIBUTE_NORETURN void PropagateSignalOrExit(int exit_code); private: static SignalHandler INSTANCE; std::string product_name_; - std::string output_base_; + blaze_util::Path output_base_; const ServerProcessInfo* server_process_info_; Callback cancel_server_; @@ -117,7 +118,7 @@ std::string GetOutputRoot(); std::string GetHomeDir(); // Warn about dubious filesystem types, such as NFS, case-insensitive (?). -void WarnFilesystemType(const std::string& output_base); +void WarnFilesystemType(const blaze_util::Path& output_base); // Returns elapsed milliseconds since some unspecified start of time. // The results are monotonic, i.e. subsequent calls to this method never return @@ -175,7 +176,7 @@ class BlazeServerStartup { int ExecuteDaemon( const std::string& exe, const std::vector& args_vector, const std::map& env, - const std::string& daemon_output, const bool daemon_output_append, + const blaze_util::Path& daemon_output, const bool daemon_output_append, const std::string& binaries_dir, const blaze_util::Path& server_dir, const StartupOptions& options, BlazeServerStartup** server_startup); @@ -185,7 +186,8 @@ extern const char kListSeparator; // Create a symlink to directory ``target`` at location ``link``. // Returns true on success, false on failure. The target must be absolute. // Implemented via junctions on Windows. -bool SymlinkDirectories(const std::string& target, const std::string& link); +bool SymlinkDirectories(const std::string& target, + const blaze_util::Path& link); struct BlazeLock { #if defined(_WIN32) || defined(__CYGWIN__) @@ -198,7 +200,7 @@ struct BlazeLock { // Acquires a lock on the output base. Exits if the lock cannot be acquired. // Sets ``lock`` to a value that can subsequently be passed to ReleaseLock(). // Returns the number of milliseconds spent with waiting for the lock. -uint64_t AcquireLock(const std::string& output_base, bool batch_mode, +uint64_t AcquireLock(const blaze_util::Path& output_base, bool batch_mode, bool block, BlazeLock* blaze_lock); // Releases the lock on the output base. In case of an error, continues as @@ -206,12 +208,12 @@ uint64_t AcquireLock(const std::string& output_base, bool batch_mode, void ReleaseLock(BlazeLock* blaze_lock); // Verifies whether the server process still exists. Returns true if it does. -bool VerifyServerProcess(int pid, const std::string& output_base); +bool VerifyServerProcess(int pid, const blaze_util::Path& output_base); // Kills a server process based on its PID. // Returns true if the server process was found and killed. // WARNING! This function can be called from a signal handler! -bool KillServerProcess(int pid, const std::string& output_base); +bool KillServerProcess(int pid, const blaze_util::Path& output_base); // Wait for approximately the specified number of milliseconds. The actual // amount of time waited may be more or less because of interrupts or system @@ -219,7 +221,7 @@ bool KillServerProcess(int pid, const std::string& output_base); void TrySleep(unsigned int milliseconds); // Mark path as being excluded from backups (if supported by operating system). -void ExcludePathFromBackup(const std::string& path); +void ExcludePathFromBackup(const blaze_util::Path& path); // Returns the canonical form of the base dir given a root and a hashable // string. The resulting dir is composed of the root + md5(hashable) @@ -229,7 +231,7 @@ std::string GetHashedBaseDir(const std::string& root, // Create a safe installation directory where we keep state, installations etc. // This method ensures that the directory is created, is owned by the current // user, and not accessible to anyone else. -void CreateSecureOutputRoot(const std::string& path); +void CreateSecureOutputRoot(const blaze_util::Path& path); std::string GetEnv(const std::string& name); diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index 23c2c8cdf61fb1..3efbbdfc23ec60 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -163,10 +163,12 @@ static void handler(int signum) { signal_handler_received_signal = SIGPIPE; break; case SIGQUIT: - SigPrintf( - "\nSending SIGQUIT to JVM process %d (see %s).\n\n", - SignalHandler::Get().GetServerProcessInfo()->server_pid_, - SignalHandler::Get().GetServerProcessInfo()->jvm_log_file_.c_str()); + SigPrintf("\nSending SIGQUIT to JVM process %d (see %s).\n\n", + SignalHandler::Get().GetServerProcessInfo()->server_pid_, + SignalHandler::Get() + .GetServerProcessInfo() + ->jvm_log_file_.AsNativePath() + .c_str()); kill(SignalHandler::Get().GetServerProcessInfo()->server_pid_, SIGQUIT); break; } @@ -174,9 +176,9 @@ static void handler(int signum) { errno = saved_errno; } -void SignalHandler::Install(const string &product_name, - const string &output_base, - const ServerProcessInfo *server_process_info, +void SignalHandler::Install(const string& product_name, + const blaze_util::Path& output_base, + const ServerProcessInfo* server_process_info, SignalHandler::Callback cancel_server) { product_name_ = product_name; output_base_ = output_base; @@ -302,8 +304,8 @@ void ExecuteRunRequest(const string& exe, const char kListSeparator = ':'; -bool SymlinkDirectories(const string &target, const string &link) { - return symlink(target.c_str(), link.c_str()) == 0; +bool SymlinkDirectories(const string& target, const blaze_util::Path& link) { + return symlink(target.c_str(), link.AsNativePath().c_str()) == 0; } // Notifies the client about the death of the server process by keeping a socket @@ -357,15 +359,16 @@ void WriteSystemSpecificProcessIdentifier(const blaze_util::Path& server_dir, int ExecuteDaemon(const string& exe, const std::vector& args_vector, const std::map& env, - const string& daemon_output, const bool daemon_output_append, - const string& binaries_dir, + const blaze_util::Path& daemon_output, + const bool daemon_output_append, const string& binaries_dir, const blaze_util::Path& server_dir, const StartupOptions& options, BlazeServerStartup** server_startup) { const blaze_util::Path pid_file = server_dir.GetRelative(kServerPidFile); const string daemonize = blaze_util::JoinPath(binaries_dir, "daemonize"); - std::vector daemonize_args = {"daemonize", "-l", daemon_output, "-p", + std::vector daemonize_args = {"daemonize", "-l", + daemon_output.AsNativePath(), "-p", pid_file.AsNativePath()}; if (daemon_output_append) { daemonize_args.push_back("-a"); @@ -448,48 +451,51 @@ string GetHashedBaseDir(const string& root, const string& hashable) { return blaze_util::JoinPath(root, digest.String()); } -void CreateSecureOutputRoot(const string& path) { - const char* root = path.c_str(); +void CreateSecureOutputRoot(const blaze_util::Path& path) { struct stat fileinfo = {}; - if (!blaze_util::MakeDirectories(root, 0755)) { + if (!blaze_util::MakeDirectories(path, 0755)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "mkdir('" << root << "'): " << GetLastErrorString(); + << "mkdir('" << path.AsPrintablePath() << "'): " << err; } // The path already exists. // Check ownership and mode, and verify that it is a directory. - if (lstat(root, &fileinfo) < 0) { + if (lstat(path.AsNativePath().c_str(), &fileinfo) < 0) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "lstat('" << root << "'): " << GetLastErrorString(); + << "lstat('" << path.AsPrintablePath() << "'): " << err; } if (fileinfo.st_uid != geteuid()) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "'" << root << "' is not owned by me"; + << "'" << path.AsPrintablePath() << "' is not owned by me"; } if ((fileinfo.st_mode & 022) != 0) { int new_mode = fileinfo.st_mode & (~022); - if (chmod(root, new_mode) < 0) { + if (chmod(path.AsNativePath().c_str(), new_mode) < 0) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "'" << root << "' has mode " << (fileinfo.st_mode & 07777) - << ", chmod to " << new_mode << " failed"; + << "'" << path.AsPrintablePath() << "' has mode " + << (fileinfo.st_mode & 07777) << ", chmod to " << new_mode + << " failed"; } } - if (stat(root, &fileinfo) < 0) { + if (stat(path.AsNativePath().c_str(), &fileinfo) < 0) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "stat('" << root << "'): " << GetLastErrorString(); + << "stat('" << path.AsPrintablePath() << "'): " << err; } if (!S_ISDIR(fileinfo.st_mode)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "'" << root << "' is not a directory"; + << "'" << path.AsPrintablePath() << "' is not a directory"; } - ExcludePathFromBackup(root); + ExcludePathFromBackup(path); } string GetEnv(const string& name) { @@ -583,22 +589,24 @@ static int setlk(int fd, struct flock *lock) { return -1; } -uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block, - BlazeLock* blaze_lock) { - string lockfile = blaze_util::JoinPath(output_base, "lock"); - int lockfd = open(lockfile.c_str(), O_CREAT|O_RDWR, 0644); +uint64_t AcquireLock(const blaze_util::Path& output_base, bool batch_mode, + bool block, BlazeLock* blaze_lock) { + blaze_util::Path lockfile = output_base.GetRelative("lock"); + int lockfd = open(lockfile.AsNativePath().c_str(), O_CREAT | O_RDWR, 0644); if (lockfd < 0) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "cannot open lockfile '" << lockfile - << "' for writing: " << GetLastErrorString(); + << "cannot open lockfile '" << lockfile.AsPrintablePath() + << "' for writing: " << err; } // Keep server from inheriting a useless fd if we are not in batch mode if (!batch_mode) { + string err = GetLastErrorString(); if (fcntl(lockfd, F_SETFD, FD_CLOEXEC) == -1) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "fcntl(F_SETFD) failed for lockfile: " << GetLastErrorString(); + << "fcntl(F_SETFD) failed for lockfile: " << err; } } @@ -680,7 +688,7 @@ void ReleaseLock(BlazeLock* blaze_lock) { close(blaze_lock->lockfd); } -bool KillServerProcess(int pid, const string& output_base) { +bool KillServerProcess(int pid, const blaze_util::Path& output_base) { // Kill the process and make sure it's dead before proceeding. killpg(pid, SIGKILL); if (!AwaitServerProcessTermination(pid, output_base, diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index dae48552739ffe..df6c4c557cbd80 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -317,9 +317,9 @@ BOOL WINAPI ConsoleCtrlHandler(_In_ DWORD ctrlType) { return false; } -void SignalHandler::Install(const string &product_name, - const string &output_base, - const ServerProcessInfo *server_process_info_, +void SignalHandler::Install(const string& product_name, + const blaze_util::Path& output_base, + const ServerProcessInfo* server_process_info_, SignalHandler::Callback cancel_server) { product_name_ = product_name; output_base_ = output_base; @@ -377,8 +377,7 @@ static void PrintErrorW(const wstring& op) { LocalFree(message_buffer); } -void WarnFilesystemType(const string& output_base) { -} +void WarnFilesystemType(const blaze_util::Path& output_base) {} string GetProcessIdAsString() { return ToString(GetCurrentProcessId()); @@ -555,7 +554,8 @@ static void WriteProcessStartupTime(const blaze_util::Path& server_dir, } } -static HANDLE CreateJvmOutputFile(const wstring& path, LPSECURITY_ATTRIBUTES sa, +static HANDLE CreateJvmOutputFile(const blaze_util::Path& path, + LPSECURITY_ATTRIBUTES sa, bool daemon_out_append) { // If the previous server process was asked to be shut down (but not killed), // it takes a while for it to comply, so wait until the JVM output file that @@ -564,18 +564,19 @@ static HANDLE CreateJvmOutputFile(const wstring& path, LPSECURITY_ATTRIBUTES sa, static const unsigned int timeout_sec = 60; for (unsigned int waited = 0; waited < timeout_sec; ++waited) { HANDLE handle = ::CreateFileW( - /* lpFileName */ path.c_str(), + /* lpFileName */ path.AsNativePath().c_str(), /* dwDesiredAccess */ GENERIC_READ | GENERIC_WRITE, /* dwShareMode */ FILE_SHARE_READ, /* lpSecurityAttributes */ sa, /* dwCreationDisposition */ - daemon_out_append ? OPEN_ALWAYS : CREATE_ALWAYS, + daemon_out_append ? OPEN_ALWAYS : CREATE_ALWAYS, /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL, /* hTemplateFile */ NULL); if (handle != INVALID_HANDLE_VALUE) { if (daemon_out_append && !SetFilePointerEx(handle, {0}, NULL, FILE_END)) { - fprintf(stderr, "Could not seek to end of file (%ls)\n", path.c_str()); + fprintf(stderr, "Could not seek to end of file (%s)\n", + path.AsPrintablePath().c_str()); return INVALID_HANDLE_VALUE; } return handle; @@ -616,20 +617,11 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup { int ExecuteDaemon(const string& exe, const std::vector& args_vector, const std::map& env, - const string& daemon_output, const bool daemon_out_append, - const string& binaries_dir, + const blaze_util::Path& daemon_output, + const bool daemon_out_append, const string& binaries_dir, const blaze_util::Path& server_dir, const StartupOptions& options, BlazeServerStartup** server_startup) { - wstring wdaemon_output; - string error; - if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output, - &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "ExecuteDaemon(" << exe << "): AsAbsoluteWindowsPath(" - << daemon_output << ") failed: " << error; - } - SECURITY_ATTRIBUTES inheritable_handle_sa = {sizeof(SECURITY_ATTRIBUTES), NULL, TRUE}; @@ -637,18 +629,18 @@ int ExecuteDaemon(const string& exe, const std::vector& args_vector, L"NUL", GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, &inheritable_handle_sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)); if (!devnull.IsValid()) { - error = GetLastErrorString(); + std::string error = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "ExecuteDaemon(" << exe << "): CreateFileA(NUL) failed: " << error; } AutoHandle stdout_file(CreateJvmOutputFile( - wdaemon_output.c_str(), &inheritable_handle_sa, daemon_out_append)); + daemon_output, &inheritable_handle_sa, daemon_out_append)); if (!stdout_file.IsValid()) { - error = GetLastErrorString(); + std::string error = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "ExecuteDaemon(" << exe << "): CreateJvmOutputFile(" - << blaze_util::WstringToString(wdaemon_output) << ") failed: " << error; + << daemon_output.AsPrintablePath() << ") failed: " << error; } HANDLE stderr_handle; // We must duplicate the handle to stdout, otherwise "bazel clean --expunge" @@ -663,10 +655,10 @@ int ExecuteDaemon(const string& exe, const std::vector& args_vector, /* dwDesiredAccess */ 0, /* bInheritHandle */ TRUE, /* dwOptions */ DUPLICATE_SAME_ACCESS)) { + std::string error = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "ExecuteDaemon(" << exe << "): DuplicateHandle(" - << blaze_util::WstringToString(wdaemon_output) - << ") failed: " << GetLastErrorString(); + << daemon_output.AsPrintablePath() << ") failed: " << error; } AutoHandle stderr_file(stderr_handle); @@ -799,33 +791,29 @@ void ExecuteRunRequest(const string& exe, const char kListSeparator = ';'; -bool SymlinkDirectories(const string &posix_target, const string &posix_name) { - wstring name; +bool SymlinkDirectories(const string& posix_target, + const blaze_util::Path& name) { wstring target; string error; - if (!blaze_util::AsAbsoluteWindowsPath(posix_name, &name, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "SymlinkDirectories(" << posix_target << ", " << posix_name - << "): AsAbsoluteWindowsPath(" << posix_target << ") failed: " << error; - return false; - } if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target, &error)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "SymlinkDirectories(" << posix_target << ", " << posix_name - << "): AsAbsoluteWindowsPath(" << posix_name << ") failed: " << error; + << "SymlinkDirectories(" << posix_target << ", " + << name.AsPrintablePath() << "): AsAbsoluteWindowsPath(" << posix_target + << ") failed: " << error; return false; } wstring werror; - if (CreateJunction(name, target, &werror) != CreateJunctionResult::kSuccess) { + if (CreateJunction(name.AsNativePath(), target, &werror) != + CreateJunctionResult::kSuccess) { string error(blaze_util::WstringToCstring(werror.c_str()).get()); BAZEL_LOG(ERROR) << "SymlinkDirectories(" << posix_target << ", " - << posix_name << "): CreateJunction: " << error; + << name.AsPrintablePath() + << "): CreateJunction: " << error; return false; } return true; } - #ifndef STILL_ACTIVE #define STILL_ACTIVE (259) // From MSDN about GetExitCodeProcess. #endif @@ -833,7 +821,7 @@ bool SymlinkDirectories(const string &posix_target, const string &posix_name) { // On Windows (and Linux) we use a combination of PID and start time to identify // the server process. That is supposed to be unique unless one can start more // processes than there are PIDs available within a single jiffy. -bool VerifyServerProcess(int pid, const string& output_base) { +bool VerifyServerProcess(int pid, const blaze_util::Path& output_base) { AutoHandle process( ::OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid)); if (!process.IsValid()) { @@ -852,15 +840,14 @@ bool VerifyServerProcess(int pid, const string& output_base) { string recorded_start_time; bool file_present = blaze_util::ReadFile( - blaze_util::JoinPath(output_base, "server/server.starttime"), - &recorded_start_time); + output_base.GetRelative("server/server.starttime"), &recorded_start_time); // If start time file got deleted, but PID file didn't, assume that this is an // old Bazel process that doesn't know how to write start time files yet. return !file_present || recorded_start_time == ToString(start_time); } -bool KillServerProcess(int pid, const string& output_base) { +bool KillServerProcess(int pid, const blaze_util::Path& output_base) { AutoHandle process(::OpenProcess( PROCESS_TERMINATE | PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid)); DWORD exitcode = 0; @@ -874,9 +861,10 @@ bool KillServerProcess(int pid, const string& output_base) { BOOL result = TerminateProcess(process, /*uExitCode*/ 0); if (!result || !AwaitServerProcessTermination(pid, output_base, kPostKillGracePeriodSeconds)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "Cannot terminate server process with PID " << pid - << ", output_base=(" << output_base << "): " << GetLastErrorString(); + << ", output_base=(" << output_base.AsPrintablePath() << "): " << err; } return result; } @@ -886,8 +874,7 @@ void TrySleep(unsigned int milliseconds) { } // Not supported. -void ExcludePathFromBackup(const string &path) { -} +void ExcludePathFromBackup(const blaze_util::Path& path) {} string GetHashedBaseDir(const string& root, const string& hashable) { // Builds a shorter output base dir name for Windows. @@ -920,21 +907,21 @@ string GetHashedBaseDir(const string& root, const string& hashable) { return blaze_util::JoinPath(root, string(coded_name)); } -void CreateSecureOutputRoot(const string& path) { +void CreateSecureOutputRoot(const blaze_util::Path& path) { // TODO(bazel-team): implement this properly, by mimicing whatever the POSIX // implementation does. - const char* root = path.c_str(); if (!blaze_util::MakeDirectories(path, 0755)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "MakeDirectories(" << root << ") failed: " << GetLastErrorString(); + << "MakeDirectories(" << path.AsPrintablePath() << ") failed: " << err; } if (!blaze_util::IsDirectory(path)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "'" << root << "' is not a directory"; + << "'" << path.AsPrintablePath() << "' is not a directory"; } - ExcludePathFromBackup(root); + ExcludePathFromBackup(path); } string GetEnv(const string& name) { @@ -1086,23 +1073,15 @@ uint64_t WindowsClock::GetMilliseconds() const { return GetMillisecondsAsLargeInt(kFrequency).QuadPart; } -uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block, - BlazeLock* blaze_lock) { - string lockfile = blaze_util::JoinPath(output_base, "lock"); - wstring wlockfile; - string error; - if (!blaze_util::AsAbsoluteWindowsPath(lockfile, &wlockfile, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "AcquireLock(" << output_base << "): AsAbsoluteWindowsPath(" - << lockfile << ") failed: " << error; - } - +uint64_t AcquireLock(const blaze_util::Path& output_base, bool batch_mode, + bool block, BlazeLock* blaze_lock) { + blaze_util::Path lockfile = output_base.GetRelative("lock"); blaze_lock->handle = INVALID_HANDLE_VALUE; bool first_lock_attempt = true; uint64_t st = GetMillisecondsMonotonic(); while (true) { blaze_lock->handle = ::CreateFileW( - /* lpFileName */ wlockfile.c_str(), + /* lpFileName */ lockfile.AsNativePath().c_str(), /* dwDesiredAccess */ GENERIC_READ | GENERIC_WRITE, /* dwShareMode */ FILE_SHARE_READ, /* lpSecurityAttributes */ NULL, @@ -1128,10 +1107,10 @@ uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block, } Sleep(/* dwMilliseconds */ 200); } else { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "AcquireLock(" << lockfile << "): CreateFileW(" - << blaze_util::WstringToString(wlockfile) - << ") failed: " << GetLastErrorString(); + << "AcquireLock(" << lockfile.AsPrintablePath() + << "): CreateFile failed: " << err; } } uint64_t wait_time = GetMillisecondsMonotonic() - st; @@ -1145,10 +1124,10 @@ uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block, /* nNumberOfBytesToLockLow */ 1, /* nNumberOfBytesToLockHigh */ 0, /* lpOverlapped */ &overlapped)) { + string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "AcquireLock(" << lockfile << "): LockFileEx(" - << blaze_util::WstringToString(wlockfile) - << ") failed: " << GetLastErrorString(); + << "AcquireLock(" << lockfile.AsPrintablePath() + << "): LockFileEx failed: " << err; } // On other platforms we write some info about this process into the lock file // such as the server PID. On Windows we don't do that because the file is diff --git a/src/main/cpp/server_process_info.cc b/src/main/cpp/server_process_info.cc index ed8fa412fb9abd..c854e589ac5e12 100644 --- a/src/main/cpp/server_process_info.cc +++ b/src/main/cpp/server_process_info.cc @@ -18,19 +18,19 @@ namespace blaze { -static std::string GetJvmOutFile( - const std::string &output_base, const std::string &server_jvm_out) { - if (!server_jvm_out.empty()) { +static blaze_util::Path GetJvmOutFile(const blaze_util::Path &output_base, + const blaze_util::Path &server_jvm_out) { + if (!server_jvm_out.IsEmpty()) { return server_jvm_out; } else { - return blaze_util::JoinPath(output_base, "server/jvm.out"); + return output_base.GetRelative("server/jvm.out"); } } -ServerProcessInfo::ServerProcessInfo( - const std::string &output_base, const std::string &server_jvm_out) +ServerProcessInfo::ServerProcessInfo(const blaze_util::Path &output_base, + const blaze_util::Path &server_jvm_out) : jvm_log_file_(GetJvmOutFile(output_base, server_jvm_out)), - jvm_log_file_append_(!server_jvm_out.empty()), + jvm_log_file_append_(!server_jvm_out.IsEmpty()), // TODO(b/69972303): Formalize the "no server" magic value or rm it. server_pid_(-1) {} diff --git a/src/main/cpp/server_process_info.h b/src/main/cpp/server_process_info.h index 1a1813aba51276..9fa69de9eeda4e 100644 --- a/src/main/cpp/server_process_info.h +++ b/src/main/cpp/server_process_info.h @@ -16,9 +16,11 @@ #define BAZEL_SRC_MAIN_CPP_SERVER_PROCESS_INFO_H_ #include + #include #include +#include "src/main/cpp/util/path.h" #include "src/main/cpp/util/port.h" // pid_t on Windows/MSVC namespace blaze { @@ -27,12 +29,12 @@ namespace blaze { // configuration. class ServerProcessInfo final { public: - ServerProcessInfo( - const std::string &output_base, const std::string &server_jvm_out); + ServerProcessInfo(const blaze_util::Path &output_base, + const blaze_util::Path &server_jvm_out); // When running as a daemon, where the deamonized server's stdout and stderr // should be written. - const std::string jvm_log_file_; + const blaze_util::Path jvm_log_file_; // Whether or not the jvm_log_file should be opened with O_APPEND. const bool jvm_log_file_append_; diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index 884feee7c83b8a..ce7dfd78539ec0 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -252,7 +252,7 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( } if ((value = GetUnaryOption(arg, next_arg, "--output_base")) != NULL) { - output_base = blaze::AbsolutePathFromFlag(value); + output_base = blaze_util::Path(blaze::AbsolutePathFromFlag(value)); option_sources["output_base"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--install_base")) != NULL) { @@ -264,7 +264,7 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( option_sources["output_user_root"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--server_jvm_out")) != NULL) { - server_jvm_out = blaze::AbsolutePathFromFlag(value); + server_jvm_out = blaze_util::Path(blaze::AbsolutePathFromFlag(value)); option_sources["server_jvm_out"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--host_jvm_profile")) != NULL) { @@ -594,12 +594,13 @@ blaze_exit_code::ExitCode StartupOptions::AddJVMArguments( } static std::string GetSimpleLogHandlerProps( - const std::string &java_log, const std::string &java_logging_formatter) { + const blaze_util::Path &java_log, + const std::string &java_logging_formatter) { return "handlers=com.google.devtools.build.lib.util.SimpleLogHandler\n" ".level=INFO\n" "com.google.devtools.build.lib.util.SimpleLogHandler.level=INFO\n" "com.google.devtools.build.lib.util.SimpleLogHandler.prefix=" + - java_log + + java_log.AsJvmArgument() + "\n" "com.google.devtools.build.lib.util.SimpleLogHandler.limit=1024000\n" "com.google.devtools.build.lib.util.SimpleLogHandler.total_limit=" @@ -610,17 +611,18 @@ static std::string GetSimpleLogHandlerProps( void StartupOptions::AddJVMLoggingArguments(std::vector *result) const { // Configure logging - const string propFile = blaze_util::PathAsJvmFlag( - blaze_util::JoinPath(output_base, "javalog.properties")); - const string java_log( - blaze_util::PathAsJvmFlag(blaze_util::JoinPath(output_base, "java.log"))); + const blaze_util::Path propFile = + output_base.GetRelative("javalog.properties"); + const blaze_util::Path java_log = output_base.GetRelative("java.log"); const std::string loggingProps = GetSimpleLogHandlerProps(java_log, java_logging_formatter); if (!blaze_util::WriteFile(loggingProps, propFile)) { - perror(("Couldn't write logging file " + propFile).c_str()); + perror( + ("Couldn't write logging file " + propFile.AsPrintablePath()).c_str()); } else { - result->push_back("-Djava.util.logging.config.file=" + propFile); + result->push_back("-Djava.util.logging.config.file=" + + propFile.AsJvmArgument()); result->push_back( "-Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=" "com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier"); diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 1af639e31b4268..36fd0005e90720 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -27,6 +27,7 @@ #include #include "src/main/cpp/util/exit_code.h" +#include "src/main/cpp/util/path.h" namespace blaze { @@ -120,11 +121,11 @@ class StartupOptions { // If supplied, alternate location to write the blaze server's jvm's stdout. // Otherwise a default path in the output base is used. - std::string server_jvm_out; + blaze_util::Path server_jvm_out; // Blaze's output base. Everything is relative to this. See // the BlazeDirectories Java class for details. - std::string output_base; + blaze_util::Path output_base; // Installation base for a specific release installation. std::string install_base; diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h index 25dc856f9d013a..bdf8d07a9c0bf8 100644 --- a/src/main/cpp/util/file_platform.h +++ b/src/main/cpp/util/file_platform.h @@ -143,7 +143,7 @@ int RenameDirectory(const std::string &old_name, const std::string &new_name); // Reads which directory a symlink points to. Puts the target of the symlink // in ``result`` and returns if the operation was successful. Will not work on // symlinks that don't point to directories on Windows. -bool ReadDirectorySymlink(const std::string &symlink, std::string *result); +bool ReadDirectorySymlink(const blaze_util::Path &symlink, std::string *result); // Unlinks the file given by 'file_path'. // Returns true on success. In case of failure sets errno. @@ -152,6 +152,7 @@ bool UnlinkPath(const Path &file_path); // Returns true if this path exists, following symlinks. bool PathExists(const std::string& path); +bool PathExists(const Path &path); // Returns the real, absolute path corresponding to `path`. // The method resolves all symlink components of `path`. @@ -172,9 +173,11 @@ bool CanExecuteFile(const std::string &path); // is both readable and writable. // Follows symlinks/junctions. bool CanAccessDirectory(const std::string &path); +bool CanAccessDirectory(const Path &path); // Returns true if `path` refers to a directory or a symlink/junction to one. bool IsDirectory(const std::string& path); +bool IsDirectory(const Path &path); // Calls fsync() on the file (or directory) specified in 'file_path'. // pdie() if syncing fails. diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index 3133345f64846c..4340aa7fbc6f44 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -261,9 +261,9 @@ int RenameDirectory(const std::string &old_name, const std::string &new_name) { } } -bool ReadDirectorySymlink(const string &name, string *result) { +bool ReadDirectorySymlink(const blaze_util::Path &name, string *result) { char buf[PATH_MAX + 1]; - int len = readlink(name.c_str(), buf, PATH_MAX); + int len = readlink(name.AsNativePath().c_str(), buf, PATH_MAX); if (len < 0) { return false; } @@ -285,6 +285,8 @@ bool PathExists(const string& path) { return access(path.c_str(), F_OK) == 0; } +bool PathExists(const Path &path) { return PathExists(path.AsNativePath()); } + string MakeCanonical(const char *path) { char *resolved_path = realpath(path, NULL); if (resolved_path == NULL) { @@ -322,11 +324,17 @@ bool CanAccessDirectory(const std::string &path) { return IsDirectory(path) && CanAccess(path, true, true, true); } +bool CanAccessDirectory(const Path &path) { + return CanAccessDirectory(path.AsNativePath()); +} + bool IsDirectory(const string& path) { struct stat buf; return stat(path.c_str(), &buf) == 0 && S_ISDIR(buf.st_mode); } +bool IsDirectory(const Path &path) { return IsDirectory(path.AsNativePath()); } + void SyncFile(const string& path) { const char* file_path = path.c_str(); int fd = open(file_path, O_RDONLY); diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 9a387bda94ceb3..a2fc9e2a16bac2 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -458,39 +458,25 @@ static bool RealPath(const WCHAR* path, unique_ptr* result = nullptr) { } } -bool ReadDirectorySymlink(const string& name, string* result) { - wstring wname; - string error; - if (!AsAbsoluteWindowsPath(name, &wname, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "ReadDirectorySymlink(" << name - << "): AsAbsoluteWindowsPath failed: " << error; - return false; - } +bool ReadDirectorySymlink(const blaze_util::Path& name, string* result) { unique_ptr result_ptr; - if (!RealPath(wname.c_str(), &result_ptr)) { + if (!RealPath(name.AsNativePath().c_str(), &result_ptr)) { return false; } *result = WstringToCstring(RemoveUncPrefixMaybe(result_ptr.get())).get(); return true; } -bool PathExists(const string& path) { - if (path.empty()) { +bool PathExists(const string& path) { return PathExists(Path(path)); } + +bool PathExists(const Path& path) { + if (path.IsEmpty()) { return false; } - if (IsDevNull(path.c_str())) { + if (path.IsNull()) { return true; } - wstring wpath; - string error; - if (!AsAbsoluteWindowsPath(path, &wpath, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "PathExists(" << path - << "): AsAbsoluteWindowsPath failed: " << error; - return false; - } - return RealPath(wpath.c_str(), nullptr); + return RealPath(path.AsNativePath().c_str(), nullptr); } string MakeCanonical(const char* path) { @@ -561,15 +547,11 @@ bool CanExecuteFile(const std::string& path) { } bool CanAccessDirectory(const std::string& path) { - wstring wpath; - string error; - if (!AsAbsoluteWindowsPath(path, &wpath, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "CanAccessDirectory(" << path - << "): AsAbsoluteWindowsPath failed: " << error; - return false; - } - DWORD attr = ::GetFileAttributesW(wpath.c_str()); + return CanAccessDirectory(Path(path)); +} + +bool CanAccessDirectory(const Path& path) { + DWORD attr = ::GetFileAttributesW(path.AsNativePath().c_str()); if ((attr == INVALID_FILE_ATTRIBUTES) || !(attr & FILE_ATTRIBUTE_DIRECTORY)) { // The path doesn't exist or is not a directory. return false; @@ -577,17 +559,13 @@ bool CanAccessDirectory(const std::string& path) { // The only easy way to know if a directory is writable is by attempting to // open a file for writing in it. - wstring dummy_path = wpath + L"\\bazel_directory_access_test"; - - // The path may have just became too long for MAX_PATH, so add the UNC prefix - // if necessary. - AddUncPrefixMaybe(&dummy_path); + Path dummy_path = path.GetRelative("bazel_directory_access_test"); // Attempt to open the dummy file for read/write access. // If the file happens to exist, no big deal, we won't overwrite it thanks to // OPEN_ALWAYS. HANDLE handle = ::CreateFileW( - /* lpFileName */ dummy_path.c_str(), + /* lpFileName */ dummy_path.AsNativePath().c_str(), /* dwDesiredAccess */ GENERIC_WRITE | GENERIC_READ, /* dwShareMode */ kAllShare, /* lpSecurityAttributes */ NULL, @@ -606,7 +584,7 @@ bool CanAccessDirectory(const std::string& path) { if (err != ERROR_ALREADY_EXISTS) { // The file didn't exist before, but due to OPEN_ALWAYS we created it just // now, so do delete it. - ::DeleteFileW(dummy_path.c_str()); + ::DeleteFileW(dummy_path.AsNativePath().c_str()); } // Otherwise the file existed before, leave it alone. return true; } @@ -623,19 +601,13 @@ bool IsDirectoryW(const wstring& path) { (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); } -bool IsDirectory(const string& path) { - if (path.empty() || IsDevNull(path.c_str())) { - return false; - } - wstring wpath; - string error; - if (!AsAbsoluteWindowsPath(path, &wpath, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "IsDirectory(" << path - << "): AsAbsoluteWindowsPath failed: " << error; +bool IsDirectory(const string& path) { return IsDirectory(Path(path)); } + +bool IsDirectory(const Path& path) { + if (path.IsEmpty() || path.IsNull()) { return false; } - return IsDirectoryW(wpath); + return IsDirectoryW(path.AsNativePath()); } void SyncFile(const string& path) { diff --git a/src/main/cpp/util/path_platform.h b/src/main/cpp/util/path_platform.h index 27656f71843843..d1637a74643a11 100644 --- a/src/main/cpp/util/path_platform.h +++ b/src/main/cpp/util/path_platform.h @@ -19,19 +19,25 @@ namespace blaze_util { // Platform-native, absolute, normalized path. +// It can be converted to a printable path (for error messages) or to a native +// path (for API calls). class Path { public: Path() {} explicit Path(const std::string &path); bool IsEmpty() const { return path_.empty(); } bool IsNull() const; + bool Contains(const char c) const; Path GetRelative(const std::string &r) const; + Path Canonicalize() const; std::string AsPrintablePath() const; + std::string AsJvmArgument() const; + std::string AsCommandLineArgument() const; #if defined(_WIN32) || defined(__CYGWIN__) - const std::wstring &AsNativePath() const { return path_; } + const std::wstring AsNativePath() const { return path_; } #else - const std::string &AsNativePath() const { return path_; } + const std::string AsNativePath() const { return path_; } #endif private: diff --git a/src/main/cpp/util/path_posix.cc b/src/main/cpp/util/path_posix.cc index eed7f640d04ee2..720a74b8a15cda 100644 --- a/src/main/cpp/util/path_posix.cc +++ b/src/main/cpp/util/path_posix.cc @@ -139,10 +139,20 @@ Path::Path(const std::string &path) bool Path::IsNull() const { return path_ == "/dev/null"; } +bool Path::Contains(const char c) const { + return path_.find_first_of(c) != std::string::npos; +} + Path Path::GetRelative(const std::string &r) const { return Path(JoinPath(path_, r)); } +Path Path::Canonicalize() const { return Path(MakeCanonical(path_.c_str())); } + std::string Path::AsPrintablePath() const { return path_; } +std::string Path::AsJvmArgument() const { return path_; } + +std::string Path::AsCommandLineArgument() const { return path_; } + } // namespace blaze_util diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc index 8621dff4fe2560..464ccf9a08099d 100644 --- a/src/main/cpp/util/path_windows.cc +++ b/src/main/cpp/util/path_windows.cc @@ -490,10 +490,24 @@ Path Path::GetRelative(const std::string& r) const { } } +Path Path::Canonicalize() const { + return Path(MakeCanonical(WstringToString(path_).c_str())); +} + bool Path::IsNull() const { return path_ == L"NUL"; } +bool Path::Contains(const char c) const { + return path_.find_first_of(c) != std::wstring::npos; +} + std::string Path::AsPrintablePath() const { return WstringToCstring(RemoveUncPrefixMaybe(path_.c_str())).get(); } +std::string Path::AsJvmArgument() const { + return PathAsJvmFlag(AsPrintablePath()); +} + +std::string Path::AsCommandLineArgument() const { return AsPrintablePath(); } + } // namespace blaze_util