Bazel client: reduce dependency on <unistd.h>
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_<platform>
--
MOS_MIGRATED_REVID=139560397
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 53e3876..8f3151a 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 @@
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 @@
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 @@
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 @@
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 @@
"\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 @@
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) {
+ if (!blaze::WriteFile(data, size, path)) {
die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
- "\nError writing zipped file to %s", path.c_str());
- }
- if (close(fd) != 0) {
- 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 @@
"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 @@
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 @@
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 @@
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<grpc::ClientReader<command_server::RunResponse>> reader(
client_->Run(&context, request));
@@ -1662,7 +1623,7 @@
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 fe2bb72..6eebf72 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -170,39 +170,55 @@
}
// 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 @@
return false; // Can fail on NFS.
}
errno = saved_errno; // Caller should see errno from write().
- return static_cast<uint>(r) == content.size();
+ return static_cast<uint>(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 06e9c9b..eee40f9 100644
--- a/src/main/cpp/blaze_util.h
+++ b/src/main/cpp/blaze_util.h
@@ -46,17 +46,26 @@
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 6a818e5..6d68e73 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 4c9ce44..b70d95c 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -38,6 +38,10 @@
using std::string;
using std::vector;
+string GetProcessIdAsString() {
+ return ToString(getpid());
+}
+
void ExecuteProgram(const string &exe, const vector<string> &args_vector) {
if (VerboseLogging()) {
string dbg;
@@ -154,7 +158,7 @@
}
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 737c02a..2905673 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -95,6 +95,10 @@
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 5cddc3d..a807c0d 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -39,6 +39,10 @@
// 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 1cfc272..b6c37e1 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 <sys/stat.h>
#include <dirent.h> // DIR, dirent, opendir, closedir
+#include <fcntl.h> // O_RDONLY
#include <limits.h> // PATH_MAX
#include <stdlib.h> // getenv
-#include <unistd.h> // access
-#include <utime.h> // utime
+#include <sys/stat.h>
+#include <unistd.h> // access, open, close, fsync
+#include <utime.h> // utime
#include <vector>
@@ -80,6 +81,23 @@
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 @@
}
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 6110bb5..1465699 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -44,6 +44,10 @@
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");
