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

11 files changed, 117 insertions, 73 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 9958d0438e..225aef5f2b 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -939,7 +939,10 @@ static void ExtractData(const string &self_path) {
     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 @@ static void ExtractData(const string &self_path) {
       // 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 @@ static void ComputeBaseDirectories(const WorkspaceLayout* workspace_layout,
           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 c196dd2119..18efa8e78f 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -83,6 +83,9 @@ bool IsSharedLibrary(const std::string& filename);
 // (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 6c223704d1..d978607123 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -127,12 +127,14 @@ string GetProcessIdAsString() {
 
 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 0595de6bcf..0d8bdb22b8 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -308,13 +308,15 @@ string FindSystemWideBlazerc() {
   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 @@ static void CreateCommandLine(CmdLine* result, const string& exe,
                               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 @@ string GetJvmVersion(const string& java_exe) {
   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 55c9cfd40b..d9a7b2bebe 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -265,7 +265,7 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
     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 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
   }
 
   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 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
   }
 
   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 c69cf0bac3..f824360b69 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -354,8 +354,9 @@ string StartupOptions::GetHostJavabase() {
 }
 
 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 StartupOptions::GetJvm() {
   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 91fc086627..2c07c3731d 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -54,12 +54,18 @@ bool PathExists(const std::string& path);
 // 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 466bae499d..c1a313a1f3 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -234,7 +234,7 @@ string MakeCanonical(const char *path) {
   }
 }
 
-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 @@ bool CanAccess(const string& path, bool read, bool write, bool exec) {
   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 3a5b5ac8e4..4293d5ead7 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -548,12 +548,23 @@ bool PathExists(const string& path) {
 }
 
 #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;
+}
+
+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;
 }
-#else  // not COMPILER_MSVC
 #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 330d2e9b28..7d8f4d7a15 100644
--- a/src/main/cpp/workspace_layout.cc
+++ b/src/main/cpp/workspace_layout.cc
@@ -61,7 +61,7 @@ static string FindDepotBlazerc(const blaze::WorkspaceLayout* workspace_layout,
   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 @@ static string FindAlongsideBinaryBlazerc(const string& cwd,
                           : 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 12030f7474..b100016076 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, PathExists) {
 }
 
 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));
-  }
-
-  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));
-  }
-
-  char* tmpdir_cstr = getenv("TEST_TMPDIR");
-  ASSERT_FALSE(tmpdir_cstr == NULL);
+  ASSERT_FALSE(CanReadFile("/this/should/not/exist/mkay"));
+  ASSERT_FALSE(CanExecuteFile("/this/should/not/exist/mkay"));
+  ASSERT_FALSE(CanAccessDirectory("/this/should/not/exist/mkay"));
 
-  string tmpdir(tmpdir_cstr);
-  ASSERT_NE("", tmpdir);
+  ASSERT_FALSE(CanReadFile("non.existent"));
+  ASSERT_FALSE(CanExecuteFile("non.existent"));
+  ASSERT_FALSE(CanAccessDirectory("non.existent"));
 
-  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);
+  const char* tmpdir = getenv("TEST_TMPDIR");
+  ASSERT_NE(nullptr, tmpdir);
+  ASSERT_NE(0, *tmpdir);
 
-  // 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 dir(JoinPath(tmpdir, "canaccesstest"));
+  ASSERT_EQ(0, mkdir(dir.c_str(), 0700));
 
-  // 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_FALSE(CanReadFile(dir));
+  ASSERT_FALSE(CanExecuteFile(dir));
+  ASSERT_TRUE(CanAccessDirectory(dir));
+
+  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);
+
+  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) {