Bazel client: split CanAccess to specific methods
The new methods (CanReadFile, CanExecuteFile,
CanAccessDirectory) are a lot easier to implement
on Windows than a generic CanAccess. On POSIX
these methods are just a wrapper around the now
static-visible CanAccess().
See https://github.com/bazelbuild/bazel/issues/2107
--
PiperOrigin-RevId: 144176710
MOS_MIGRATED_REVID=144176710
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 9958d04..225aef5 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -939,7 +939,10 @@
for (const auto& it : globals->extracted_binaries) {
string path = blaze_util::JoinPath(real_install_dir, it);
// Check that the file exists and is readable.
- if (!blaze_util::CanAccess(path, true, false, false)) {
+ if (blaze_util::IsDirectory(path)) {
+ continue;
+ }
+ if (!blaze_util::CanReadFile(path)) {
die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
"Error: corrupt installation: file '%s' missing."
" Please remove '%s' and try again.",
@@ -948,19 +951,17 @@
// Check that the timestamp is in the future. A past timestamp would
// indicate that the file has been tampered with.
// See ActuallyExtractData().
- if (!blaze_util::IsDirectory(path)) {
- time_t mtime = blaze_util::GetMtimeMillisec(path);
- if (mtime == -1) {
- die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
- "Error: could not retrieve mtime of file '%s'. "
- "Please remove '%s' and try again.",
- path.c_str(), globals->options->install_base.c_str());
- } else if (mtime <= time_now) {
- die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
- "Error: corrupt installation: file '%s' "
- "modified. Please remove '%s' and try again.",
- path.c_str(), globals->options->install_base.c_str());
- }
+ time_t mtime = blaze_util::GetMtimeMillisec(path);
+ if (mtime == -1) {
+ die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
+ "Error: could not retrieve mtime of file '%s'. "
+ "Please remove '%s' and try again.",
+ path.c_str(), globals->options->install_base.c_str());
+ } else if (mtime <= time_now) {
+ die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
+ "Error: corrupt installation: file '%s' "
+ "modified. Please remove '%s' and try again.",
+ path.c_str(), globals->options->install_base.c_str());
}
}
}
@@ -1199,7 +1200,7 @@
output_base);
}
}
- if (!blaze_util::CanAccess(globals->options->output_base, true, true, true)) {
+ if (!blaze_util::CanAccessDirectory(globals->options->output_base)) {
die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
"Error: Output base directory '%s' must be readable and writable.",
output_base);
diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h
index c196dd2..18efa8e 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -83,6 +83,9 @@
// (must be an absolute directory).
std::string GetDefaultHostJavabase();
+// Return the path to the JVM binary relative to a javabase, e.g. "bin/java".
+std::string GetJavaBinaryUnderJavabase();
+
// Replace the current process with the given program in the current working
// directory, using the given argument vector.
// This function does not return on success.
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index 6c22370..d978607 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -127,12 +127,14 @@
string FindSystemWideBlazerc() {
string path = "/etc/bazel.bazelrc";
- if (blaze_util::CanAccess(path, true, false, false)) {
+ if (blaze_util::CanReadFile(path)) {
return path;
}
return "";
}
+string GetJavaBinaryUnderJavabase() { return "bin/java"; }
+
void ExecuteProgram(const string &exe, const vector<string> &args_vector) {
if (VerboseLogging()) {
string dbg;
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index 0595de6..0d8bdb2 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -308,13 +308,15 @@
return "";
#else // not COMPILER_MSVC
string path = "/etc/bazel.bazelrc";
- if (blaze_util::CanAccess(path, true, false, false)) {
+ if (blaze_util::CanReadFile(path)) {
return path;
}
return "";
#endif // COMPILER_MSVC
}
+string GetJavaBinaryUnderJavabase() { return "bin/java.exe"; }
+
uint64_t GetMillisecondsMonotonic() {
return WindowsClock::INSTANCE.GetMilliseconds();
}
@@ -374,17 +376,16 @@
const vector<string>& args_vector) {
std::ostringstream cmdline;
string short_exe;
- if (!blaze_util::AsShortWindowsPath(exe + ".exe", &short_exe)) {
+ if (!blaze_util::AsShortWindowsPath(exe, &short_exe)) {
pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
- "CreateCommandLine: AsShortWindowsPath(%s.exe) failed, err=%d",
+ "CreateCommandLine: AsShortWindowsPath(%s) failed, err=%d",
exe.c_str(), GetLastError());
}
bool first = true;
for (const auto& s : args_vector) {
if (first) {
first = false;
- // Skip first argument, instead use quoted executable name with ".exe"
- // suffix.
+ // Skip first argument, instead use quoted executable name.
cmdline << '\"' << exe << '\"';
continue;
} else {
@@ -468,11 +469,11 @@
startupInfo.dwFlags |= STARTF_USESTDHANDLES;
string win_java_exe;
- if (!blaze_util::AsShortWindowsPath(java_exe + ".exe", &win_java_exe)) {
+ if (!blaze_util::AsShortWindowsPath(java_exe, &win_java_exe)) {
CloseHandle(pipe_read);
CloseHandle(pipe_write);
pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
- "GetJvmVersion: AsWindowsPath(%s.exe)", java_exe.c_str());
+ "GetJvmVersion: AsWindowsPath(%s)", java_exe.c_str());
}
win_java_exe = string("\"") + win_java_exe + "\" -version";
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 55c9cfd..d9a7b2b 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -265,7 +265,7 @@
string* error) {
if (cmdLineRcFile != NULL) {
string rcFile = MakeAbsolute(cmdLineRcFile);
- if (!blaze_util::CanAccess(rcFile, true, false, false)) {
+ if (!blaze_util::CanReadFile(rcFile)) {
blaze_util::StringPrintf(error,
"Error: Unable to read .blazerc file '%s'.", rcFile.c_str());
return blaze_exit_code::BAD_ARGV;
@@ -275,7 +275,7 @@
}
string workspaceRcFile = blaze_util::JoinPath(workspace, rc_basename);
- if (blaze_util::CanAccess(workspaceRcFile, true, false, false)) {
+ if (blaze_util::CanReadFile(workspaceRcFile)) {
*blaze_rc_file = workspaceRcFile;
return blaze_exit_code::SUCCESS;
}
@@ -287,7 +287,7 @@
}
string userRcFile = blaze_util::JoinPath(home, rc_basename);
- if (blaze_util::CanAccess(userRcFile, true, false, false)) {
+ if (blaze_util::CanReadFile(userRcFile)) {
*blaze_rc_file = userRcFile;
return blaze_exit_code::SUCCESS;
}
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index c69cf0b..f824360 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -354,8 +354,9 @@
}
string StartupOptions::GetJvm() {
- string java_program = blaze_util::JoinPath(GetHostJavabase(), "bin/java");
- if (!blaze_util::CanAccess(java_program, false, false, true)) {
+ string java_program =
+ blaze_util::JoinPath(GetHostJavabase(), GetJavaBinaryUnderJavabase());
+ if (!blaze_util::CanExecuteFile(java_program)) {
if (!blaze_util::PathExists(java_program)) {
fprintf(stderr, "Couldn't find java at '%s'.\n", java_program.c_str());
} else {
@@ -368,8 +369,8 @@
string jdk_rt_jar = blaze_util::JoinPath(GetHostJavabase(), "jre/lib/rt.jar");
// If just the JRE is installed
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)) {
+ if (blaze_util::CanReadFile(jdk_rt_jar) ||
+ blaze_util::CanReadFile(jre_rt_jar)) {
return java_program;
}
fprintf(stderr, "Problem with java installation: "
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index 91fc0866..2c07c37 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -54,12 +54,18 @@
// This is a wrapper around realpath(3).
std::string MakeCanonical(const char *path);
-// Returns true if the path exists and can be accessed to read/write as desired.
-//
-// If `exec` is true and the path refers to a file, it means the file must be
-// executable; if the path is a directory, it means the directory must be
-// openable.
-bool CanAccess(const std::string& path, bool read, bool write, bool exec);
+// Returns true if `path` exists, is a file or symlink to one, and is readable.
+// Follows symlinks.
+bool CanReadFile(const std::string &path);
+
+// Returns true if `path` exists, is a file or symlink to one, and is writable.
+// Follows symlinks.
+bool CanExecuteFile(const std::string &path);
+
+// Returns true if `path` exists, is a directory or symlink/junction to one, and
+// is both readable and writable.
+// Follows symlinks/junctions.
+bool CanAccessDirectory(const std::string &path);
// Returns true if `path` refers to a directory or a symlink/junction to one.
bool IsDirectory(const std::string& path);
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 466bae4..c1a313a 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -234,7 +234,7 @@
}
}
-bool CanAccess(const string& path, bool read, bool write, bool exec) {
+static bool CanAccess(const string &path, bool read, bool write, bool exec) {
int mode = 0;
if (read) {
mode |= R_OK;
@@ -248,6 +248,18 @@
return access(path.c_str(), mode) == 0;
}
+bool CanReadFile(const std::string &path) {
+ return !IsDirectory(path) && CanAccess(path, true, false, false);
+}
+
+bool CanExecuteFile(const std::string &path) {
+ return !IsDirectory(path) && CanAccess(path, false, false, true);
+}
+
+bool CanAccessDirectory(const std::string &path) {
+ return IsDirectory(path) && CanAccess(path, true, true, true);
+}
+
#ifndef __CYGWIN__
bool IsDirectory(const string& path) {
struct stat buf;
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index 3a5b5ac..4293d5e 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -548,12 +548,23 @@
}
#ifdef COMPILER_MSVC
-bool CanAccess(const string& path, bool read, bool write, bool exec) {
+bool CanReadFile(const std::string& path) {
// TODO(bazel-team): implement this.
- pdie(255, "blaze_util::CanAccess is not implemented on Windows");
+ pdie(255, "not implemented on Windows");
return false;
}
-#else // not COMPILER_MSVC
+
+bool CanExecuteFile(const std::string& path) {
+ // TODO(bazel-team): implement this.
+ pdie(255, "not implemented on Windows");
+ return false;
+}
+
+bool CanAccessDirectory(const std::string& path) {
+ // TODO(bazel-team): implement this.
+ pdie(255, "not implemented on Windows");
+ return false;
+}
#endif // COMPILER_MSVC
static bool IsDirectoryW(const wstring& path) {
diff --git a/src/main/cpp/workspace_layout.cc b/src/main/cpp/workspace_layout.cc
index 330d2e9..7d8f4d7 100644
--- a/src/main/cpp/workspace_layout.cc
+++ b/src/main/cpp/workspace_layout.cc
@@ -61,7 +61,7 @@
workspace_layout->WorkspaceRcFileSearchPath(&candidates);
for (const auto& candidate : candidates) {
string blazerc = blaze_util::JoinPath(workspace, candidate);
- if (blaze_util::CanAccess(blazerc, true, false, false)) {
+ if (blaze_util::CanReadFile(blazerc)) {
return blazerc;
}
}
@@ -77,7 +77,7 @@
: 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)) {
+ if (blaze_util::CanReadFile(binary_blazerc_path)) {
return binary_blazerc_path;
}
return "";
diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc
index 12030f7..b100016 100644
--- a/src/test/cpp/util/file_posix_test.cc
+++ b/src/test/cpp/util/file_posix_test.cc
@@ -226,41 +226,48 @@
}
TEST(FilePosixTest, CanAccess) {
- for (int i = 0; i < 8; ++i) {
- ASSERT_FALSE(CanAccess("/this/should/not/exist/mkay", i & 1, i & 2, i & 4));
- ASSERT_FALSE(CanAccess("non.existent", i & 1, i & 2, i & 4));
- }
+ ASSERT_FALSE(CanReadFile("/this/should/not/exist/mkay"));
+ ASSERT_FALSE(CanExecuteFile("/this/should/not/exist/mkay"));
+ ASSERT_FALSE(CanAccessDirectory("/this/should/not/exist/mkay"));
- for (int i = 0; i < 4; ++i) {
- // /usr/bin/yes exists on Linux, Darwin, and MSYS
- ASSERT_TRUE(CanAccess("/", i & 1, false, i & 2));
- ASSERT_TRUE(CanAccess("/usr", i & 1, false, i & 2));
- ASSERT_TRUE(CanAccess("/usr/", i & 1, false, i & 2));
- ASSERT_TRUE(CanAccess("/usr/bin/yes", i & 1, false, i & 2));
- }
+ ASSERT_FALSE(CanReadFile("non.existent"));
+ ASSERT_FALSE(CanExecuteFile("non.existent"));
+ ASSERT_FALSE(CanAccessDirectory("non.existent"));
- char* tmpdir_cstr = getenv("TEST_TMPDIR");
- ASSERT_FALSE(tmpdir_cstr == NULL);
+ const char* tmpdir = getenv("TEST_TMPDIR");
+ ASSERT_NE(nullptr, tmpdir);
+ ASSERT_NE(0, *tmpdir);
- string tmpdir(tmpdir_cstr);
- ASSERT_NE("", tmpdir);
+ string dir(JoinPath(tmpdir, "canaccesstest"));
+ ASSERT_EQ(0, mkdir(dir.c_str(), 0700));
- string mock_file = tmpdir + (tmpdir.back() == '/' ? "" : "/") +
- "FilePosixTest.CanAccess.mock_file";
- int fd = open(mock_file.c_str(), O_CREAT, 0500);
- ASSERT_GT(fd, 0);
- close(fd);
+ ASSERT_FALSE(CanReadFile(dir));
+ ASSERT_FALSE(CanExecuteFile(dir));
+ ASSERT_TRUE(CanAccessDirectory(dir));
- // Sanity check: assert that we successfully created the file with the given
- // permissions.
- ASSERT_EQ(0, access(mock_file.c_str(), R_OK | X_OK));
- ASSERT_NE(0, access(mock_file.c_str(), R_OK | W_OK | X_OK));
+ string file(JoinPath(dir, "foo.txt"));
+ FILE* fh = fopen(file.c_str(), "wt");
+ ASSERT_NE(nullptr, fh);
+ ASSERT_LT(0, fprintf(fh, "hello"));
+ fclose(fh);
- // Actual assertion
- for (int i = 0; i < 4; ++i) {
- ASSERT_TRUE(CanAccess(mock_file, i & 1, false, i & 2));
- ASSERT_FALSE(CanAccess(mock_file, i & 1, true, i & 2));
- }
+ ASSERT_TRUE(CanReadFile(file));
+ ASSERT_FALSE(CanExecuteFile(file));
+ ASSERT_FALSE(CanAccessDirectory(file));
+
+ ASSERT_EQ(0, chmod(file.c_str(), 0100));
+ ASSERT_FALSE(CanReadFile(file));
+ ASSERT_TRUE(CanExecuteFile(file));
+ ASSERT_FALSE(CanAccessDirectory(file));
+
+ ASSERT_EQ(0, chmod(dir.c_str(), 0500));
+ ASSERT_FALSE(CanReadFile(dir));
+ ASSERT_FALSE(CanExecuteFile(dir));
+ ASSERT_FALSE(CanAccessDirectory(dir));
+ ASSERT_EQ(0, chmod(dir.c_str(), 0700));
+
+ ASSERT_EQ(0, unlink(file.c_str()));
+ ASSERT_EQ(0, rmdir(dir.c_str()));
}
TEST(FilePosixTest, GetCwd) {
