Bazel client: generalize path handling
Use/implement utility methods to join paths, check
if they are the root directory or are absolute,
etc. Doing so (instead of say checking if a path
starts with "/") allows for correct behavior on
Windows.
See https://github.com/bazelbuild/bazel/issues/2107
--
PiperOrigin-RevId: 142446027
MOS_MIGRATED_REVID=142446027
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index a80ba26..89ab712 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -316,7 +316,7 @@
die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
"\nFailed to find install_base_key's in zip file");
}
- return root + "/" + globals->install_md5;
+ return blaze_util::JoinPath(root, globals->install_md5);
}
// Escapes colons by replacing them with '_C' and underscores by replacing them
@@ -606,13 +606,15 @@
BlazeServerStartup** server_startup) {
vector<string> jvm_args_vector = GetArgumentArray();
string argument_string = GetArgumentString(jvm_args_vector);
- string server_dir = globals->options->output_base + "/server";
+ string server_dir =
+ blaze_util::JoinPath(globals->options->output_base, "server");
// Write the cmdline argument string to the server dir. If we get to this
// point, there is no server running, so we don't overwrite the cmdline file
// for the existing server. If might be that the server dies and the cmdline
// file stays there, but that is not a problem, since we always check the
// server, too.
- blaze_util::WriteFile(argument_string, server_dir + "/cmdline");
+ blaze_util::WriteFile(argument_string,
+ blaze_util::JoinPath(server_dir, "cmdline"));
// unless we restarted for a new-version, mark this as initial start
if (globals->restart_reason == NO_RESTART) {
@@ -717,7 +719,8 @@
// Starts up a new server and connects to it. Exits if it didn't work not.
static void StartServerAndConnect(const WorkspaceLayout* workspace_layout,
BlazeServer *server) {
- string server_dir = globals->options->output_base + "/server";
+ string server_dir =
+ blaze_util::JoinPath(globals->options->output_base, "server");
// The server dir has the socket, so we don't allow access by other
// users.
@@ -880,14 +883,13 @@
// Now walk up until embedded_binaries and sync every directory in between.
// synced_directories is used to avoid syncing the same directory twice.
- // The !directory.empty() and directory != "/" conditions are not strictly
- // needed, but it makes this loop more robust, because otherwise, if due to
- // some glitch, directory was not under embedded_binaries, it would get
- // into an infinite loop.
+ // The !directory.empty() and !blaze_util::IsRootDirectory(directory)
+ // conditions are not strictly needed, but it makes this loop more robust,
+ // because otherwise, if due to some glitch, directory was not under
+ // embedded_binaries, it would get into an infinite loop.
while (directory != embedded_binaries &&
- synced_directories.count(directory) == 0 &&
- !directory.empty() &&
- directory != "/") {
+ synced_directories.count(directory) == 0 && !directory.empty() &&
+ !blaze_util::IsRootDirectory(directory)) {
blaze_util::SyncFile(directory);
synced_directories.insert(directory);
directory = blaze_util::Dirname(directory);
@@ -911,7 +913,8 @@
// Work in a temp dir to avoid races.
string tmp_install = globals->options->install_base + ".tmp." +
blaze::GetProcessIdAsString();
- string tmp_binaries = tmp_install + "/_embedded_binaries";
+ string tmp_binaries =
+ blaze_util::JoinPath(tmp_install, "_embedded_binaries");
ActuallyExtractData(self_path, tmp_binaries);
uint64_t et = GetMillisecondsMonotonic();
@@ -1017,7 +1020,8 @@
return;
}
- string cmdline_path = globals->options->output_base + "/server/cmdline";
+ string cmdline_path =
+ blaze_util::JoinPath(globals->options->output_base, "server/cmdline");
string joined_arguments;
// No, /proc/$PID/cmdline does not work, because it is limited to 4K. Even
@@ -1050,7 +1054,8 @@
// 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.
- string installation_path = globals->options->output_base + "/install";
+ string installation_path =
+ blaze_util::JoinPath(globals->options->output_base, "install");
string prev_installation;
bool ok = ReadDirectorySymlink(installation_path, &prev_installation);
if (!ok || !CompareAbsolutePaths(
@@ -1170,7 +1175,8 @@
// but if an install_base is specified on the command line, we use that as
// the base instead.
if (globals->options->install_base.empty()) {
- string install_user_root = globals->options->output_user_root + "/install";
+ string install_user_root =
+ blaze_util::JoinPath(globals->options->output_user_root, "install");
globals->options->install_base =
GetInstallBase(install_user_root, self_path);
} else {
@@ -1212,8 +1218,10 @@
"blaze_util::MakeCanonical('%s') failed", output_base);
}
- globals->lockfile = globals->options->output_base + "/lock";
- globals->jvm_log_file = globals->options->output_base + "/server/jvm.out";
+ globals->lockfile =
+ blaze_util::JoinPath(globals->options->output_base, "lock");
+ globals->jvm_log_file =
+ blaze_util::JoinPath(globals->options->output_base, "server/jvm.out");
}
static void CheckEnvironment() {
@@ -1260,10 +1268,10 @@
}
static string CheckAndGetBinaryPath(const string& argv0) {
- if (argv0[0] == '/') {
+ if (blaze_util::IsAbsolute(argv0)) {
return argv0;
} else {
- string abs_path = globals->cwd + '/' + argv0;
+ string abs_path = blaze_util::JoinPath(globals->cwd, argv0);
string resolved_path = blaze_util::MakeCanonical(abs_path.c_str());
if (!resolved_path.empty()) {
return resolved_path;
@@ -1374,13 +1382,15 @@
bool GrpcBlazeServer::Connect() {
assert(!connected_);
- std::string server_dir = globals->options->output_base + "/server";
+ std::string server_dir =
+ blaze_util::JoinPath(globals->options->output_base, "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(server_dir + "/command_port", &port)) {
+ if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "command_port"),
+ &port)) {
return false;
}
@@ -1391,11 +1401,12 @@
return false;
}
- if (!blaze_util::ReadFile(server_dir + "/request_cookie", &request_cookie_)) {
+ if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "request_cookie"),
+ &request_cookie_)) {
return false;
}
- if (!blaze_util::ReadFile(server_dir + "/response_cookie",
+ if (!blaze_util::ReadFile(blaze_util::JoinPath(server_dir, "response_cookie"),
&response_cookie_)) {
return false;
}
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index 99b2d20..30034e4 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -43,15 +43,13 @@
string MakeAbsolute(const string &path) {
// Check if path is already absolute.
- if (path.empty() || path[0] == '/' || (isalpha(path[0]) && path[1] == ':')) {
+ // TODO(laszlocsomor): remove the "path.empty() ||" clause; empty paths are
+ // not absolute!
+ if (path.empty() || blaze_util::IsAbsolute(path)) {
return path;
}
- string cwd = blaze_util::GetCwd();
-
- // Determine whether the cwd ends with "/" or not.
- string separator = cwd.back() == '/' ? "" : "/";
- return cwd + separator + path;
+ return blaze_util::JoinPath(blaze_util::GetCwd(), path);
}
const char* GetUnaryOption(const char *arg,
diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h
index 873d005..9fd0338 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -56,6 +56,9 @@
// Returns the directory Bazel can use to store output.
std::string GetOutputRoot();
+// Returns the location of the global bazelrc file if it exists, otherwise "".
+std::string FindSystemWideBlazerc();
+
// Warn about dubious filesystem types, such as NFS, case-insensitive (?).
void WarnFilesystemType(const std::string& output_base);
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index a90c602..9be1c4e 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -125,6 +125,14 @@
return ToString(getpid());
}
+string FindSystemWideBlazerc() {
+ string path = "/etc/bazel.bazelrc";
+ if (blaze_util::CanAccess(path, true, false, false)) {
+ return path;
+ }
+ return "";
+}
+
void ExecuteProgram(const string &exe, const vector<string> &args_vector) {
if (VerboseLogging()) {
string dbg;
@@ -323,7 +331,7 @@
blaze_util::Md5Digest digest;
digest.Update(hashable.data(), hashable.size());
digest.Finish(buf);
- return root + "/" + digest.String();
+ return blaze_util::JoinPath(root, digest.String());
}
void CreateSecureOutputRoot(const string& path) {
@@ -424,7 +432,7 @@
uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block,
BlazeLock* blaze_lock) {
- string lockfile = output_base + "/lock";
+ string lockfile = blaze_util::JoinPath(output_base, "lock");
int lockfd = open(lockfile.c_str(), O_CREAT|O_RDWR, 0644);
if (lockfd < 0) {
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index d74bd8d..fbabf07 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -300,6 +300,20 @@
#endif // COMPILER_MSVC
}
+string FindSystemWideBlazerc() {
+#ifdef COMPILER_MSVC
+ // TODO(bazel-team): implement this.
+ pdie(255, "FindSystemWideBlazer is not yet implemented on Windows");
+ return "";
+#else // not COMPILER_MSVC
+ string path = "/etc/bazel.bazelrc";
+ if (blaze_util::CanAccess(path, true, false, false)) {
+ return path;
+ }
+ return "";
+#endif // COMPILER_MSVC
+}
+
uint64_t GetMillisecondsMonotonic() {
return WindowsClock::INSTANCE.GetMilliseconds();
}
@@ -1050,7 +1064,7 @@
coded_name[i] = alphabet[buf[i] & 0x3F];
}
coded_name[filename_length] = '\0';
- return root + "/" + string(coded_name);
+ return blaze_util::JoinPath(root, string(coded_name));
}
void CreateSecureOutputRoot(const string& path) {
@@ -1217,7 +1231,7 @@
pdie(255, "blaze::AcquireLock is not implemented on Windows");
return 0;
#else // not COMPILER_MSVC
- string lockfile = output_base + "/lock";
+ string lockfile = blaze_util::JoinPath(output_base, "lock");
int lockfd = open(lockfile.c_str(), O_CREAT|O_RDWR, 0644);
if (lockfd < 0) {
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index ea92cff..c69cf0b 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -354,7 +354,7 @@
}
string StartupOptions::GetJvm() {
- string java_program = GetHostJavabase() + "/bin/java";
+ string java_program = blaze_util::JoinPath(GetHostJavabase(), "bin/java");
if (!blaze_util::CanAccess(java_program, false, false, true)) {
if (!blaze_util::PathExists(java_program)) {
fprintf(stderr, "Couldn't find java at '%s'.\n", java_program.c_str());
@@ -365,9 +365,9 @@
exit(1);
}
// If the full JDK is installed
- string jdk_rt_jar = GetHostJavabase() + "/jre/lib/rt.jar";
+ string jdk_rt_jar = blaze_util::JoinPath(GetHostJavabase(), "jre/lib/rt.jar");
// If just the JRE is installed
- string jre_rt_jar = GetHostJavabase() + "/lib/rt.jar";
+ string jre_rt_jar = blaze_util::JoinPath(GetHostJavabase(), "lib/rt.jar");
if (blaze_util::CanAccess(jdk_rt_jar, true, false, false)
|| blaze_util::CanAccess(jre_rt_jar, true, false, false)) {
return java_program;
@@ -397,13 +397,14 @@
const string &host_javabase, vector<string> *result,
const vector<string> &user_options, string *error) const {
// Configure logging
- const string propFile = output_base + "/javalog.properties";
+ const string propFile =
+ blaze_util::JoinPath(output_base, "javalog.properties");
if (!blaze_util::WriteFile("handlers=java.util.logging.FileHandler\n"
".level=INFO\n"
"java.util.logging.FileHandler.level=INFO\n"
"java.util.logging.FileHandler.pattern=" +
- output_base +
- "/java.log\n"
+ blaze_util::JoinPath(output_base, "java.log") +
+ "\n"
"java.util.logging.FileHandler.limit=50000\n"
"java.util.logging.FileHandler.count=1\n"
"java.util.logging.FileHandler.formatter="
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index 78322df..e6df884 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -69,6 +69,12 @@
// Returns true if `path` refers to a directory or a symlink/junction to one.
bool IsDirectory(const std::string& path);
+// Returns true if `path` is the root directory or a Windows drive root.
+bool IsRootDirectory(const std::string &path);
+
+// Returns true if `path` is absolute.
+bool IsAbsolute(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);
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 9e835d7..cb7bd8b 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -77,7 +77,7 @@
}
static bool MakeDirectories(const string &path, mode_t mode, bool childmost) {
- if (path.empty() || path == "/") {
+ if (path.empty() || IsRootDirectory(path)) {
errno = EACCES;
return false;
}
@@ -272,6 +272,12 @@
return stat(path.c_str(), &buf) == 0 && S_ISDIR(buf.st_mode);
}
+bool IsRootDirectory(const string &path) {
+ return path.size() == 1 && path[0] == '/';
+}
+
+bool IsAbsolute(const string &path) { return !path.empty() && path[0] == '/'; }
+
void SyncFile(const string& path) {
// fsync always fails on Cygwin with "Permission denied" for some reason.
#ifndef __CYGWIN__
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index 1d00269..bf6e79c 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -13,6 +13,7 @@
// limitations under the License.
#include "src/main/cpp/util/file_platform.h"
+#include <ctype.h> // isalpha
#include <windows.h>
#include "src/main/cpp/util/errors.h"
@@ -72,22 +73,30 @@
return new WindowsPipe(read_handle, write_handle);
}
-static bool IsRootDirectory(const string& path) {
- // Return true if path is "/", "\", "c:/", "c:\", "\\?\c:\", or "\??\c:\".
+// Checks if the path is absolute and/or is a root path.
+//
+// If `must_be_root` is true, then in addition to being absolute, the path must
+// also be just the root part, no other components, e.g. "c:\" is both absolute
+// and root, but "c:\foo" is just absolute.
+static bool IsRootOrAbsolute(const string& path, bool must_be_root) {
+ // An absolute path is one that starts with "/", "\", "c:/", "c:\",
+ // "\\?\c:\", or "\??\c:\".
//
// It is unclear whether the UNC prefix is just "\\?\" or is "\??\" also
// valid (in some cases it seems to be, though MSDN doesn't mention it).
return
- // path is "/" or "\"
- (path.size() == 1 && (path[0] == '/' || path[0] == '\\')) ||
- // path is "c:/" or "c:\"
- (path.size() == 3 && isalpha(path[0]) && path[1] == ':' &&
+ // path is (or starts with) "/" or "\"
+ ((must_be_root ? path.size() == 1 : !path.empty()) &&
+ (path[0] == '/' || path[0] == '\\')) ||
+ // path is (or starts with) "c:/" or "c:\" or similar
+ ((must_be_root ? path.size() == 3 : path.size() >= 3) &&
+ isalpha(path[0]) && path[1] == ':' &&
(path[2] == '/' || path[2] == '\\')) ||
- // path is "\\?\c:\" or "\??\c:\"
- (path.size() == 7 && path[0] == '\\' &&
- (path[1] == '\\' || path[1] == '?') && path[2] == '?' &&
- path[3] == '\\' && isalpha(path[4]) && path[5] == ':' &&
- path[6] == '\\');
+ // path is (or starts with) "\\?\c:\" or "\??\c:\" or similar
+ ((must_be_root ? path.size() == 7 : path.size() >= 7) &&
+ path[0] == '\\' && (path[1] == '\\' || path[1] == '?') &&
+ path[2] == '?' && path[3] == '\\' && isalpha(path[4]) &&
+ path[5] == ':' && path[6] == '\\');
}
pair<string, string> SplitPath(const string& path) {
@@ -107,7 +116,8 @@
// Include the "/" or "\" in the drive specifier.
path.substr(0, pos + 1), path.substr(pos + 1));
} else {
- // Unix path, or relative path.
+ // Windows path (neither top-level nor drive root), Unix path, or
+ // relative path.
return std::make_pair(
// If the only "/" is the leading one, then that shall be the first
// pair element, otherwise the substring up to the rightmost "/".
@@ -193,6 +203,12 @@
#else // not COMPILER_MSVC
#endif // COMPILER_MSVC
+bool IsRootDirectory(const string& path) {
+ return IsRootOrAbsolute(path, true);
+}
+
+bool IsAbsolute(const string& path) { return IsRootOrAbsolute(path, false); }
+
#ifdef COMPILER_MSVC
void SyncFile(const string& path) {
// No-op on Windows native; unsupported by Cygwin.
diff --git a/src/main/cpp/workspace_layout.cc b/src/main/cpp/workspace_layout.cc
index 1e22a1d..330d2e9 100644
--- a/src/main/cpp/workspace_layout.cc
+++ b/src/main/cpp/workspace_layout.cc
@@ -45,7 +45,7 @@
return workspace;
}
workspace = blaze_util::Dirname(workspace);
- } while (!workspace.empty() && workspace != "/");
+ } while (!workspace.empty() && !blaze_util::IsRootDirectory(workspace));
return "";
}
@@ -72,9 +72,9 @@
const string& path_to_binary) {
// TODO(b/32115171): This doesn't work on Windows. Fix this together with the
// associated bug.
- const string path = path_to_binary[0] == '/'
- ? path_to_binary
- : blaze_util::JoinPath(cwd, path_to_binary);
+ const string path = blaze_util::IsAbsolute(path_to_binary)
+ ? path_to_binary
+ : blaze_util::JoinPath(cwd, path_to_binary);
const string base = blaze_util::Basename(path_to_binary);
const string binary_blazerc_path = path + "." + base + "rc";
if (blaze_util::CanAccess(binary_blazerc_path, true, false, false)) {
@@ -83,14 +83,6 @@
return "";
}
-static string FindSystemWideBlazerc() {
- string path = "/etc/bazel.bazelrc";
- if (blaze_util::CanAccess(path, true, false, false)) {
- return path;
- }
- return "";
-}
-
void WorkspaceLayout::FindCandidateBlazercPaths(
const string& workspace,
const string& cwd,
diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc
index 0acd392..110a059 100644
--- a/src/test/cpp/util/file_posix_test.cc
+++ b/src/test/cpp/util/file_posix_test.cc
@@ -211,6 +211,7 @@
// ASSERT_LE(0, fork());
// ASSERT_TRUE(MakeDirectories(path, 0755));
}
+
TEST(FilePosixTest, Which) {
ASSERT_EQ("", Which(""));
ASSERT_EQ("", Which("foo"));
@@ -407,4 +408,32 @@
rmdir(root.c_str());
}
+TEST(FileTest, IsAbsolute) {
+ ASSERT_FALSE(IsAbsolute(""));
+ ASSERT_TRUE(IsAbsolute("/"));
+ ASSERT_TRUE(IsAbsolute("/foo"));
+ ASSERT_FALSE(IsAbsolute("\\"));
+ ASSERT_FALSE(IsAbsolute("\\foo"));
+ ASSERT_FALSE(IsAbsolute("c:"));
+ ASSERT_FALSE(IsAbsolute("c:/"));
+ ASSERT_FALSE(IsAbsolute("c:\\"));
+ ASSERT_FALSE(IsAbsolute("c:\\foo"));
+ ASSERT_FALSE(IsAbsolute("\\\\?\\c:\\"));
+ ASSERT_FALSE(IsAbsolute("\\\\?\\c:\\foo"));
+}
+
+TEST(FileTest, IsRootDirectory) {
+ ASSERT_FALSE(IsRootDirectory(""));
+ ASSERT_TRUE(IsRootDirectory("/"));
+ ASSERT_FALSE(IsRootDirectory("/foo"));
+ ASSERT_FALSE(IsRootDirectory("\\"));
+ ASSERT_FALSE(IsRootDirectory("\\foo"));
+ ASSERT_FALSE(IsRootDirectory("c:"));
+ ASSERT_FALSE(IsRootDirectory("c:/"));
+ ASSERT_FALSE(IsRootDirectory("c:\\"));
+ ASSERT_FALSE(IsRootDirectory("c:\\foo"));
+ ASSERT_FALSE(IsRootDirectory("\\\\?\\c:\\"));
+ ASSERT_FALSE(IsRootDirectory("\\\\?\\c:\\foo"));
+}
+
} // namespace blaze_util
diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc
index 85cc7a4..00486d4 100644
--- a/src/test/cpp/util/file_windows_test.cc
+++ b/src/test/cpp/util/file_windows_test.cc
@@ -65,4 +65,32 @@
ASSERT_EQ("foo", Basename("\\\\?\\c:\\foo"));
}
+TEST(FileTest, IsAbsolute) {
+ ASSERT_FALSE(IsAbsolute(""));
+ ASSERT_TRUE(IsAbsolute("/"));
+ ASSERT_TRUE(IsAbsolute("/foo"));
+ ASSERT_TRUE(IsAbsolute("\\"));
+ ASSERT_TRUE(IsAbsolute("\\foo"));
+ ASSERT_FALSE(IsAbsolute("c:"));
+ ASSERT_TRUE(IsAbsolute("c:/"));
+ ASSERT_TRUE(IsAbsolute("c:\\"));
+ ASSERT_TRUE(IsAbsolute("c:\\foo"));
+ ASSERT_TRUE(IsAbsolute("\\\\?\\c:\\"));
+ ASSERT_TRUE(IsAbsolute("\\\\?\\c:\\foo"));
+}
+
+TEST(FileTest, IsRootDirectory) {
+ ASSERT_FALSE(IsRootDirectory(""));
+ ASSERT_TRUE(IsRootDirectory("/"));
+ ASSERT_FALSE(IsRootDirectory("/foo"));
+ ASSERT_TRUE(IsRootDirectory("\\"));
+ ASSERT_FALSE(IsRootDirectory("\\foo"));
+ ASSERT_FALSE(IsRootDirectory("c:"));
+ ASSERT_TRUE(IsRootDirectory("c:/"));
+ ASSERT_TRUE(IsRootDirectory("c:\\"));
+ ASSERT_FALSE(IsRootDirectory("c:\\foo"));
+ ASSERT_TRUE(IsRootDirectory("\\\\?\\c:\\"));
+ ASSERT_FALSE(IsRootDirectory("\\\\?\\c:\\foo"));
+}
+
} // namespace blaze_util
