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 "";