From ae16e763ce161af4c34f55e469723accbd0a9f03 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Fri, 18 Nov 2016 10:16:08 +0000 Subject: Bazel client: reduce dependency on In this change: - rename WriteFileToStreamOrDie to WriteFileToStderrOrDie (since we only ever used it for stderr) - replace open/write/read/close operations with blaze_util::ReadFile/WriteFile - wrap ToString(getpid()) in a utility function - move SyncFile to file_ -- MOS_MIGRATED_REVID=139560397 --- src/main/cpp/blaze.cc | 77 ++++++++++---------------------------- src/main/cpp/blaze_util.cc | 28 +++++++++++--- src/main/cpp/blaze_util.h | 13 ++++++- src/main/cpp/blaze_util_platform.h | 2 + src/main/cpp/blaze_util_posix.cc | 6 ++- src/main/cpp/blaze_util_windows.cc | 4 ++ src/main/cpp/util/file_platform.h | 4 ++ src/main/cpp/util/file_posix.cc | 26 +++++++++++-- src/main/cpp/util/file_windows.cc | 4 ++ 9 files changed, 93 insertions(+), 71 deletions(-) diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 53e38765a9..8f3151aa54 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -76,18 +76,12 @@ using blaze_util::die; using blaze_util::pdie; -// This should already be defined in sched.h, but it's not. -#ifndef SCHED_BATCH -#define SCHED_BATCH 3 -#endif - namespace blaze { using std::set; using std::string; using std::vector; -static void WriteFileToStreamOrDie(FILE *stream, const char *file_name); static int GetServerPid(const string &server_dir); static void VerifyJavaVersionAndSetJvm(); @@ -673,8 +667,7 @@ static void StartStandalone(BlazeServer* server) { pdie(blaze_exit_code::INTERNAL_ERROR, "execv of '%s' failed", exe.c_str()); } -// Write the contents of file_name to stream. -static void WriteFileToStreamOrDie(FILE *stream, const char *file_name) { +static void WriteFileToStderrOrDie(const char *file_name) { FILE *fp = fopen(file_name, "r"); if (fp == NULL) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, @@ -687,7 +680,7 @@ static void WriteFileToStreamOrDie(FILE *stream, const char *file_name) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "failed to read from '%s'", file_name); } - fwrite(buffer, 1, num_read, stream); + fwrite(buffer, 1, num_read, stderr); } fclose(fp); } @@ -705,21 +698,15 @@ static int GetServerPid(const string &server_dir) { string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile); string pid_symlink = blaze_util::JoinPath(server_dir, kServerPidSymlink); len = readlink(pid_symlink.c_str(), buf, sizeof(buf) - 1); - if (len < 0) { - int fd = open(pid_file.c_str(), O_RDONLY); - if (fd < 0) { - return -1; - } - len = read(fd, buf, 32); - close(fd); - if (len < 0) { - return -1; - } + string bufstr; + if (len > 0) { + bufstr = string(buf, len); + } else if (!blaze::ReadFile(pid_file, &bufstr, 32)) { + return -1; } int result; - buf[len] = 0; - if (!blaze_util::safe_strto32(string(buf), &result)) { + if (!blaze_util::safe_strto32(bufstr, &result)) { return -1; } @@ -786,7 +773,7 @@ static void StartServerAndConnect(BlazeServer *server) { fprintf(stderr, "\nunexpected pipe read status: %s\n" "Server presumed dead. Now printing '%s':\n", strerror(errno), globals->jvm_log_file.c_str()); - WriteFileToStreamOrDie(stderr, globals->jvm_log_file.c_str()); + WriteFileToStderrOrDie(globals->jvm_log_file.c_str()); exit(blaze_exit_code::INTERNAL_ERROR); } } @@ -794,24 +781,6 @@ static void StartServerAndConnect(BlazeServer *server) { "\nError: couldn't connect to server after 120 seconds."); } -// Calls fsync() on the file (or directory) specified in 'file_path'. -// pdie()'s if syncing fails. -static void SyncFile(const char *file_path) { - // fsync always fails on Cygwin with "Permission denied" for some reason. -#ifndef __CYGWIN__ - int fd = open(file_path, O_RDONLY); - if (fd < 0) { - pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "failed to open '%s' for syncing", file_path); - } - if (fsync(fd) < 0) { - pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "failed to sync '%s'", file_path); - } - close(fd); -#endif -} - // A devtools_ijar::ZipExtractorProcessor to extract the files from the blaze // zip. class ExtractBlazeZipProcessor : public devtools_ijar::ZipExtractorProcessor { @@ -830,19 +799,11 @@ class ExtractBlazeZipProcessor : public devtools_ijar::ZipExtractorProcessor { pdie(blaze_exit_code::INTERNAL_ERROR, "couldn't create '%s'", path.c_str()); } - int fd = open(path.c_str(), O_CREAT | O_WRONLY, 0755); - if (fd < 0) { - die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "\nFailed to open extraction file: %s", strerror(errno)); - } - if (write(fd, data, size) != size) { - die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "\nError writing zipped file to %s", path.c_str()); - } - if (close(fd) != 0) { + if (!blaze::WriteFile(data, size, path)) { die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "\nCould not close file %s", path.c_str()); + "\nFailed to write zipped file \"%s\": %s", path.c_str(), + strerror(errno)); } } @@ -902,7 +863,7 @@ static void ActuallyExtractData(const string &argv0, "failed to set timestamp on '%s'", extracted_path); } - SyncFile(extracted_path); + blaze_util::SyncFile(it); string directory = blaze_util::Dirname(extracted_path); @@ -916,13 +877,13 @@ static void ActuallyExtractData(const string &argv0, synced_directories.count(directory) == 0 && !directory.empty() && directory != "/") { - SyncFile(directory.c_str()); + blaze_util::SyncFile(directory); synced_directories.insert(directory); directory = blaze_util::Dirname(directory); } } - SyncFile(embedded_binaries.c_str()); + blaze_util::SyncFile(embedded_binaries); } // Installs Blaze by extracting the embedded data files, iff necessary. @@ -938,7 +899,7 @@ static void ExtractData(const string &self_path) { uint64_t st = GetMillisecondsMonotonic(); // Work in a temp dir to avoid races. string tmp_install = globals->options->install_base + ".tmp." + - ToString(getpid()); + blaze::GetProcessIdAsString(); string tmp_binaries = tmp_install + "/_embedded_binaries"; ActuallyExtractData(self_path, tmp_binaries); @@ -1630,8 +1591,8 @@ void GrpcBlazeServer::KillRunningServer() { command_server::RunResponse response; request.set_cookie(request_cookie_); request.set_block_for_lock(globals->options->block_for_lock); - request.set_client_description( - "pid=" + ToString(getpid()) + " (for shutdown)"); + request.set_client_description("pid=" + blaze::GetProcessIdAsString() + + " (for shutdown)"); request.add_arg("shutdown"); std::unique_ptr> reader( client_->Run(&context, request)); @@ -1662,7 +1623,7 @@ unsigned int GrpcBlazeServer::Communicate() { command_server::RunRequest request; request.set_cookie(request_cookie_); request.set_block_for_lock(globals->options->block_for_lock); - request.set_client_description("pid=" + ToString(getpid())); + request.set_client_description("pid=" + blaze::GetProcessIdAsString()); for (const string& arg : arg_vector) { request.add_arg(arg); } diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index fe2bb72931..6eebf72232 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -170,39 +170,55 @@ int MakeDirectories(const string& path, mode_t mode) { } // Replaces 'contents' with contents of 'fd' file descriptor. +// If `max_size` is positive, the method reads at most that many bytes; if it +// is 0, the method reads the whole file. // Returns false on error. -bool ReadFileDescriptor(int fd, string *content) { +bool ReadFileDescriptor(int fd, string *content, size_t max_size) { content->clear(); char buf[4096]; // OPT: This loop generates one spurious read on regular files. - while (int r = read(fd, buf, sizeof buf)) { + while (int r = read(fd, buf, max_size > 0 ? std::min(max_size, sizeof buf) + : sizeof buf)) { if (r == -1) { if (errno == EINTR || errno == EAGAIN) continue; return false; } content->append(buf, r); + if (max_size > 0) { + if (max_size > r) { + max_size -= r; + } else { + break; + } + } } close(fd); return true; } // Replaces 'content' with contents of file 'filename'. +// If `max_size` is positive, the method reads at most that many bytes; if it +// is 0, the method reads the whole file. // Returns false on error. -bool ReadFile(const string &filename, string *content) { +bool ReadFile(const string &filename, string *content, size_t max_size) { int fd = open(filename.c_str(), O_RDONLY); if (fd == -1) return false; - return ReadFileDescriptor(fd, content); + return ReadFileDescriptor(fd, content, max_size); } // Writes 'content' into file 'filename', and makes it executable. // Returns false on failure, sets errno. bool WriteFile(const string &content, const string &filename) { + return WriteFile(content.data(), content.size(), filename); +} + +bool WriteFile(const void *data, size_t size, const std::string &filename) { UnlinkPath(filename); // We don't care about the success of this. int fd = open(filename.c_str(), O_CREAT|O_WRONLY|O_TRUNC, 0755); // chmod +x if (fd == -1) { return false; } - int r = write(fd, content.data(), content.size()); + int r = write(fd, data, size); if (r == -1) { return false; } @@ -211,7 +227,7 @@ bool WriteFile(const string &content, const string &filename) { return false; // Can fail on NFS. } errno = saved_errno; // Caller should see errno from write(). - return static_cast(r) == content.size(); + return static_cast(r) == size; } bool UnlinkPath(const string &file_path) { diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 06e9c9b3a9..eee40f9d69 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -46,17 +46,26 @@ std::string MakeAbsolute(const std::string &path); int MakeDirectories(const std::string &path, mode_t mode); // Replaces 'content' with contents of file 'filename'. +// If `max_size` is positive, the method reads at most that many bytes; if it +// is 0, the method reads the whole file. // Returns false on error. Can be called from a signal handler. -bool ReadFile(const std::string &filename, std::string *content); +bool ReadFile(const std::string &filename, std::string *content, + size_t max_size = 0); // Replaces 'content' with contents of file descriptor 'fd'. +// If `max_size` is positive, the method reads at most that many bytes; if it +// is 0, the method reads the whole file. // Returns false on error. Can be called from a signal handler. -bool ReadFileDescriptor(int fd, std::string *content); +bool ReadFileDescriptor(int fd, std::string *content, size_t max_size = 0); // Writes 'content' into file 'filename', and makes it executable. // Returns false on failure, sets errno. bool WriteFile(const std::string &content, const std::string &filename); +// Writes `size` bytes from `data` into file 'filename' and makes it executable. +// Returns false on failure, sets errno. +bool WriteFile(const void* data, size_t size, const std::string &filename); + // Unlinks the file given by 'file_path'. // Returns true on success. In case of failure sets errno. bool UnlinkPath(const std::string &file_path); diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index 6a818e5e0c..6d68e73707 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -21,6 +21,8 @@ namespace blaze { +std::string GetProcessIdAsString(); + // Get the absolute path to the binary being executed. std::string GetSelfPath(); diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index 4c9ce44159..b70d95c21e 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -38,6 +38,10 @@ using blaze_util::pdie; using std::string; using std::vector; +string GetProcessIdAsString() { + return ToString(getpid()); +} + void ExecuteProgram(const string &exe, const vector &args_vector) { if (VerboseLogging()) { string dbg; @@ -154,7 +158,7 @@ void ExecuteDaemon(const string& exe, } Daemonize(daemon_output); - string pid_string = ToString(getpid()); + string pid_string = GetProcessIdAsString(); string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile); string pid_symlink_file = blaze_util::JoinPath(server_dir, kServerPidSymlink); diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index 737c02a30c..2905673b25 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -95,6 +95,10 @@ static void PrintError(const string& op) { void WarnFilesystemType(const string& output_base) { } +string GetProcessIdAsString() { + return ToString(GetCurrentProcessId()); +} + string GetSelfPath() { char buffer[PATH_MAX] = {}; if (!GetModuleFileName(0, buffer, sizeof(buffer))) { diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h index 5cddc3da07..a807c0dcb0 100644 --- a/src/main/cpp/util/file_platform.h +++ b/src/main/cpp/util/file_platform.h @@ -39,6 +39,10 @@ bool CanAccess(const std::string& path, bool read, bool write, bool exec); // Returns true if `path` refers to a directory or a symlink/junction to one. bool IsDirectory(const std::string& path); +// Calls fsync() on the file (or directory) specified in 'file_path'. +// pdie() if syncing fails. +void SyncFile(const std::string& path); + // Returns the last modification time of `path` in milliseconds since the Epoch. // Returns -1 upon failure. time_t GetMtimeMillisec(const std::string& path); diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index 1cfc272562..b6c37e19cc 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -13,12 +13,13 @@ // limitations under the License. #include "src/main/cpp/util/file_platform.h" -#include #include // DIR, dirent, opendir, closedir +#include // O_RDONLY #include // PATH_MAX #include // getenv -#include // access -#include // utime +#include +#include // access, open, close, fsync +#include // utime #include @@ -80,6 +81,23 @@ bool IsDirectory(const string& path) { return stat(path.c_str(), &buf) == 0 && S_ISDIR(buf.st_mode); } +void SyncFile(const string& path) { +// fsync always fails on Cygwin with "Permission denied" for some reason. +#ifndef __CYGWIN__ + const char* file_path = path.c_str(); + int fd = open(file_path, O_RDONLY); + if (fd < 0) { + pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, + "failed to open '%s' for syncing", file_path); + } + if (fsync(fd) < 0) { + pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "failed to sync '%s'", + file_path); + } + close(fd); +#endif +} + time_t GetMtimeMillisec(const string& path) { struct stat buf; if (stat(path.c_str(), &buf)) { @@ -90,7 +108,7 @@ time_t GetMtimeMillisec(const string& path) { } bool SetMtimeMillisec(const string& path, time_t mtime) { - struct utimbuf times = { mtime, mtime }; + struct utimbuf times = {mtime, mtime}; return utime(path.c_str(), ×) == 0; } diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 6110bb5652..1465699046 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -44,6 +44,10 @@ bool IsDirectory(const string& path) { return false; } +void SyncFile(const string& path) { + // No-op on Windows native; unsupported by Cygwin. +} + time_t GetMtimeMillisec(const string& path) { // TODO(bazel-team): implement this. pdie(255, "blaze_util::GetMtimeMillisec is not implemented on Windows"); -- cgit v1.2.3