From 6906ba6a0efe18171a78b396ca3057f515c34a0a Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 21 Nov 2024 09:51:28 -0800 Subject: [PATCH] Use blaze_util::Path instead of std::string in more places. PiperOrigin-RevId: 698818638 Change-Id: Iea02eb6d3524ecc98970a63aa554927c701efe28 --- src/main/cpp/archive_utils.cc | 60 ++++---- src/main/cpp/archive_utils.h | 4 +- src/main/cpp/bazel_startup_options.cc | 6 +- src/main/cpp/blaze.cc | 44 +++--- src/main/cpp/blaze_util_platform.h | 13 +- src/main/cpp/blaze_util_posix.cc | 48 +++--- src/main/cpp/blaze_util_windows.cc | 75 +++++----- src/main/cpp/startup_options.cc | 25 ++-- src/main/cpp/startup_options.h | 6 +- src/main/cpp/util/file.cc | 34 ++--- src/main/cpp/util/file.h | 14 +- src/main/cpp/util/file_platform.h | 35 ++--- src/main/cpp/util/file_posix.cc | 69 +++++---- src/main/cpp/util/file_windows.cc | 125 +++++++--------- src/main/cpp/util/path_platform.h | 12 +- src/main/cpp/util/path_posix.cc | 4 +- src/main/cpp/util/path_windows.cc | 9 +- src/test/cpp/option_processor_test.cc | 10 +- src/test/cpp/rc_file_test.cc | 25 +--- src/test/cpp/startup_options_test.cc | 27 ++-- src/test/cpp/util/BUILD | 1 + src/test/cpp/util/file_posix_test.cc | 207 ++++++++++---------------- src/test/cpp/util/file_test.cc | 168 ++++++++++++--------- src/test/cpp/workspace_layout_test.cc | 10 +- 24 files changed, 470 insertions(+), 561 deletions(-) diff --git a/src/main/cpp/archive_utils.cc b/src/main/cpp/archive_utils.cc index e54d6f2503954c..a08d544833e89c 100644 --- a/src/main/cpp/archive_utils.cc +++ b/src/main/cpp/archive_utils.cc @@ -29,7 +29,7 @@ #include "src/main/cpp/util/file.h" #include "src/main/cpp/util/file_platform.h" #include "src/main/cpp/util/logging.h" -#include "src/main/cpp/util/path.h" +#include "src/main/cpp/util/path_platform.h" #include "third_party/ijar/zip.h" namespace blaze { @@ -116,12 +116,13 @@ std::optional ExtractData( const string &self_path, const vector &archive_contents, const string &expected_install_md5, const StartupOptions &startup_options, LoggingInfo *logging_info) { - const string &install_base = startup_options.install_base; + const blaze_util::Path &install_base = startup_options.install_base; // If the install dir doesn't exist, create it, if it does, we know it's good. if (!blaze_util::PathExists(install_base)) { uint64_t st = GetMillisecondsMonotonic(); - // Work in a temp dir to avoid races. - string tmp_install = blaze_util::CreateTempDir(install_base + ".tmp."); + // Work in a sibling temporary directory to avoid races. + blaze_util::Path tmp_install = + blaze_util::CreateSiblingTempDir(install_base); ExtractArchiveOrDie(self_path, startup_options.product_name, expected_install_md5, tmp_install); BlessFiles(tmp_install); @@ -144,7 +145,8 @@ std::optional ExtractData( // (in case we're running on Windows) so we need to wait for that to // finish and try renaming again. ++attempts; - BAZEL_LOG(USER) << "install base directory '" << tmp_install + BAZEL_LOG(USER) << "install base directory '" + << tmp_install.AsPrintablePath() << "' could not be renamed into place after " << attempts << " second(s), trying again\r"; std::this_thread::sleep_for(std::chrono::seconds(1)); @@ -155,7 +157,7 @@ std::optional ExtractData( if (attempts == 120) { blaze_util::RemoveRecursively(tmp_install); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "install base directory '" << tmp_install + << "install base directory '" << tmp_install.AsPrintablePath() << "' could not be renamed into place: " << blaze_util::GetLastErrorString(); } @@ -165,7 +167,7 @@ std::optional ExtractData( // us give a better error message. if (!blaze_util::IsDirectory(install_base)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "install base directory '" << install_base + << "install base directory '" << install_base.AsPrintablePath() << "' could not be created. It exists but is not a directory."; } blaze_util::Path install_dir(install_base); @@ -175,8 +177,8 @@ std::optional ExtractData( if (!IsUntampered(path)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "corrupt installation: file '" << path.AsPrintablePath() - << "' is missing or modified. Please remove '" << install_base - << "' and try again."; + << "' is missing or modified. Please remove '" + << install_base.AsPrintablePath() << "' and try again."; } } // Also check that the installed files claim to match this binary. @@ -191,7 +193,7 @@ std::optional ExtractData( } if (on_disk_key != expected_install_md5) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "The install_base directory '" << install_base + << "The install_base directory '" << install_base.AsPrintablePath() << "' contains a different " << startup_options.product_name << " version (found " << on_disk_key << " but this binary is " << expected_install_md5 @@ -209,7 +211,7 @@ void DetermineArchiveContents(const string &archive_path, vector *files, void ExtractArchiveOrDie(const string &archive_path, const string &product_name, const string &expected_install_md5, - const string &output_dir) { + const blaze_util::Path &output_dir) { string error; std::unique_ptr dumper( blaze::embedded_binaries::Create(&error)); @@ -219,17 +221,18 @@ void ExtractArchiveOrDie(const string &archive_path, const string &product_name, if (!blaze_util::PathExists(output_dir)) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) - << "Archive output directory didn't exist: " << output_dir; + << "Archive output directory didn't exist: " + << output_dir.AsPrintablePath(); } BAZEL_LOG(USER) << "Extracting " << product_name << " installation..."; PartialZipExtractor pze; - string install_md5 = pze.UnzipUntil( - archive_path, "install_base_key", nullptr, - [&](const char *name, const char *data, size_t size) { - dumper->Dump(data, size, blaze_util::JoinPath(output_dir, name)); - }); + string install_md5 = + pze.UnzipUntil(archive_path, "install_base_key", nullptr, + [&](const char *name, const char *data, size_t size) { + dumper->Dump(data, size, output_dir.GetRelative(name)); + }); if (!dumper->Finish(&error)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) @@ -248,22 +251,17 @@ void ExtractArchiveOrDie(const string &archive_path, const string &product_name, } } -void BlessFiles(const string &embedded_binaries) { - blaze_util::Path embedded_binaries_(embedded_binaries); - +void BlessFiles(const blaze_util::Path &embedded_binaries) { // Set the timestamps of the extracted files to the future and make sure (or // at least as sure as we can...) that the files we have written are actually // on the disk. - - vector extracted_files; + vector extracted_files; // Walks the temporary directory recursively and collects full file paths. blaze_util::GetAllFilesUnder(embedded_binaries, &extracted_files); set synced_directories; - for (const auto &f : extracted_files) { - blaze_util::Path it(f); - + for (const auto &file : extracted_files) { // Set the time to a distantly futuristic value so we can observe tampering. // Note that keeping a static, deterministic timestamp, such as the default // timestamp set by unzip (1970-01-01) and using that to detect tampering is @@ -271,16 +269,16 @@ void BlessFiles(const string &embedded_binaries) { // releases so that the metadata cache knows that the files may have // changed. This is essential for the correctness of actions that use // embedded binaries as artifacts. - if (!SetMtimeToDistantFuture(it)) { + if (!SetMtimeToDistantFuture(file)) { string err = blaze_util::GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "failed to set timestamp on '" << it.AsPrintablePath() + << "failed to set timestamp on '" << file.AsPrintablePath() << "': " << err; } - blaze_util::SyncFile(it); + blaze_util::SyncFile(file); - blaze_util::Path directory = it.GetParent(); + blaze_util::Path directory = file.GetParent(); // Now walk up until embedded_binaries and sync every directory in between. // synced_directories is used to avoid syncing the same directory twice. @@ -288,7 +286,7 @@ void BlessFiles(const string &embedded_binaries) { // 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_ && !directory.IsEmpty() && + while (directory != embedded_binaries && !directory.IsEmpty() && !blaze_util::IsRootDirectory(directory) && synced_directories.insert(directory).second) { blaze_util::SyncFile(directory); @@ -296,7 +294,7 @@ void BlessFiles(const string &embedded_binaries) { } } - blaze_util::SyncFile(embedded_binaries_); + blaze_util::SyncFile(embedded_binaries); } void ExtractBuildLabel(const string &archive_path, string *build_label) { diff --git a/src/main/cpp/archive_utils.h b/src/main/cpp/archive_utils.h index 15177b88e86136..58f676445e2de1 100644 --- a/src/main/cpp/archive_utils.h +++ b/src/main/cpp/archive_utils.h @@ -87,14 +87,14 @@ std::optional ExtractData( void ExtractArchiveOrDie(const std::string &archive_path, const std::string &product_name, const std::string &expected_install_md5, - const std::string &output_dir); + const blaze_util::Path &output_dir); // Sets the timestamps of the extracted files to the future via // blaze_util::IFileMtime::SetToDistanceFuture and ensures that the files we // have written are actually on the disk. Later, the blaze client calls // blaze_util::IFileMtime::IsUntampered to ensure the files were "blessed" with // these distant mtimes. -void BlessFiles(const std::string &embedded_binaries); +void BlessFiles(const blaze_util::Path &embedded_binaries); // Retrieves the build label (version string) from `archive_path` into // `build_label`. diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc index 1de82ed8895e2d..fe62b5f820f833 100644 --- a/src/main/cpp/bazel_startup_options.cc +++ b/src/main/cpp/bazel_startup_options.cc @@ -77,11 +77,9 @@ void BazelStartupOptions::MaybeLogStartupOptionWarnings() const { "ignored, since --ignore_all_rc_files is on."; } } - bool output_user_root_has_space = - output_user_root.find_first_of(' ') != std::string::npos; - if (output_user_root_has_space) { + if (output_user_root.Contains(' ')) { BAZEL_LOG(WARNING) - << "Output user root \"" << output_user_root + << "Output user root \"" << output_user_root.AsPrintablePath() << "\" contains a space. This will probably break the build. " "You should set a different --output_user_root."; } else if (output_base.Contains(' ')) { diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 7e22563fe3cdba..34ad73407f4da6 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -425,9 +425,9 @@ static vector GetServerExeArgs(const blaze_util::Path &jvm_path, blaze_util::ToString(startup_options.connect_timeout_secs)); result.push_back("--output_user_root=" + - blaze_util::ConvertPath(startup_options.output_user_root)); + startup_options.output_user_root.AsCommandLineArgument()); result.push_back("--install_base=" + - blaze_util::ConvertPath(startup_options.install_base)); + startup_options.install_base.AsCommandLineArgument()); result.push_back("--install_md5=" + install_md5); result.push_back("--output_base=" + startup_options.output_base.AsCommandLineArgument()); @@ -1006,13 +1006,13 @@ static void EnsureCorrectRunningVersion(const StartupOptions &startup_options, // 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. - const blaze_util::Path installation_path = + const blaze_util::Path install_base_symlink = startup_options.output_base.GetRelative("install"); - string prev_installation; - bool ok = - blaze_util::ReadDirectorySymlink(installation_path, &prev_installation); - if (!ok || !blaze_util::CompareAbsolutePaths(prev_installation, - startup_options.install_base)) { + blaze_util::Path prev_install_base; + if (!blaze_util::ReadDirectorySymlink(install_base_symlink, + &prev_install_base) || + !blaze_util::ArePathsEquivalent(prev_install_base, + startup_options.install_base)) { if (server->Connected()) { BAZEL_LOG(INFO) << "Killing running server because it is using another version of " @@ -1021,12 +1021,13 @@ static void EnsureCorrectRunningVersion(const StartupOptions &startup_options, logging_info->restart_reason = NEW_VERSION; } - blaze_util::UnlinkPath(installation_path); - if (!SymlinkDirectories(startup_options.install_base, installation_path)) { - string err = GetLastErrorString(); + blaze_util::UnlinkPath(install_base_symlink); + if (!SymlinkDirectories(startup_options.install_base, + install_base_symlink)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "failed to create installation symlink '" - << installation_path.AsPrintablePath() << "': " << err; + << install_base_symlink.AsPrintablePath() + << "': " << GetLastErrorString(); } // Update the mtime of the install base so that cleanup tools can @@ -1034,10 +1035,10 @@ static void EnsureCorrectRunningVersion(const StartupOptions &startup_options, // Ignore permissions errors (i.e. if the install base is not writable). if (!SetMtimeToNowIfPossible( blaze_util::Path(startup_options.install_base))) { - string err = GetLastErrorString(); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "failed to set timestamp on '" << startup_options.install_base - << "': " << err; + << "failed to set timestamp on '" + << startup_options.install_base.AsPrintablePath() + << "': " << GetLastErrorString(); } } } @@ -1142,15 +1143,14 @@ static void UpdateConfiguration(const string &install_md5, // The default install_base is /install/ // but if an install_base is specified on the command line, we use that as // the base instead. - if (startup_options->install_base.empty()) { + if (startup_options->install_base.IsEmpty()) { if (server_mode) { BAZEL_DIE(blaze_exit_code::BAD_ARGV) << "exec-server requires --install_base"; } - string install_user_root = - blaze_util::JoinPath(startup_options->output_user_root, "install"); - startup_options->install_base = - blaze_util::JoinPath(install_user_root, install_md5); + blaze_util::Path install_user_root = + startup_options->output_user_root.GetRelative("install"); + startup_options->install_base = install_user_root.GetRelative(install_md5); } if (startup_options->output_base.IsEmpty()) { @@ -1158,8 +1158,8 @@ static void UpdateConfiguration(const string &install_md5, BAZEL_DIE(blaze_exit_code::BAD_ARGV) << "exec-server requires --output_base"; } - startup_options->output_base = blaze_util::Path( - blaze::GetHashedBaseDir(startup_options->output_user_root, workspace)); + startup_options->output_base = + blaze::GetHashedBaseDir(startup_options->output_user_root, workspace); } if (!blaze_util::PathExists(startup_options->output_base)) { diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index 08dadc5e524afd..41eb047e84616f 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -1,3 +1,4 @@ +#include "src/main/cpp/util/path_platform.h" // Copyright 2014 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -47,7 +48,7 @@ class Dumper { // If writing fails, this method sets a flag in the `Dumper`, and `Finish` // will return false. Subsequent `Dump` calls will have no effect. virtual void Dump(const void* data, const size_t size, - const std::string& path) = 0; + const blaze_util::Path& path) = 0; // Finishes dumping data. // @@ -187,16 +188,16 @@ int ExecuteDaemon( const blaze_util::Path& exe, const std::vector& args_vector, const std::map& env, const blaze_util::Path& daemon_output, const bool daemon_output_append, - const std::string& binaries_dir, const blaze_util::Path& server_dir, + const blaze_util::Path& binaries_dir, const blaze_util::Path& server_dir, const StartupOptions& options, BlazeServerStartup** server_startup); // A character used to separate paths in a list. extern const char kListSeparator; // Create a symlink to directory ``target`` at location ``link``. -// Returns true on success, false on failure. The target must be absolute. +// Returns true on success, false on failure. // Implemented via junctions on Windows. -bool SymlinkDirectories(const std::string& target, +bool SymlinkDirectories(const blaze_util::Path& target, const blaze_util::Path& link); typedef uintptr_t LockHandle; @@ -237,8 +238,8 @@ void ExcludePathFromBackup(const blaze_util::Path& path); // Returns the canonical form of the base dir given a root and a hashable // string. The resulting dir is composed of the root + md5(hashable) -std::string GetHashedBaseDir(const std::string& root, - const std::string& hashable); +blaze_util::Path GetHashedBaseDir(const blaze_util::Path& root, + const std::string& hashable); // Create a safe installation directory where we keep state, installations etc. // This method ensures that the directory is created, is owned by the current diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index b5bc003e81eca6..efa105d8d49a6f 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -73,13 +73,14 @@ class PosixDumper : public Dumper { public: static PosixDumper* Create(string* error); ~PosixDumper() { Finish(nullptr); } - void Dump(const void* data, const size_t size, const string& path) override; + void Dump(const void* data, size_t size, + const blaze_util::Path& path) override; bool Finish(string* error) override; private: PosixDumper() : was_error_(false) {} - set dir_cache_; + set dir_cache_; string error_msg_; bool was_error_; }; @@ -89,20 +90,21 @@ Dumper* Create(string* error) { return PosixDumper::Create(error); } PosixDumper* PosixDumper::Create(string* error) { return new PosixDumper(); } void PosixDumper::Dump(const void* data, const size_t size, - const string& path) { + const blaze_util::Path& path) { if (was_error_) { return; } - string dirname = blaze_util::Dirname(path); + blaze_util::Path parent = path.GetParent(); // Performance optimization: memoize the paths we already created a // directory for, to spare a stat in attempting to recreate an already // existing directory. - if (dir_cache_.insert(dirname).second) { - if (!blaze_util::MakeDirectories(dirname, 0777)) { + if (dir_cache_.insert(parent).second) { + if (!blaze_util::MakeDirectories(parent, 0777)) { was_error_ = true; string msg = GetLastErrorString(); - error_msg_ = string("couldn't create '") + path + "': " + msg; + error_msg_ = + string("couldn't create '") + path.AsPrintablePath() + "': " + msg; } } @@ -113,7 +115,8 @@ void PosixDumper::Dump(const void* data, const size_t size, if (!blaze_util::WriteFile(data, size, path, 0755)) { was_error_ = true; string msg = GetLastErrorString(); - error_msg_ = string("Failed to write zipped file '") + path + "': " + msg; + error_msg_ = string("Failed to write zipped file '") + + path.AsPrintablePath() + "': " + msg; } } @@ -342,8 +345,10 @@ void ExecuteRunRequest(const blaze_util::Path& exe, const char kListSeparator = ':'; -bool SymlinkDirectories(const string& target, const blaze_util::Path& link) { - return symlink(target.c_str(), link.AsNativePath().c_str()) == 0; +bool SymlinkDirectories(const blaze_util::Path& target, + const blaze_util::Path& link) { + return symlink(target.AsNativePath().c_str(), link.AsNativePath().c_str()) == + 0; } // Notifies the client about the death of the server process by keeping a socket @@ -395,16 +400,14 @@ int ConfigureDaemonProcess(posix_spawnattr_t* attrp, void WriteSystemSpecificProcessIdentifier(const blaze_util::Path& server_dir, pid_t server_pid); -int ExecuteDaemon(const blaze_util::Path& exe, - const std::vector& args_vector, - const std::map& env, - const blaze_util::Path& daemon_output, - const bool daemon_output_append, const string& binaries_dir, - const blaze_util::Path& server_dir, - const StartupOptions& options, - BlazeServerStartup** server_startup) { +int ExecuteDaemon( + const blaze_util::Path& exe, const std::vector& args_vector, + const std::map& env, + const blaze_util::Path& daemon_output, const bool daemon_output_append, + const blaze_util::Path& binaries_dir, const blaze_util::Path& server_dir, + const StartupOptions& options, BlazeServerStartup** server_startup) { const blaze_util::Path pid_file = server_dir.GetRelative(kServerPidFile); - const string daemonize = blaze_util::JoinPath(binaries_dir, "daemonize"); + const string daemonize = binaries_dir.GetRelative("daemonize").AsNativePath(); std::vector daemonize_args = {"daemonize", "-l", daemon_output.AsNativePath(), "-p", @@ -492,12 +495,13 @@ int ExecuteDaemon(const blaze_util::Path& exe, return server_pid; } -string GetHashedBaseDir(const string& root, const string& hashable) { +blaze_util::Path GetHashedBaseDir(const blaze_util::Path& root, + const string& hashable) { unsigned char buf[blaze_util::Md5Digest::kDigestLength]; blaze_util::Md5Digest digest; digest.Update(hashable.data(), hashable.size()); digest.Finish(buf); - return blaze_util::JoinPath(root, digest.String()); + return root.GetRelative(digest.String()); } void CreateSecureOutputRoot(const blaze_util::Path& path) { @@ -891,4 +895,4 @@ void EnsurePythonPathOption(vector* options) { // do nothing. } -} // namespace blaze. +} // namespace blaze. diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index acefea4f725471..f80d04e9ee650b 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -87,7 +87,8 @@ class WindowsDumper : public Dumper { public: static WindowsDumper* Create(string* error); ~WindowsDumper() { Finish(nullptr); } - void Dump(const void* data, const size_t size, const string& path) override; + void Dump(const void* data, const size_t size, + const blaze_util::Path& path) override; bool Finish(string* error) override; private: @@ -98,7 +99,7 @@ class WindowsDumper : public Dumper { TP_CALLBACK_ENVIRON threadpool_env_; std::mutex dir_cache_lock_; - std::set dir_cache_; + std::set dir_cache_; std::mutex error_lock_; string error_msg_; @@ -108,9 +109,10 @@ namespace { class DumpContext { public: - DumpContext(unique_ptr data, const size_t size, const string path, - std::mutex* dir_cache_lock, std::set* dir_cache, - std::mutex* error_lock_, string* error_msg); + DumpContext(unique_ptr data, const size_t size, + const blaze_util::Path& path, std::mutex* dir_cache_lock, + std::set* dir_cache, std::mutex* error_lock_, + string* error_msg); void Run(); private: @@ -118,10 +120,10 @@ class DumpContext { unique_ptr data_; const size_t size_; - const string path_; + const blaze_util::Path path_; std::mutex* dir_cache_lock_; - std::set* dir_cache_; + std::set* dir_cache_; std::mutex* error_lock_; string* error_msg_; @@ -172,7 +174,7 @@ WindowsDumper* WindowsDumper::Create(string* error) { } void WindowsDumper::Dump(const void* data, const size_t size, - const string& path) { + const blaze_util::Path& path) { { std::lock_guard g(error_lock_); if (!error_msg_.empty()) { @@ -218,9 +220,10 @@ bool WindowsDumper::Finish(string* error) { namespace { DumpContext::DumpContext(unique_ptr data, const size_t size, - const string path, std::mutex* dir_cache_lock, - std::set* dir_cache, std::mutex* error_lock_, - string* error_msg) + const blaze_util::Path& path, + std::mutex* dir_cache_lock, + std::set* dir_cache, + std::mutex* error_lock_, string* error_msg) : data_(std::move(data)), size_(size), path_(path), @@ -229,7 +232,7 @@ DumpContext::DumpContext(unique_ptr data, const size_t size, error_msg_(error_msg) {} void DumpContext::Run() { - string dirname = blaze_util::Dirname(path_); + blaze_util::Path parent = path_.GetParent(); bool success = true; // Performance optimization: memoize the paths we already created a @@ -238,18 +241,20 @@ void DumpContext::Run() { // extraction time on Windows. { std::lock_guard guard(*dir_cache_lock_); - if (dir_cache_->insert(dirname).second) { - success = blaze_util::MakeDirectories(dirname, 0777); + if (dir_cache_->insert(parent).second) { + success = blaze_util::MakeDirectories(parent, 0777); } } if (!success) { - MaybeSignalError(string("Couldn't create directory '") + dirname + "'"); + MaybeSignalError(string("Couldn't create directory '") + + parent.AsPrintablePath() + "'"); return; } if (!blaze_util::WriteFile(data_.get(), size_, path_, 0755)) { - MaybeSignalError(string("Failed to write zipped file '") + path_ + "'"); + MaybeSignalError(string("Failed to write zipped file '") + + path_.AsPrintablePath() + "'"); } } @@ -626,14 +631,12 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup { AutoHandle proc; }; -int ExecuteDaemon(const blaze_util::Path& exe, - const std::vector& args_vector, - const std::map& env, - const blaze_util::Path& daemon_output, - const bool daemon_out_append, const string& binaries_dir, - const blaze_util::Path& server_dir, - const StartupOptions& options, - BlazeServerStartup** server_startup) { +int ExecuteDaemon( + const blaze_util::Path& exe, const std::vector& args_vector, + const std::map& env, + const blaze_util::Path& daemon_output, const bool daemon_out_append, + const blaze_util::Path& binaries_dir, const blaze_util::Path& server_dir, + const StartupOptions& options, BlazeServerStartup** server_startup) { SECURITY_ATTRIBUTES inheritable_handle_sa = {sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE}; @@ -804,23 +807,14 @@ void ExecuteRunRequest(const blaze_util::Path& exe, const char kListSeparator = ';'; -bool SymlinkDirectories(const string& posix_target, - const blaze_util::Path& name) { - wstring target; - string error; - if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "SymlinkDirectories(" << posix_target << ", " - << name.AsPrintablePath() << "): AsAbsoluteWindowsPath(" << posix_target - << ") failed: " << error; - return false; - } +bool SymlinkDirectories(const blaze_util::Path& target, + const blaze_util::Path& link) { wstring werror; - if (CreateJunction(name.AsNativePath(), target, &werror) != + if (CreateJunction(link.AsNativePath(), target.AsNativePath(), &werror) != CreateJunctionResult::kSuccess) { string error(blaze_util::WstringToCstring(werror)); - BAZEL_LOG(ERROR) << "SymlinkDirectories(" << posix_target << ", " - << name.AsPrintablePath() + BAZEL_LOG(ERROR) << "SymlinkDirectories(" << target.AsPrintablePath() + << ", " << link.AsPrintablePath() << "): CreateJunction: " << error; return false; } @@ -890,7 +884,8 @@ void TrySleep(unsigned int milliseconds) { // Not supported. void ExcludePathFromBackup(const blaze_util::Path& path) {} -string GetHashedBaseDir(const string& root, const string& hashable) { +blaze_util::Path GetHashedBaseDir(const blaze_util::Path& root, + const string& hashable) { // Builds a shorter output base dir name for Windows. // We create a path name representing the 128 bits of MD5 digest. To avoid @@ -918,7 +913,7 @@ string GetHashedBaseDir(const string& root, const string& hashable) { coded_name[i] = alphabet[md5[i] & kLower5BitsMask]; } coded_name[filename_length] = '\0'; - return blaze_util::JoinPath(root, string(coded_name)); + return root.GetRelative(string(coded_name)); } void CreateSecureOutputRoot(const blaze_util::Path& path) { diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index a15cfe0593b4bb..42ec59b6d66924 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -106,15 +106,18 @@ StartupOptions::StartupOptions(const string &product_name, // relative path starting with "~/", PathFragmentConverter would shell-expand // it as a path relative to the home directory, and Bazel would crash. if (blaze::IsRunningWithinTest()) { - output_root = blaze_util::MakeAbsolute(blaze::GetPathEnv("TEST_TMPDIR")); + output_root = blaze_util::Path( + blaze_util::MakeAbsolute(blaze::GetPathEnv("TEST_TMPDIR"))); max_idle_secs = 15; BAZEL_LOG(USER) << "$TEST_TMPDIR defined: output root default is '" - << output_root << "' and max_idle_secs default is '" - << max_idle_secs << "'."; + << output_root.AsPrintablePath() + << "' and max_idle_secs default is '" << max_idle_secs + << "'."; } else { - output_root = blaze_util::MakeAbsolute(workspace_layout->GetOutputRoot()); + output_root = blaze_util::Path( + blaze_util::MakeAbsolute(workspace_layout->GetOutputRoot())); max_idle_secs = 3 * 3600; - BAZEL_LOG(INFO) << "output root is '" << output_root + BAZEL_LOG(INFO) << "output root is '" << output_root.AsPrintablePath() << "' and max_idle_secs default is '" << max_idle_secs << "'."; } @@ -128,8 +131,8 @@ StartupOptions::StartupOptions(const string &product_name, #endif // defined(_WIN32) || defined(__CYGWIN__) const string product_name_lower = GetLowercaseProductName(); - output_user_root = blaze_util::JoinPath( - output_root, "_" + product_name_lower + "_" + GetUserName()); + output_user_root = + output_root.GetRelative("_" + product_name_lower + "_" + GetUserName()); // IMPORTANT: Before modifying the statements below please contact a Bazel // core team member that knows the internal procedure for adding/deprecating @@ -270,11 +273,11 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( option_sources["output_base"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--install_base")) != nullptr) { - install_base = blaze::AbsolutePathFromFlag(value); + install_base = blaze_util::Path(blaze::AbsolutePathFromFlag(value)); option_sources["install_base"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--output_user_root")) != nullptr) { - output_user_root = blaze::AbsolutePathFromFlag(value); + output_user_root = blaze_util::Path(blaze::AbsolutePathFromFlag(value)); option_sources["output_user_root"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--server_jvm_out")) != nullptr) { @@ -455,8 +458,8 @@ blaze_util::Path StartupOptions::GetSystemJavabase() const { } blaze_util::Path StartupOptions::GetEmbeddedJavabase() const { - blaze_util::Path bundled_jre_path = blaze_util::Path( - blaze_util::JoinPath(install_base, "embedded_tools/jdk")); + blaze_util::Path bundled_jre_path = + install_base.GetRelative("embedded_tools").GetRelative("jdk"); if (blaze_util::CanExecuteFile( bundled_jre_path.GetRelative(GetJavaBinaryUnderJavabase()))) { return bundled_jre_path; diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 82bccf80f3425b..1869bada61aeb5 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -144,16 +144,16 @@ class StartupOptions { blaze_util::Path output_base; // Installation base for a specific release installation. - std::string install_base; + blaze_util::Path install_base; // The toplevel directory containing Blaze's output. When Blaze is // run by a test, we use TEST_TMPDIR, simplifying the correct // hermetic invocation of Blaze from tests. - std::string output_root; + blaze_util::Path output_root; // Blaze's output_user_root. Used only for computing install_base and // output_base. - std::string output_user_root; + blaze_util::Path output_user_root; // Override more finegrained rc file flags and ignore them all. bool ignore_all_rc_files; diff --git a/src/main/cpp/util/file.cc b/src/main/cpp/util/file.cc index 75212b68dac787..06c0a183469422 100644 --- a/src/main/cpp/util/file.cc +++ b/src/main/cpp/util/file.cc @@ -15,15 +15,14 @@ #include "src/main/cpp/util/file.h" #include +#include #include -#include +#include #include -#include "src/main/cpp/util/errors.h" -#include "src/main/cpp/util/exit_code.h" -#include "src/main/cpp/util/path.h" -#include "src/main/cpp/util/strings.h" +#include "src/main/cpp/util/file_platform.h" +#include "src/main/cpp/util/path_platform.h" namespace blaze_util { @@ -95,33 +94,24 @@ bool WriteFile(const std::string &content, const Path &path, class DirectoryTreeWalker : public DirectoryEntryConsumer { public: - DirectoryTreeWalker(vector *files, - _ForEachDirectoryEntry walk_entries) - : _files(files), _walk_entries(walk_entries) {} + explicit DirectoryTreeWalker(vector *files) : files(files) {} - void Consume(const string &path, bool follow_directory) override { - if (follow_directory) { + void Consume(const Path &path, bool is_directory) override { + if (is_directory) { Walk(path); } else { - _files->push_back(path); + files->push_back(path); } } - void Walk(const string &path) { _walk_entries(path, this); } + void Walk(const Path &path) { ForEachDirectoryEntry(path, this); } private: - vector *_files; - _ForEachDirectoryEntry _walk_entries; + vector *files; }; -void GetAllFilesUnder(const string &path, vector *result) { - _GetAllFilesUnder(path, result, &ForEachDirectoryEntry); -} - -void _GetAllFilesUnder(const string &path, - vector *result, - _ForEachDirectoryEntry walk_entries) { - DirectoryTreeWalker(result, walk_entries).Walk(path); +void GetAllFilesUnder(const Path &path, vector *result) { + DirectoryTreeWalker(result).Walk(path); } } // namespace blaze_util diff --git a/src/main/cpp/util/file.h b/src/main/cpp/util/file.h index be9917b41f78e3..b40033e86f621f 100644 --- a/src/main/cpp/util/file.h +++ b/src/main/cpp/util/file.h @@ -73,19 +73,7 @@ bool WriteFile(const std::string &content, const Path &path, // Populates `result` with the full paths of the files. Every entry will have // `path` as its prefix. If `path` is a file, `result` contains just this // file. -void GetAllFilesUnder(const std::string &path, - std::vector *result); - -class DirectoryEntryConsumer; - -// Visible for testing only. -typedef void (*_ForEachDirectoryEntry)(const std::string &path, - DirectoryEntryConsumer *consume); - -// Visible for testing only. -void _GetAllFilesUnder(const std::string &path, - std::vector *result, - _ForEachDirectoryEntry walk_entries); +void GetAllFilesUnder(const Path &path, std::vector *result); } // namespace blaze_util diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h index 3e0eb3d703a159..fe918069873e04 100644 --- a/src/main/cpp/util/file_platform.h +++ b/src/main/cpp/util/file_platform.h @@ -136,14 +136,15 @@ enum RenameDirectoryResult { kRenameDirectoryFailureOtherError = 2, }; -// Renames the directory at `old_name` to `new_name`. +// Renames the directory at `old_path` to `new_path`. // Returns one of the RenameDirectoryResult enum values. -int RenameDirectory(const std::string &old_name, const std::string &new_name); +int RenameDirectory(const Path &old_path, const Path &new_path); // Reads which directory a symlink points to. Puts the target of the symlink // in ``result`` and returns if the operation was successful. Will not work on // symlinks that don't point to directories on Windows. -bool ReadDirectorySymlink(const blaze_util::Path &symlink, std::string *result); +bool ReadDirectorySymlink(const blaze_util::Path &symlink, + blaze_util::Path *result); // Unlinks the file given by 'file_path'. // Returns true on success. In case of failure sets errno. @@ -192,14 +193,16 @@ void SyncFile(const Path &path); bool MakeDirectories(const std::string &path, unsigned int mode); bool MakeDirectories(const Path &path, unsigned int mode); -// Creates a directory starting with prefix for temporary usage. The directory -// name is guaranteed to be at least unique to this process. -std::string CreateTempDir(const std::string &prefix); +// Creates a temporary directory as a sibling of the given path, and returns it. +// The name of the temporary name matches the given path followed by ".tmp." and +// a string unique to the current process. +Path CreateSiblingTempDir(const Path &other_path); -// Removes the specified path or directory, and in the latter case, all of its -// contents. Returns true iff the path doesn't exists when the method completes -// (including if the path didn't exist to begin with). Does not follow symlinks. +// Removes the specified path, recursively for a directory. +// Returns true iff the path doesn't exist after the call, including if it +// didn't exist to begin with. Does not follow symlinks. bool RemoveRecursively(const std::string &path); +bool RemoveRecursively(const Path &path); // Returns the current working directory. // The path is platform-specific (e.g. Windows path of Windows) and absolute. @@ -217,7 +220,7 @@ class DirectoryEntryConsumer { // `name` is the full path of the entry. // `is_directory` is true if this entry is a directory (but false if this is a // symlink pointing to a directory). - virtual void Consume(const std::string &name, bool is_directory) = 0; + virtual void Consume(const Path &path, bool is_directory) = 0; }; // Executes a function for each entry in a directory (except "." and ".."). @@ -226,8 +229,7 @@ class DirectoryEntryConsumer { // false otherwise. // // See DirectoryEntryConsumer for more details. -void ForEachDirectoryEntry(const std::string &path, - DirectoryEntryConsumer *consume); +void ForEachDirectoryEntry(const Path &path, DirectoryEntryConsumer *consumer); #if defined(_WIN32) || defined(__CYGWIN__) std::wstring GetCwdW(); @@ -257,15 +259,6 @@ class DirectoryEntryConsumerW { // file. void GetAllFilesUnderW(const std::wstring &path, std::vector *result); - -// Visible for testing only. -typedef void (*_ForEachDirectoryEntryW)(const std::wstring &path, - DirectoryEntryConsumerW *consume); - -// Visible for testing only. -void _GetAllFilesUnderW(const std::wstring &path, - std::vector *result, - _ForEachDirectoryEntryW walk_entries); #endif // defined(_WIN32) || defined(__CYGWIN__) } // namespace blaze_util diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index feae70e09c2d64..05f169e24e3c50 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -122,46 +123,46 @@ static bool MakeDirectories(const string &path, mode_t mode, bool childmost) { return stat_succeeded; } - -string CreateTempDir(const std::string &prefix) { - std::string parent = Dirname(prefix); +Path CreateSiblingTempDir(const Path &other_path) { // Need parent to exist first. + Path parent = other_path.GetParent(); if (!blaze_util::PathExists(parent) && !blaze_util::MakeDirectories(parent, 0777)) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) - << "couldn't create '" << parent << "': " - << blaze_util::GetLastErrorString(); + << "couldn't create '" << parent.AsPrintablePath() + << "': " << blaze_util::GetLastErrorString(); } - std::string result(prefix + "XXXXXX"); - if (mkdtemp(&result[0]) == nullptr) { - std::string err = GetLastErrorString(); + std::string path(other_path.AsNativePath() + ".tmp.XXXXXX"); + if (mkdtemp(&path.data()[0]) == nullptr) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "could not create temporary directory under " << parent - << " to extract install base into (" << err << ")"; + << "could not create temporary directory under " + << parent.AsPrintablePath() << " to extract install base into (" + << GetLastErrorString() << ")"; } // There's no better way to get the current umask than to set and reset it. const mode_t um = umask(0); umask(um); - chmod(result.c_str(), 0777 & ~um); + chmod(path.c_str(), 0777 & ~um); - return result; + return Path(path); } -static bool RemoveDirRecursively(const std::string &path) { +static bool RemoveDirRecursively(const Path &path) { DIR *dir; - if ((dir = opendir(path.c_str())) == nullptr) { + if ((dir = opendir(path.AsNativePath().c_str())) == nullptr) { return false; } struct dirent *ent; while ((ent = readdir(dir)) != nullptr) { - if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + string name = ent->d_name; + if (name == "." || name == "..") { continue; } - if (!RemoveRecursively(blaze_util::JoinPath(path, ent->d_name))) { + if (!RemoveRecursively(path.GetRelative(name))) { closedir(dir); return false; } @@ -171,12 +172,12 @@ static bool RemoveDirRecursively(const std::string &path) { return false; } - return rmdir(path.c_str()) == 0; + return rmdir(path.AsNativePath().c_str()) == 0; } -bool RemoveRecursively(const std::string &path) { +bool RemoveRecursively(const Path &path) { struct stat stat_buf; - if (lstat(path.c_str(), &stat_buf) == -1) { + if (lstat(path.AsNativePath().c_str(), &stat_buf) == -1) { // Non-existent is good enough. return errno == ENOENT; } @@ -334,8 +335,9 @@ int WriteToStdOutErr(const void *data, size_t size, bool to_stdout) { : WriteResult::OTHER_ERROR); } -int RenameDirectory(const std::string &old_name, const std::string &new_name) { - if (rename(old_name.c_str(), new_name.c_str()) == 0) { +int RenameDirectory(const Path &old_path, const Path &new_path) { + if (rename(old_path.AsNativePath().c_str(), + new_path.AsNativePath().c_str()) == 0) { return kRenameDirectorySuccess; } else { if (errno == ENOTEMPTY || errno == EEXIST) { @@ -346,15 +348,16 @@ int RenameDirectory(const std::string &old_name, const std::string &new_name) { } } -bool ReadDirectorySymlink(const blaze_util::Path &name, string *result) { +bool ReadDirectorySymlink(const blaze_util::Path &symlink, + blaze_util::Path *result) { char buf[PATH_MAX + 1]; - int len = readlink(name.AsNativePath().c_str(), buf, PATH_MAX); + int len = readlink(symlink.AsNativePath().c_str(), buf, PATH_MAX); if (len < 0) { return false; } buf[len] = 0; - *result = buf; + *result = Path(buf); return true; } @@ -527,22 +530,22 @@ bool ChangeDirectory(const string& path) { return chdir(path.c_str()) == 0; } -void ForEachDirectoryEntry(const string &path, - DirectoryEntryConsumer *consume) { +void ForEachDirectoryEntry(const Path &path, DirectoryEntryConsumer *consumer) { DIR *dir; struct dirent *ent; - if ((dir = opendir(path.c_str())) == nullptr) { + if ((dir = opendir(path.AsNativePath().c_str())) == nullptr) { // This is not a directory or it cannot be opened. return; } while ((ent = readdir(dir)) != nullptr) { - if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + string name(ent->d_name); + if (name == "." || name == "..") { continue; } - string filename(blaze_util::JoinPath(path, ent->d_name)); + Path child_path = path.GetRelative(name); bool is_directory; // 'd_type' field isn't part of the POSIX spec. #ifdef _DIRENT_HAVE_D_TYPE @@ -552,18 +555,18 @@ void ForEachDirectoryEntry(const string &path, #endif { struct stat buf; - if (lstat(filename.c_str(), &buf) == -1) { + if (lstat(child_path.AsNativePath().c_str(), &buf) == -1) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) - << "stat failed for filename '" << filename + << "stat failed for filename '" << child_path.AsPrintablePath() << "': " << GetLastErrorString(); } is_directory = S_ISDIR(buf.st_mode); } - consume->Consume(filename, is_directory); + consumer->Consume(child_path, is_directory); } closedir(dir); - } +} } // namespace blaze_util diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 5bd94f36a2d86d..fa122990076642 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -1,3 +1,4 @@ +#include "src/main/cpp/util/file_platform.h" // Copyright 2016 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -359,25 +360,9 @@ int WriteToStdOutErr(const void* data, size_t size, bool to_stdout) { } } -int RenameDirectory(const std::string& old_name, const std::string& new_name) { - wstring wold_name; - string error; - if (!AsAbsoluteWindowsPath(old_name, &wold_name, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "RenameDirectory(" << old_name << ", " << new_name - << "): AsAbsoluteWindowsPath(" << old_name << ") failed: " << error; - return kRenameDirectoryFailureOtherError; - } - - wstring wnew_name; - if (!AsAbsoluteWindowsPath(new_name, &wnew_name, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "RenameDirectory(" << old_name << ", " << new_name - << "): AsAbsoluteWindowsPath(" << new_name << ") failed: " << error; - return kRenameDirectoryFailureOtherError; - } - - if (!::MoveFileExW(wold_name.c_str(), wnew_name.c_str(), +int RenameDirectory(const Path& old_path, const Path& new_path) { + if (!::MoveFileExW(old_path.AsNativePath().c_str(), + new_path.AsNativePath().c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_FAIL_IF_NOT_TRACKABLE | MOVEFILE_WRITE_THROUGH)) { return GetLastError() == ERROR_ALREADY_EXISTS @@ -447,12 +432,13 @@ static bool RealPath(const WCHAR* path, unique_ptr* result = nullptr) { } } -bool ReadDirectorySymlink(const blaze_util::Path& name, string* result) { +bool ReadDirectorySymlink(const blaze_util::Path& symlink, + blaze_util::Path* result) { unique_ptr result_ptr; - if (!RealPath(name.AsNativePath().c_str(), &result_ptr)) { + if (!RealPath(symlink.AsNativePath().c_str(), &result_ptr)) { return false; } - *result = WstringToCstring(RemoveUncPrefixMaybe(result_ptr.get())); + *result = Path(WstringToCstring(RemoveUncPrefixMaybe(result_ptr.get()))); return true; } @@ -654,14 +640,16 @@ bool MakeDirectories(const Path& path, unsigned int mode) { return MakeDirectoriesW(path.AsNativePath(), mode); } -string CreateTempDir(const std::string &prefix) { - string result = prefix + blaze_util::ToString(GetCurrentProcessId()); - if (!blaze_util::MakeDirectories(result, 0777)) { +Path CreateSiblingTempDir(const Path& other_path) { + Path path = other_path.GetParent().GetRelative( + other_path.GetBaseName() + ".tmp." + + blaze_util::ToString(GetCurrentProcessId())); + if (!blaze_util::MakeDirectories(path, 0777)) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) - << "couldn't create '" << result + << "couldn't create '" << path.AsPrintablePath() << "': " << blaze_util::GetLastErrorString(); } - return result; + return path; } static bool RemoveContents(wstring path) { @@ -726,8 +714,12 @@ static bool RemoveRecursivelyW(const wstring& path) { } } -bool RemoveRecursively(const string& path) { - return RemoveRecursivelyW(Path(path).AsNativePath()); +bool RemoveRecursively(const std::string& path) { + return RemoveRecursively(Path(path)); +} + +bool RemoveRecursively(const Path& path) { + return RemoveRecursivelyW(path.AsNativePath()); } static inline void ToLowerW(WCHAR* p) { @@ -775,30 +767,8 @@ bool ChangeDirectory(const string& path) { } return ::SetCurrentDirectoryA(spath.c_str()) == TRUE; } - -class DirectoryTreeWalkerW : public DirectoryEntryConsumerW { - public: - DirectoryTreeWalkerW(vector* files, - _ForEachDirectoryEntryW walk_entries) - : _files(files), _walk_entries(walk_entries) {} - - void Consume(const wstring& path, bool follow_directory) override { - if (follow_directory) { - Walk(path); - } else { - _files->push_back(path); - } - } - - void Walk(const wstring& path) { _walk_entries(path, this); } - - private: - vector* _files; - _ForEachDirectoryEntryW _walk_entries; -}; - -void ForEachDirectoryEntryW(const wstring& path, - DirectoryEntryConsumerW* consume) { +static void ForEachDirectoryEntryW(const wstring& path, + DirectoryEntryConsumerW* consumer) { wstring wpath; if (path.empty() || IsDevNull(path.c_str())) { return; @@ -831,37 +801,44 @@ void ForEachDirectoryEntryW(const wstring& path, do { if (kDot != metadata.cFileName && kDotDot != metadata.cFileName) { wstring wname = wpath + metadata.cFileName; - wstring name(/* omit prefix */ 4 + wname.c_str()); + wstring name(wname.substr(kUncPrefix.length())); bool is_dir = (metadata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; bool is_junc = (metadata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0; - consume->Consume(name, is_dir && !is_junc); + consumer->Consume(name, is_dir && !is_junc); } } while (::FindNextFileW(handle, &metadata)); ::FindClose(handle); } -void GetAllFilesUnderW(const wstring& path, vector* result) { - _GetAllFilesUnderW(path, result, &ForEachDirectoryEntryW); -} +class DirectoryTreeWalkerW : public DirectoryEntryConsumerW { + public: + explicit DirectoryTreeWalkerW(vector* files) : files(files) {} + + void Consume(const wstring& path, bool is_directory) override { + if (is_directory) { + Walk(path); + } else { + files->push_back(path); + } + } -void _GetAllFilesUnderW(const wstring& path, vector* result, - _ForEachDirectoryEntryW walk_entries) { - DirectoryTreeWalkerW(result, walk_entries).Walk(path); + void Walk(const wstring& path) { ForEachDirectoryEntryW(path, this); } + + private: + vector* files; +}; + +void GetAllFilesUnderW(const wstring& path, vector* result) { + DirectoryTreeWalkerW(result).Walk(path); } -void ForEachDirectoryEntry(const string &path, - DirectoryEntryConsumer *consume) { - wstring wpath; - if (path.empty() || IsDevNull(path.c_str())) { +void ForEachDirectoryEntry(const Path& path, DirectoryEntryConsumer* consumer) { + if (path.IsEmpty() || path.IsNull()) { return; } - string error; - if (!AsWindowsPath(path, &wpath, &error)) { - BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) - << "ForEachDirectoryEntry(" << path - << "): AsWindowsPath failed: " << GetLastErrorString(); - } + + wstring wpath = path.AsNativePath(); static const wstring kUncPrefix(L"\\\\?\\"); static const wstring kDot(L"."); @@ -882,12 +859,12 @@ void ForEachDirectoryEntry(const string &path, do { if (kDot != metadata.cFileName && kDotDot != metadata.cFileName) { - wstring wname = wpath + metadata.cFileName; - string name(WstringToCstring(/* omit prefix */ 4 + wname.c_str())); + string name = WstringToCstring(metadata.cFileName); + Path child_path = path.GetRelative(name); bool is_dir = (metadata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; bool is_junc = (metadata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0; - consume->Consume(name, is_dir && !is_junc); + consumer->Consume(child_path, is_dir && !is_junc); } } while (::FindNextFileW(handle, &metadata)); ::FindClose(handle); diff --git a/src/main/cpp/util/path_platform.h b/src/main/cpp/util/path_platform.h index ccf69815d349e5..4a1a7024f12b7b 100644 --- a/src/main/cpp/util/path_platform.h +++ b/src/main/cpp/util/path_platform.h @@ -42,6 +42,8 @@ class Path { Path GetParent() const; + std::string GetBaseName() const; + // Returns a printable string representing this path. // Only use when printing user messages, do not pass to filesystem API // functions. @@ -84,11 +86,11 @@ std::string ConvertPath(const std::string &path); // See https://github.com/bazelbuild/bazel/issues/2576 std::string PathAsJvmFlag(const std::string &path); -// Compares two absolute paths. Necessary because the same path can have -// multiple different names under msys2: "C:\foo\bar" or "C:/foo/bar" -// (Windows-style) and "/c/foo/bar" (msys2 style). Returns if the paths are -// equal. -bool CompareAbsolutePaths(const std::string &a, const std::string &b); +// Returns whether two paths are equivalent. +// On Windows, there are multiple ways to represent the same path, e.g. +// "C:\foo\bar" or "C:/foo/bar" or "/c/foo/bar" (msys2 style). +// On Unix, this is a simple string comparison. +bool ArePathsEquivalent(const blaze_util::Path &a, const blaze_util::Path &b); // Split a path to dirname and basename parts. std::pair SplitPath(const std::string &path); diff --git a/src/main/cpp/util/path_posix.cc b/src/main/cpp/util/path_posix.cc index a9774277cc8df4..96d6fac63b8847 100644 --- a/src/main/cpp/util/path_posix.cc +++ b/src/main/cpp/util/path_posix.cc @@ -31,7 +31,7 @@ std::string ConvertPath(const std::string &path) { return path; } std::string PathAsJvmFlag(const std::string &path) { return path; } -bool CompareAbsolutePaths(const std::string &a, const std::string &b) { +bool ArePathsEquivalent(const blaze_util::Path &a, const blaze_util::Path &b) { return a == b; } @@ -160,6 +160,8 @@ Path Path::GetRelative(const std::string &r) const { Path Path::Canonicalize() const { return Path(MakeCanonical(path_.c_str())); } +std::string Path::GetBaseName() const { return SplitPath(path_).second; } + Path Path::GetParent() const { return Path(SplitPath(path_).first); } std::string Path::AsPrintablePath() const { return path_; } diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc index b241942ef999c8..eabae1f6c6df2e 100644 --- a/src/main/cpp/util/path_windows.cc +++ b/src/main/cpp/util/path_windows.cc @@ -116,8 +116,9 @@ std::string MakeAbsoluteAndResolveEnvvars(const std::string& path) { return MakeAbsolute(std::string(resolved.get())); } -bool CompareAbsolutePaths(const std::string& a, const std::string& b) { - return ConvertPath(a) == ConvertPath(b); +bool ArePathsEquivalent(const blaze_util::Path& a, const blaze_util::Path& b) { + return ConvertPath(WstringToCstring(a.AsNativePath())) == + ConvertPath(WstringToCstring(b.AsNativePath())); } std::string PathAsJvmFlag(const std::string& path) { @@ -513,6 +514,10 @@ Path Path::Canonicalize() const { Path Path::GetParent() const { return Path(SplitPathW(path_).first); } +std::string Path::GetBaseName() const { + return WstringToCstring(SplitPathW(path_).second); +} + bool Path::IsNull() const { return path_ == L"NUL"; } bool Path::Contains(const char c) const { diff --git a/src/test/cpp/option_processor_test.cc b/src/test/cpp/option_processor_test.cc index 4cdeb3133d5e9f..46e08fcd1c42e1 100644 --- a/src/test/cpp/option_processor_test.cc +++ b/src/test/cpp/option_processor_test.cc @@ -47,15 +47,7 @@ class OptionProcessorTest : public ::testing::Test { } void TearDown() override { - // TODO(bazel-team): The code below deletes all the files in the workspace - // but it intentionally skips directories. As a consequence, there may be - // empty directories from test to test. Remove this once - // blaze_util::DeleteDirectories(path) exists. - std::vector files_in_workspace; - blaze_util::GetAllFilesUnder(workspace_, &files_in_workspace); - for (const std::string& file : files_in_workspace) { - blaze_util::UnlinkPath(file); - } + blaze_util::RemoveRecursively(blaze_util::Path(workspace_)); } void FailedSplitCommandLineTest(const std::vector& args, diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc index 11151540237d20..9efd03a1bd1013 100644 --- a/src/test/cpp/rc_file_test.cc +++ b/src/test/cpp/rc_file_test.cc @@ -98,27 +98,10 @@ class RcFileTest : public ::testing::Test { } void TearDown() override { - // TODO(bazel-team): The code below deletes all the files in the workspace - // and other rc-related directories, but it intentionally skips directories. - // As a consequence, there may be empty directories from test to test. - // Remove this once blaze_util::DeleteDirectories(path) exists. - std::vector files; - blaze_util::GetAllFilesUnder(workspace_, &files); - for (const std::string& file : files) { - blaze_util::UnlinkPath(file); - } - blaze_util::GetAllFilesUnder(cwd_, &files); - for (const std::string& file : files) { - blaze_util::UnlinkPath(file); - } - blaze_util::GetAllFilesUnder(home_, &files); - for (const std::string& file : files) { - blaze_util::UnlinkPath(file); - } - blaze_util::GetAllFilesUnder(binary_dir_, &files); - for (const std::string& file : files) { - blaze_util::UnlinkPath(file); - } + blaze_util::RemoveRecursively(blaze_util::Path(workspace_)); + blaze_util::RemoveRecursively(blaze_util::Path(cwd_)); + blaze_util::RemoveRecursively(blaze_util::Path(home_)); + blaze_util::RemoveRecursively(blaze_util::Path(binary_dir_)); } bool SetUpSystemRcFile(const std::string& contents, diff --git a/src/test/cpp/startup_options_test.cc b/src/test/cpp/startup_options_test.cc index f9167dd4043295..3e7ba3aacdc5e6 100644 --- a/src/test/cpp/startup_options_test.cc +++ b/src/test/cpp/startup_options_test.cc @@ -99,7 +99,8 @@ TEST_F(StartupOptionsTest, OutputRootPreferTestTmpdirIfSet) { SetEnv("TEST_TMPDIR", "/nonexistent/tmpdir"); ReinitStartupOptions(); - ASSERT_EQ("/nonexistent/tmpdir", startup_options_->output_root); + ASSERT_EQ(blaze_util::Path("/nonexistent/tmpdir"), + startup_options_->output_root); } TEST_F(StartupOptionsTest, @@ -109,7 +110,8 @@ TEST_F(StartupOptionsTest, UnsetEnv("TEST_TMPDIR"); ReinitStartupOptions(); - ASSERT_EQ("/nonexistent/cache/bazel", startup_options_->output_root); + ASSERT_EQ(blaze_util::Path("/nonexistent/cache/bazel"), + startup_options_->output_root); } TEST_F(StartupOptionsTest, OutputRootUseHomeDirectory) { @@ -118,7 +120,8 @@ TEST_F(StartupOptionsTest, OutputRootUseHomeDirectory) { UnsetEnv("XDG_CACHE_HOME"); ReinitStartupOptions(); - ASSERT_EQ("/nonexistent/home/.cache/bazel", startup_options_->output_root); + ASSERT_EQ(blaze_util::Path("/nonexistent/home/.cache/bazel"), + startup_options_->output_root); } TEST_F(StartupOptionsTest, OutputRootIsAbsoluteAndNotShellExpanded) { @@ -127,29 +130,32 @@ TEST_F(StartupOptionsTest, OutputRootIsAbsoluteAndNotShellExpanded) { SetEnv("HOME", "~/home$(echo baz)"); ReinitStartupOptions(); - ASSERT_EQ(blaze_util::GetCwd() + "/~/\"$foo/test\"", + ASSERT_EQ(blaze_util::Path(blaze_util::GetCwd() + "/~/\"$foo/test\""), startup_options_->output_root); UnsetEnv("TEST_TMPDIR"); ReinitStartupOptions(); - ASSERT_EQ(blaze_util::GetCwd() + "/~/cache${bar}/bazel", + ASSERT_EQ(blaze_util::Path(blaze_util::GetCwd() + "/~/cache${bar}/bazel"), startup_options_->output_root); UnsetEnv("XDG_CACHE_HOME"); ReinitStartupOptions(); - ASSERT_EQ(blaze_util::GetCwd() + "/~/home$(echo baz)/.cache/bazel", + ASSERT_EQ(blaze_util::Path(blaze_util::GetCwd() + + "/~/home$(echo baz)/.cache/bazel"), startup_options_->output_root); } #endif // __linux TEST_F(StartupOptionsTest, OutputUserRootTildeExpansion) { #if defined(_WIN32) - std::string home = "C:/nonexistent/home/"; + std::string home_str = "C:/nonexistent/home/"; #else - std::string home = "/nonexistent/home/"; + std::string home_str = "/nonexistent/home/"; #endif - SetEnv("HOME", home); + SetEnv("HOME", home_str); + + blaze_util::Path home(home_str); std::string error; @@ -164,8 +170,7 @@ TEST_F(StartupOptionsTest, OutputUserRootTildeExpansion) { ASSERT_EQ(blaze_exit_code::SUCCESS, ec) << "ProcessArgs failed with error " << error; - EXPECT_EQ(blaze_util::JoinPath(home, "test"), - startup_options_->output_user_root); + EXPECT_EQ(home.GetRelative("test"), startup_options_->output_user_root); } { diff --git a/src/test/cpp/util/BUILD b/src/test/cpp/util/BUILD index 0b1bf303c9a454..03ba945e3ae7b5 100644 --- a/src/test/cpp/util/BUILD +++ b/src/test/cpp/util/BUILD @@ -31,6 +31,7 @@ cc_test( deps = [ ":test_util", "//src/main/cpp/util:filesystem", + "@abseil-cpp//absl/strings", "@com_google_googletest//:gtest_main", ] + select({ "//src/conditions:windows": [ diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc index c934bf51e083e6..467503ee59e028 100644 --- a/src/test/cpp/util/file_posix_test.cc +++ b/src/test/cpp/util/file_posix_test.cc @@ -13,9 +13,12 @@ // limitations under the License. #include #include +#include #include -#include +#include +#include +#include #include "src/main/cpp/util/file.h" #include "src/main/cpp/util/file_platform.h" @@ -23,10 +26,11 @@ #include "src/main/cpp/util/path_platform.h" #include "src/test/cpp/util/test_util.h" #include "googletest/include/gtest/gtest.h" +#include "absl/strings/match.h" namespace blaze_util { -using std::pair; +using std::map; using std::string; using std::vector; @@ -34,6 +38,10 @@ static bool Symlink(const string& old_path, const string& new_path) { return symlink(old_path.c_str(), new_path.c_str()) == 0; } +static bool Symlink(const Path& old_path, const Path& new_path) { + return Symlink(old_path.AsNativePath(), new_path.AsNativePath()); +} + static bool CreateEmptyFile(const string& path) { // From the man page of open (man 2 open): // int open(const char *pathname, int flags, mode_t mode); @@ -48,37 +56,6 @@ static bool CreateEmptyFile(const string& path) { return close(fd) == 0; } -void MockDirectoryListingFunction(const string& path, - DirectoryEntryConsumer* consume) { - if (path == "root") { - consume->Consume("root/file1", false); - consume->Consume("root/dir2", true); - consume->Consume("root/dir1", true); - } else if (path == "root/dir1") { - consume->Consume("root/dir1/dir3", true); - consume->Consume("root/dir1/file2", false); - } else if (path == "root/dir2") { - consume->Consume("root/dir2/file3", false); - } else if (path == "root/dir1/dir3") { - consume->Consume("root/dir1/dir3/file4", false); - consume->Consume("root/dir1/dir3/file5", false); - } else { - // Unexpected path - GTEST_FAIL(); - } -} - -TEST(FilePosixTest, GetAllFilesUnder) { - vector result; - _GetAllFilesUnder("root", &result, &MockDirectoryListingFunction); - std::sort(result.begin(), result.end()); - - vector expected({"root/dir1/dir3/file4", "root/dir1/dir3/file5", - "root/dir1/file2", "root/dir2/file3", - "root/file1"}); - ASSERT_EQ(expected, result); -} - TEST(FilePosixTest, MakeDirectories) { const char* tmp_dir = getenv("TEST_TMPDIR"); ASSERT_STRNE(tmp_dir, nullptr); @@ -227,136 +204,102 @@ TEST(FilePosixTest, ChangeDirectory) { class MockDirectoryEntryConsumer : public DirectoryEntryConsumer { public: - void Consume(const std::string& name, bool is_directory) override { - entries.push_back(pair(name, is_directory)); + void Consume(const Path& path, bool is_directory) override { + entries[path] = is_directory; } - vector > entries; + map entries; }; TEST(FilePosixTest, ForEachDirectoryEntry) { // Get the test's temp dir. char* tmpdir_cstr = getenv("TEST_TMPDIR"); - ASSERT_FALSE(tmpdir_cstr == nullptr); - string tempdir(tmpdir_cstr); - ASSERT_FALSE(tempdir.empty()); - if (tempdir.back() == '/') { - tempdir = tempdir.substr(0, tempdir.size() - 1); - } - - // Create the root directory for the mock directory tree. - string root = tempdir + "/FilePosixTest.ForEachDirectoryEntry.root"; - ASSERT_EQ(0, mkdir(root.c_str(), 0700)); + ASSERT_NE(tmpdir_cstr, nullptr); + Path tmpdir(tmpdir_cstr); + ASSERT_TRUE(PathExists(tmpdir)); // Names of mock files and directories. - string dir = root + "/dir"; - string file = root + "/file"; - string dir_sym = root + "/dir_sym"; - string file_sym = root + "/file_sym"; - string subfile = dir + "/subfile"; - string subfile_through_sym = dir_sym + "/subfile"; + Path root = tmpdir.GetRelative("FilePosixTest.ForEachDirectoryEntry.root"); + Path dir = root.GetRelative("dir"); + Path file = root.GetRelative("file"); + Path dir_sym = root.GetRelative("dir_sym"); + Path file_sym = root.GetRelative("file_sym"); + Path subfile = dir.GetRelative("subfile"); + Path subfile_through_sym = dir_sym.GetRelative("subfile"); // Create mock directory, file, and symlinks. - AutoFd fd(open(file.c_str(), O_CREAT, 0700)); - ASSERT_TRUE(fd.IsOpen()); - fd.Close(); - ASSERT_EQ(0, mkdir(dir.c_str(), 0700)); - ASSERT_EQ(0, symlink("dir", dir_sym.c_str())); - ASSERT_EQ(0, symlink("file", file_sym.c_str())); - fd = open(subfile.c_str(), O_CREAT, 0700); - ASSERT_TRUE(fd.IsOpen()); - fd.Close(); - - // Assert that stat'ing the symlinks (with following them) point to the - // right filesystem entry types. - struct stat stat_buf; - ASSERT_EQ(0, stat(dir_sym.c_str(), &stat_buf)); - ASSERT_TRUE(S_ISDIR(stat_buf.st_mode)); - ASSERT_EQ(0, stat(file_sym.c_str(), &stat_buf)); - ASSERT_FALSE(S_ISDIR(stat_buf.st_mode)); + ASSERT_TRUE(MakeDirectories(root, 0700)); + ASSERT_TRUE(WriteFile("", file)); + ASSERT_TRUE(MakeDirectories(dir, 0700)); + ASSERT_TRUE(Symlink(dir, dir_sym)); + ASSERT_TRUE(Symlink(file, file_sym)); + ASSERT_TRUE(WriteFile("", subfile)); // Actual test: list the directory. - MockDirectoryEntryConsumer consumer; - ForEachDirectoryEntry(root, &consumer); - ASSERT_EQ(size_t(4), consumer.entries.size()); - - // Sort the collected directory entries. - struct { - bool operator()(const pair& a, - const pair& b) { - return a.first < b.first; - } - } sort_pairs; - - std::sort(consumer.entries.begin(), consumer.entries.end(), sort_pairs); - - // Assert that the directory entries have the right name and type. - pair expected; - expected = pair(dir, true); - ASSERT_EQ(expected, consumer.entries[0]); - expected = pair(dir_sym, false); - ASSERT_EQ(expected, consumer.entries[1]); - expected = pair(file, false); - ASSERT_EQ(expected, consumer.entries[2]); - expected = pair(file_sym, false); - ASSERT_EQ(expected, consumer.entries[3]); + MockDirectoryEntryConsumer consumer1; + ForEachDirectoryEntry(root, &consumer1); + map expected1; + expected1[dir] = true; + expected1[dir_sym] = false; + expected1[file] = false; + expected1[file_sym] = false; + EXPECT_EQ(expected1, consumer1.entries); // Actual test: list a directory symlink. - consumer.entries.clear(); - ForEachDirectoryEntry(dir_sym, &consumer); - ASSERT_EQ(size_t(1), consumer.entries.size()); - expected = pair(subfile_through_sym, false); - ASSERT_EQ(expected, consumer.entries[0]); + MockDirectoryEntryConsumer consumer2; + ForEachDirectoryEntry(dir_sym, &consumer2); + map expected2; + expected2[subfile_through_sym] = false; + EXPECT_EQ(expected2, consumer2.entries); // Actual test: list a path that's actually a file, not a directory. - consumer.entries.clear(); - ForEachDirectoryEntry(file, &consumer); - ASSERT_TRUE(consumer.entries.empty()); + MockDirectoryEntryConsumer consumer3; + ForEachDirectoryEntry(file, &consumer3); + EXPECT_TRUE(consumer3.entries.empty()); // Cleanup: delete mock directory tree. - rmdir(subfile.c_str()); - rmdir(dir.c_str()); - unlink(dir_sym.c_str()); - unlink(file.c_str()); - unlink(file_sym.c_str()); - rmdir(root.c_str()); + EXPECT_TRUE(blaze_util::RemoveRecursively(root)); } TEST(FileTest, TestRemoveRecursivelyPosix) { const char* tempdir_cstr = getenv("TEST_TMPDIR"); ASSERT_NE(tempdir_cstr, nullptr); - string tempdir(tempdir_cstr); + Path tempdir(tempdir_cstr); ASSERT_TRUE(PathExists(tempdir)); - string unwritable_dir(JoinPath(tempdir, "test_rmr_unwritable")); + Path unwritable_dir = tempdir.GetRelative("test_rmr_unwritable"); EXPECT_TRUE(MakeDirectories(unwritable_dir, 0700)); - EXPECT_TRUE(WriteFile("junkdata", 8, JoinPath(unwritable_dir, "file"))); - ASSERT_EQ(0, chmod(unwritable_dir.c_str(), 0500)); + EXPECT_TRUE(WriteFile("junkdata", 8, unwritable_dir.GetRelative("file"))); + ASSERT_EQ(0, chmod(unwritable_dir.AsNativePath().c_str(), 0500)); EXPECT_FALSE(RemoveRecursively(unwritable_dir)); - string symlink_target_dir(JoinPath(tempdir, "test_rmr_symlink_target_dir")); + Path symlink_target_dir = tempdir.GetRelative("test_rmr_symlink_target_dir"); EXPECT_TRUE(MakeDirectories(symlink_target_dir, 0700)); - string symlink_target_dir_file(JoinPath(symlink_target_dir, "file")); + Path symlink_target_dir_file = symlink_target_dir.GetRelative("file"); EXPECT_TRUE(WriteFile("junkdata", 8, symlink_target_dir_file)); - string symlink_dir(JoinPath(tempdir, "test_rmr_symlink_dir")); - EXPECT_EQ(0, symlink(symlink_target_dir.c_str(), symlink_dir.c_str())); + Path symlink_dir = tempdir.GetRelative("test_rmr_symlink_dir"); + EXPECT_EQ(0, symlink(symlink_target_dir.AsNativePath().c_str(), + symlink_dir.AsNativePath().c_str())); EXPECT_TRUE(RemoveRecursively(symlink_dir)); EXPECT_FALSE(PathExists(symlink_dir)); EXPECT_TRUE(PathExists(symlink_target_dir)); EXPECT_TRUE(PathExists(symlink_target_dir_file)); - string dir_with_symlinks(JoinPath(tempdir, "test_rmr_dir_w_symlinks")); + Path dir_with_symlinks = tempdir.GetRelative("test_rmr_dir_w_symlinks"); EXPECT_TRUE(MakeDirectories(dir_with_symlinks, 0700)); - string file_symlink_target(JoinPath(tempdir, "test_rmr_dir_w_symlinks_file")); + Path file_symlink_target = + tempdir.GetRelative("test_rmr_dir_w_symlinks_file"); EXPECT_TRUE(WriteFile("junkdata", 8, file_symlink_target)); - EXPECT_EQ(0, symlink(file_symlink_target.c_str(), - JoinPath(dir_with_symlinks, "file").c_str())); - string dir_symlink_target(JoinPath(tempdir, "test_rmr_dir_w_symlinks_dir")); + EXPECT_EQ( + 0, symlink(file_symlink_target.AsNativePath().c_str(), + dir_with_symlinks.GetRelative("file").AsNativePath().c_str())); + Path dir_symlink_target = tempdir.GetRelative("test_rmr_dir_w_symlinks_dir"); EXPECT_TRUE(MakeDirectories(dir_symlink_target, 0700)); - string dir_symlink_target_file(JoinPath(dir_symlink_target, "file")); + Path dir_symlink_target_file = dir_symlink_target.GetRelative("file"); EXPECT_TRUE(WriteFile("junkdata", 8, dir_symlink_target_file)); - EXPECT_EQ(0, symlink(dir_symlink_target.c_str(), - JoinPath(dir_with_symlinks, "dir").c_str())); + EXPECT_EQ( + 0, symlink(dir_symlink_target.AsNativePath().c_str(), + dir_with_symlinks.GetRelative("dir").AsNativePath().c_str())); EXPECT_TRUE(RemoveRecursively(dir_with_symlinks)); EXPECT_FALSE(PathExists(dir_with_symlinks)); EXPECT_TRUE(PathExists(dir_symlink_target)); @@ -364,20 +307,22 @@ TEST(FileTest, TestRemoveRecursivelyPosix) { EXPECT_TRUE(PathExists(file_symlink_target)); } -TEST(FileTest, TestCreateTempDirDoesntClobberParentPerms) { +TEST(FileTest, TestCreatSiblingTempDirDoesntClobberParentPerms) { const char* tempdir_cstr = getenv("TEST_TMPDIR"); ASSERT_NE(tempdir_cstr, nullptr); - string tempdir(tempdir_cstr); + Path tempdir(tempdir_cstr); ASSERT_TRUE(PathExists(tempdir)); - string existing_parent_dir(JoinPath(tempdir, "existing")); + Path existing_parent_dir = tempdir.GetRelative("existing"); ASSERT_TRUE(MakeDirectories(existing_parent_dir, 0700)); - string prefix(JoinPath(existing_parent_dir, "mytmp")); - string result(CreateTempDir(prefix)); - ASSERT_EQ(0, result.find(prefix)); - EXPECT_TRUE(PathExists(result)); + Path other_dir = existing_parent_dir.GetRelative("other"); + Path temp_dir = CreateSiblingTempDir(other_dir); + ASSERT_EQ(other_dir.GetParent(), temp_dir.GetParent()); + ASSERT_TRUE(absl::StartsWith(temp_dir.GetBaseName(), + other_dir.GetBaseName() + ".tmp.")); + EXPECT_TRUE(PathExists(temp_dir)); struct stat filestat = {}; - ASSERT_EQ(0, stat(existing_parent_dir.c_str(), &filestat)); + ASSERT_EQ(0, stat(existing_parent_dir.AsNativePath().c_str(), &filestat)); ASSERT_EQ(mode_t(0700), filestat.st_mode & 0777); } diff --git a/src/test/cpp/util/file_test.cc b/src/test/cpp/util/file_test.cc index 5522765f3f1f1f..88432f19f6c13a 100644 --- a/src/test/cpp/util/file_test.cc +++ b/src/test/cpp/util/file_test.cc @@ -1,3 +1,4 @@ +#include // Copyright 2014 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -11,21 +12,22 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -#include "src/main/cpp/util/file.h" - #include #include #include #include -#include // unique_ptr +#include #include // NOLINT (to silence Google-internal linter) +#include +#include "src/main/cpp/util/file.h" #include "src/main/cpp/util/file_platform.h" #include "src/main/cpp/util/path.h" #include "src/main/cpp/util/path_platform.h" #include "src/test/cpp/util/test_util.h" #include "googletest/include/gtest/gtest.h" +#include "absl/strings/match.h" namespace blaze_util { @@ -184,38 +186,41 @@ TEST(FileTest, TestMtimeHandling) { ASSERT_FALSE(IsUntampered(file)); } -TEST(FileTest, TestCreateTempDir) { +TEST(FileTest, TestCreateSiblingTempDir) { const char* tempdir_cstr = getenv("TEST_TMPDIR"); EXPECT_NE(tempdir_cstr, nullptr); EXPECT_NE(tempdir_cstr[0], 0); - string tempdir(tempdir_cstr); - string tmpdir(tempdir_cstr); - - string prefix_in_existing_dir(JoinPath(tempdir, "foo.")); - string result_in_existing_dir(CreateTempDir(prefix_in_existing_dir)); - ASSERT_NE(result_in_existing_dir, prefix_in_existing_dir); - ASSERT_EQ(0, result_in_existing_dir.find(prefix_in_existing_dir)); - EXPECT_TRUE(PathExists(result_in_existing_dir)); - - string base_dir(JoinPath(tempdir, "doesntexistyet")); - ASSERT_FALSE(PathExists(base_dir)); - string prefix_in_new_dir(JoinPath(base_dir, "foo.")); - string result_in_new_dir(CreateTempDir(prefix_in_new_dir)); - ASSERT_NE(result_in_new_dir, prefix_in_new_dir); - ASSERT_EQ(0, result_in_new_dir.find(prefix_in_new_dir)); - EXPECT_TRUE(PathExists(result_in_new_dir)); + Path tempdir(tempdir_cstr); + + Path input_in_existing = tempdir.GetRelative("other"); + Path output_in_existing = CreateSiblingTempDir(input_in_existing); + ASSERT_NE(input_in_existing, output_in_existing); + ASSERT_EQ(input_in_existing.GetParent(), output_in_existing.GetParent()); + ASSERT_TRUE(absl::StartsWith(output_in_existing.GetBaseName(), + input_in_existing.GetBaseName() + ".tmp.")); + EXPECT_TRUE(PathExists(output_in_existing)); + + Path missing_dir = tempdir.GetRelative("doesntexistyet"); + ASSERT_FALSE(PathExists(missing_dir)); + Path input_in_missing = missing_dir.GetRelative("other"); + Path output_in_missing = CreateSiblingTempDir(input_in_missing); + ASSERT_NE(input_in_missing, output_in_missing); + ASSERT_EQ(input_in_missing.GetParent(), output_in_missing.GetParent()); + ASSERT_TRUE(absl::StartsWith(output_in_missing.GetBaseName(), + input_in_missing.GetBaseName() + ".tmp.")); + EXPECT_TRUE(PathExists(output_in_missing)); } TEST(FileTest, TestRenameDirectory) { const char* tempdir_cstr = getenv("TEST_TMPDIR"); EXPECT_NE(tempdir_cstr, nullptr); EXPECT_NE(tempdir_cstr[0], 0); - string tempdir(tempdir_cstr); + Path tempdir(tempdir_cstr); - string dir1(JoinPath(tempdir, "test_rename_dir/dir1")); - string dir2(JoinPath(tempdir, "test_rename_dir/dir2")); + Path dir1 = tempdir.GetRelative("test_rename_dir/dir1"); + Path dir2 = tempdir.GetRelative("test_rename_dir/dir2"); EXPECT_TRUE(MakeDirectories(dir1, 0700)); - string file1(JoinPath(dir1, "file1.txt")); + Path file1 = dir1.GetRelative("file1.txt"); EXPECT_TRUE(WriteFile("hello", 5, file1)); ASSERT_EQ(RenameDirectory(dir1, dir2), kRenameDirectorySuccess); @@ -225,54 +230,81 @@ TEST(FileTest, TestRenameDirectory) { ASSERT_EQ(RenameDirectory(dir2, dir1), kRenameDirectoryFailureNotEmpty); } -class CollectingDirectoryEntryConsumer : public DirectoryEntryConsumer { +class MockDirectoryEntryConsumer : public DirectoryEntryConsumer { public: - CollectingDirectoryEntryConsumer(const string& _rootname) - : rootname(_rootname) {} - - void Consume(const string& name, bool is_directory) override { - // Strip the path prefix up to the `rootname` to ease testing on all - // platforms. - size_t index = name.rfind(rootname); - string key = (index == string::npos) ? name : name.substr(index); - // Replace backslashes with forward slashes (necessary on Windows only). - std::replace(key.begin(), key.end(), '\\', '/'); - entries[key] = is_directory; + void Consume(const Path& path, bool is_directory) override { + entries[path] = is_directory; } - const string rootname; - std::map entries; + std::map entries; }; TEST(FileTest, ForEachDirectoryEntryTest) { - string tmpdir(getenv("TEST_TMPDIR")); - EXPECT_FALSE(tmpdir.empty()); - // Create a directory structure: + const char* tmpdir_cstr = getenv("TEST_TMPDIR"); + ASSERT_NE(tmpdir_cstr, nullptr); + Path tmpdir(tmpdir_cstr); + ASSERT_TRUE(PathExists(tmpdir)); // $TEST_TMPDIR/ // foo/ // bar/ // file3.txt // file1.txt // file2.txt - string rootdir(JoinPath(tmpdir, "foo")); - string file1(JoinPath(rootdir, "file1.txt")); - string file2(JoinPath(rootdir, "file2.txt")); - string subdir(JoinPath(rootdir, "bar")); - string file3(JoinPath(subdir, "file3.txt")); - - EXPECT_TRUE(MakeDirectories(subdir, 0700)); - EXPECT_TRUE(WriteFile("hello", 5, file1)); - EXPECT_TRUE(WriteFile("hello", 5, file2)); - EXPECT_TRUE(WriteFile("hello", 5, file3)); - - std::map expected; - expected["foo/file1.txt"] = false; - expected["foo/file2.txt"] = false; - expected["foo/bar"] = true; - - CollectingDirectoryEntryConsumer consumer("foo"); + Path rootdir = tmpdir.GetRelative("foo"); + Path file1 = rootdir.GetRelative("file1.txt"); + Path file2 = rootdir.GetRelative("file2.txt"); + Path subdir = rootdir.GetRelative("bar"); + Path file3 = subdir.GetRelative("file3.txt"); + + ASSERT_TRUE(MakeDirectories(subdir, 0700)); + ASSERT_TRUE(WriteFile("hello", 5, file1)); + ASSERT_TRUE(WriteFile("hello", 5, file2)); + ASSERT_TRUE(WriteFile("hello", 5, file3)); + + std::map expected; + expected[file1] = false; + expected[file2] = false; + expected[subdir] = true; + + MockDirectoryEntryConsumer consumer; ForEachDirectoryEntry(rootdir, &consumer); - ASSERT_EQ(consumer.entries, expected); + EXPECT_EQ(consumer.entries, expected); +} + +TEST(FilePosixTest, GetAllFilesUnder) { + const char* tmpdir_cstr = getenv("TEST_TMPDIR"); + ASSERT_NE(tmpdir_cstr, nullptr); + Path tmpdir(tmpdir_cstr); + ASSERT_TRUE(PathExists(tmpdir)); + + Path root = tmpdir.GetRelative("FilePosixTest.GetAllFilesUnder.root"); + Path file1 = root.GetRelative("file1"); + Path dir1 = root.GetRelative("dir1"); + Path dir2 = root.GetRelative("dir2"); + Path dir3 = dir1.GetRelative("dir3"); + Path file2 = dir1.GetRelative("file2"); + Path file3 = dir2.GetRelative("file3"); + Path file4 = dir3.GetRelative("file4"); + Path file5 = dir3.GetRelative("file5"); + + ASSERT_TRUE(MakeDirectories(root, 0700)); + ASSERT_TRUE(MakeDirectories(dir1, 0700)); + ASSERT_TRUE(MakeDirectories(dir2, 0700)); + ASSERT_TRUE(MakeDirectories(dir3, 0700)); + ASSERT_TRUE(WriteFile("", file1)); + ASSERT_TRUE(WriteFile("", file2)); + ASSERT_TRUE(WriteFile("", file3)); + ASSERT_TRUE(WriteFile("", file4)); + ASSERT_TRUE(WriteFile("", file5)); + + std::vector result; + GetAllFilesUnder(root, &result); + std::sort(result.begin(), result.end()); + + std::vector expected({file4, file5, file2, file3, file1}); + EXPECT_EQ(expected, result); + + EXPECT_TRUE(RemoveRecursively(root)); } TEST(FileTest, IsDevNullTest) { @@ -287,30 +319,30 @@ TEST(FileTest, IsDevNullTest) { TEST(FileTest, TestRemoveRecursively) { const char* tempdir_cstr = getenv("TEST_TMPDIR"); ASSERT_NE(tempdir_cstr, nullptr); - string tempdir(tempdir_cstr); + Path tempdir(tempdir_cstr); ASSERT_TRUE(PathExists(tempdir)); - string non_existent_dir(JoinPath(tempdir, "test_rmr_non_existent")); + Path non_existent_dir = tempdir.GetRelative("test_rmr_non_existent"); EXPECT_TRUE(RemoveRecursively(non_existent_dir)); EXPECT_FALSE(PathExists(non_existent_dir)); - string empty_dir(JoinPath(tempdir, "test_rmr_empty_dir")); + Path empty_dir = tempdir.GetRelative("test_rmr_empty_dir"); EXPECT_TRUE(MakeDirectories(empty_dir, 0700)); EXPECT_TRUE(RemoveRecursively(empty_dir)); EXPECT_FALSE(PathExists(empty_dir)); - string dir_with_content(JoinPath(tempdir, "test_rmr_dir_w_content")); + Path dir_with_content = tempdir.GetRelative("test_rmr_dir_w_content"); EXPECT_TRUE(MakeDirectories(dir_with_content, 0700)); - EXPECT_TRUE(WriteFile("junkdata", 8, JoinPath(dir_with_content, "file"))); - string subdir = JoinPath(dir_with_content, "dir"); + EXPECT_TRUE(WriteFile("junkdata", 8, dir_with_content.GetRelative("file"))); + Path subdir = dir_with_content.GetRelative("dir"); EXPECT_TRUE(MakeDirectories(subdir, 0700)); - string subsubdir = JoinPath(subdir, "dir"); + Path subsubdir = subdir.GetRelative("dir"); EXPECT_TRUE(MakeDirectories(subsubdir, 0700)); - EXPECT_TRUE(WriteFile("junkdata", 8, JoinPath(subsubdir, "deep_file"))); + EXPECT_TRUE(WriteFile("junkdata", 8, subsubdir.GetRelative("deep_file"))); EXPECT_TRUE(RemoveRecursively(dir_with_content)); EXPECT_FALSE(PathExists(dir_with_content)); - string regular_file(JoinPath(tempdir, "test_rmr_regular_file")); + Path regular_file = tempdir.GetRelative("test_rmr_regular_file"); EXPECT_TRUE(WriteFile("junkdata", 8, regular_file)); EXPECT_TRUE(RemoveRecursively(regular_file)); EXPECT_FALSE(PathExists(regular_file)); diff --git a/src/test/cpp/workspace_layout_test.cc b/src/test/cpp/workspace_layout_test.cc index 9c73d960211c14..4f75a67472f098 100644 --- a/src/test/cpp/workspace_layout_test.cc +++ b/src/test/cpp/workspace_layout_test.cc @@ -38,15 +38,7 @@ class WorkspaceLayoutTest : public ::testing::Test { } void TearDown() override { - // TODO(bazel-team): The code below deletes all the files in the workspace - // but it intentionally skips directories. As a consequence, there may be - // empty directories from test to test. Remove this once - // blaze_util::DeleteDirectories(path) exists. - std::vector files_in_workspace; - blaze_util::GetAllFilesUnder(build_root_, &files_in_workspace); - for (const std::string& file : files_in_workspace) { - blaze_util::UnlinkPath(file); - } + blaze_util::RemoveRecursively(blaze_util::Path(build_root_)); } const std::string build_root_;