From 14bbdf849917289bc049a9ca3ca180b06d1defed Mon Sep 17 00:00:00 2001 From: Matvey Larionov Date: Fri, 30 Jun 2017 14:10:55 +0300 Subject: [PATCH 1/2] Move file operations to where possible --- dist-clang.files | 6 +- src/base/BUILD.gn | 2 +- src/base/aliases.h | 6 + src/base/c_utils.h | 39 +++++-- src/base/c_utils_forward.h | 11 +- src/base/c_utils_posix.h | 21 +--- src/base/const_string.h | 1 + src/base/file/file.h | 29 +++-- src/base/file/file_posix.cc | 147 ++++++++++++------------ src/base/file/file_test.cc | 142 +++++++++-------------- src/base/file_utils.cc | 111 ++++++++++++++++++ src/base/file_utils.h | 45 +++----- src/base/file_utils_posix.cc | 159 -------------------------- src/base/file_utils_test.cc | 130 ++++++--------------- src/base/process.cc | 2 +- src/base/process.h | 10 +- src/base/process_impl.cc | 6 +- src/base/process_impl.h | 4 +- src/base/process_test.cc | 22 ++-- src/base/protobuf_utils.cc | 4 +- src/base/protobuf_utils.h | 4 +- src/base/temporary_dir.h | 8 +- src/base/temporary_dir_posix.cc | 2 +- src/base/test_process.cc | 6 +- src/base/test_process.h | 6 +- src/cache/database_leveldb.cc | 4 +- src/cache/database_leveldb.h | 4 +- src/cache/database_sqlite.cc | 5 +- src/cache/database_sqlite.h | 4 +- src/cache/file_cache.cc | 78 +++++++------ src/cache/file_cache.h | 24 ++-- src/cache/file_cache_migrator_test.cc | 16 ++- src/cache/file_cache_test.cc | 158 +++++++++++++------------ src/client/clang.cc | 2 +- src/client/clang.h | 2 +- src/client/clang_command.cc | 7 +- src/client/clang_command.hh | 4 +- src/client/clang_test.cc | 7 +- src/client/clean_command.cc | 4 +- src/client/clean_command.hh | 4 +- src/client/command.hh | 4 +- src/client/command_test.cc | 1 + src/client/configuration.cc | 21 ++-- src/client/configuration_test.cc | 12 +- src/client/driver_command.cc | 2 +- src/client/driver_command.hh | 4 +- src/daemon/absorber.cc | 4 +- src/daemon/absorber.h | 2 +- src/daemon/compilation_daemon.cc | 16 +-- src/daemon/compilation_daemon.h | 8 +- src/daemon/emitter.cc | 22 ++-- src/daemon/emitter_test.cc | 114 +++++++++--------- src/net/connection_linux_test.cc | 2 +- src/net/network_service_impl.cc | 2 +- tools/clang/main.cc | 2 +- 55 files changed, 679 insertions(+), 783 deletions(-) create mode 100644 src/base/file_utils.cc delete mode 100644 src/base/file_utils_posix.cc diff --git a/dist-clang.files b/dist-clang.files index 1cd592a4..d4ebf845 100644 --- a/dist-clang.files +++ b/dist-clang.files @@ -2215,7 +2215,7 @@ src/base/file/pipe.h src/base/file/pipe_linux.cc src/base/file/pipe_mac.cc src/base/file_utils.h -src/base/file_utils_posix.cc +src/base/file_utils.cc src/base/file_utils_test.cc src/base/future.h src/base/future_test.cc @@ -2283,7 +2283,10 @@ src/client/clean_command.hh src/client/command.cc src/client/command.hh src/client/command_test.cc +src/client/configuration.cc +src/client/configuration.hh src/client/configuration.proto +src/client/configuration_test.cc src/client/driver_command.cc src/client/driver_command.hh src/daemon/absorber.cc @@ -8735,6 +8738,7 @@ src/third_party/snappy/exported/snappy_unittest.cc src/third_party/snappy/linux/snappy-stubs-public.h src/third_party/snappy/mac/snappy-stubs-public.h src/third_party/snappy/win/snappy-stubs-public.h +tools/clang/main.cc tools/args_parser/BUILD.gn tools/args_parser/main.cc tools/get_report.py diff --git a/src/base/BUILD.gn b/src/base/BUILD.gn index d575602d..40a3ab90 100644 --- a/src/base/BUILD.gn +++ b/src/base/BUILD.gn @@ -42,7 +42,7 @@ shared_library("base") { "file/pipe_linux.cc", "file/pipe_mac.cc", "file_utils.h", - "file_utils_posix.cc", + "file_utils.cc", "future.h", "locked_list.h", "locked_queue.h", diff --git a/src/base/aliases.h b/src/base/aliases.h index f8ca741d..9d4ae5d4 100644 --- a/src/base/aliases.h +++ b/src/base/aliases.h @@ -18,6 +18,8 @@ #include STL(unordered_set) #include STL(vector) +#include STL_EXPERIMENTAL(filesystem) + namespace dist_clang { namespace base { @@ -67,6 +69,10 @@ using Mutex = std::mutex; template using Pair = std::pair; +using Path = std::experimental::filesystem::path; + +using Perms = std::experimental::filesystem::perms; + using Seconds = std::chrono::seconds; template diff --git a/src/base/c_utils.h b/src/base/c_utils.h index 7007494d..b88dd243 100644 --- a/src/base/c_utils.h +++ b/src/base/c_utils.h @@ -15,24 +15,31 @@ #include #include +#include STL_EXPERIMENTAL(filesystem) +#include STL(system_error) + namespace dist_clang { namespace base { -inline bool ChangeCurrentDir(Immutable path, String* error) { - if (chdir(path.c_str()) == -1) { - GetLastError(error); +inline bool ChangeCurrentDir(const Path& path, String* error) { + std::error_code ec; + std::experimental::filesystem::current_path(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } return false; } return true; } -inline Immutable GetCurrentDir(String* error) { - UniquePtr buf(new char[PATH_MAX]); - if (!getcwd(buf.get(), PATH_MAX)) { - GetLastError(error); - return Immutable(); +inline Path GetCurrentDir(String* error) { + std::error_code ec; + const auto& current_dir = std::experimental::filesystem::current_path(ec); + if (ec && error) { + *error = ec.message(); } - return buf; + return current_dir; } inline Literal GetEnv(Literal env_name, Literal default_env) { @@ -52,5 +59,19 @@ inline void GetLastError(String* error) { } } +inline bool SetPermissions(const Path& path, const Perms permissions, + String* error) { + std::error_code ec; + std::experimental::filesystem::permissions(path.c_str(), permissions, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return false; + } + return true; +} + + } // namespace base } // namespace dist_clang diff --git a/src/base/c_utils_forward.h b/src/base/c_utils_forward.h index 0ddceef6..3f6d5b46 100644 --- a/src/base/c_utils_forward.h +++ b/src/base/c_utils_forward.h @@ -6,16 +6,17 @@ namespace dist_clang { namespace base { -bool ChangeCurrentDir(Immutable path, String* error = nullptr); -String CreateTempFile(String* error = nullptr); -String CreateTempFile(const char suffix[], String* error = nullptr); -Immutable GetCurrentDir(String* error = nullptr); +bool ChangeCurrentDir(const Path& path, String* error = nullptr); +Path CreateTempFile(String* error = nullptr); +Path CreateTempFile(const char suffix[], String* error = nullptr); +Path GetCurrentDir(String* error = nullptr); Literal GetEnv(Literal env_name, Literal default_env = Literal::empty); Literal GetHomeDir(String* error = nullptr); void GetLastError(String* error); bool GetSelfPath(String& result, String* error = nullptr); Literal SetEnv(Literal env_name, const String& value, String* error = nullptr); -bool SetPermissions(const String& path, int mask, String* error = nullptr); +bool SetPermissions(const Path& path, const Perms permissions, + String* error = nullptr); } // namespace base } // namespace dist_clang diff --git a/src/base/c_utils_posix.h b/src/base/c_utils_posix.h index 16ae1ddb..dc6aa7c5 100644 --- a/src/base/c_utils_posix.h +++ b/src/base/c_utils_posix.h @@ -23,7 +23,7 @@ inline Literal SetEnv(Literal env_name, const String& value, String* error) { return old_value; } -inline String CreateTempFile(String* error) { +inline Path CreateTempFile(String* error) { char buf[] = "/tmp/clangd-XXXXXX.files"; #if defined(OS_LINUX) const int fd = mkostemps(buf, 6, O_CLOEXEC); @@ -35,13 +35,13 @@ inline String CreateTempFile(String* error) { #endif if (fd == -1) { GetLastError(error); - return String(); + return Path(); } close(fd); - return String(buf); + return Path(buf); } -inline String CreateTempFile(const char suffix[], String* error) { +inline Path CreateTempFile(const char suffix[], String* error) { const char prefix[] = "/tmp/clangd-XXXXXX"; const size_t prefix_size = sizeof(prefix) - 1; UniquePtr buf(new char[prefix_size + strlen(suffix) + 1]); @@ -59,11 +59,10 @@ inline String CreateTempFile(const char suffix[], String* error) { #endif if (fd == -1) { GetLastError(error); - return String(); + return Path(); } close(fd); - const auto result = String(buf.get()); - return result; + return Path(buf.get()); } inline Literal GetHomeDir(String* error) { @@ -104,13 +103,5 @@ inline bool GetSelfPath(String& result, String* error) { return true; } -inline bool SetPermissions(const String& path, int mask, String* error) { - if (chmod(path.c_str(), mask) == -1) { - GetLastError(error); - return false; - } - return true; -} - } // namespace base } // namespace dist_clang diff --git a/src/base/const_string.h b/src/base/const_string.h index ae7ca195..d3c7af77 100644 --- a/src/base/const_string.h +++ b/src/base/const_string.h @@ -16,6 +16,7 @@ namespace base { class Literal { public: + inline const char* c_str() const { return str_; } inline operator const char*() const { return str_; } inline size_t size() const { return strlen(str_); } inline bool operator==(const Literal& other) const { diff --git a/src/base/file/file.h b/src/base/file/file.h index 6821584d..7f5cb99c 100644 --- a/src/base/file/file.h +++ b/src/base/file/file.h @@ -7,7 +7,7 @@ namespace base { class File final : public Data { public: - explicit File(const String& path); // Open read-only file + explicit File(const Path& path); // Open read-only file using Handle::Close; @@ -17,38 +17,37 @@ class File final : public Data { } } - static bool IsFile(const String& path, String* error = nullptr); - static bool IsExecutable(const String& path, String* error = nullptr); - static bool Exists(const String& path, String* error = nullptr); + static bool IsFile(const Path& path, String* error = nullptr); + static bool IsExecutable(const Path& path, String* error = nullptr); + static bool Exists(const Path& path, String* error = nullptr); ui64 Size(String* error = nullptr) const; bool Read(Immutable* output, String* error = nullptr); bool Hash(Immutable* output, const List& skip_list = List(), String* error = nullptr); - bool CopyInto(const String& dst_path, String* error = nullptr); + bool CopyInto(const Path& dst_path, String* error = nullptr); - static ui64 Size(const String& path, String* error = nullptr); - static bool Read(const String& path, Immutable* output, + static ui64 Size(const Path& path, String* error = nullptr); + static bool Read(const Path& path, Immutable* output, String* error = nullptr); - static bool Write(const String& path, Immutable input, + static bool Write(const Path& path, Immutable input, String* error = nullptr); - static bool Hash(const String& path, Immutable* output, + static bool Hash(const Path& path, Immutable* output, const List& skip_list = List(), String* error = nullptr); - static bool Copy(const String& src_path, const String& dst_path, + static bool Copy(const Path& src_path, const Path& dst_path, String* error = nullptr); - static bool Link(const String& src_path, const String& dst_path, + static bool Move(const Path& src, const Path& dst, String* error = nullptr); - static bool Move(const String& src, const String& dst, - String* error = nullptr); - static bool Delete(const String& path, String* error = nullptr); + static bool Delete(const Path& path, String* error = nullptr); private: - File(const String& path, ui64 size); // Open truncated write-only file + File(const Path& path, ui64 size); // Open truncated write-only file bool Close(String* error = nullptr); + String path_; String error_; String move_on_close_; }; diff --git a/src/base/file/file_posix.cc b/src/base/file/file_posix.cc index 4a38f9cf..6f13b958 100644 --- a/src/base/file/file_posix.cc +++ b/src/base/file/file_posix.cc @@ -2,8 +2,12 @@ #include #include +#include #include +#include STL_EXPERIMENTAL(filesystem) +#include STL(system_error) + #include #include #include @@ -15,16 +19,41 @@ #endif namespace dist_clang { + +namespace { + +bool GetStatus(const Path& path, + std::experimental::filesystem::file_status* status, + String* error) { + CHECK(status); + std::error_code ec; + *status = std::experimental::filesystem::status(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return false; + } + return true; +} + +Path AddTempEnding(const Path& path) { + return base::AppendExtension(path, ".tmp"); +} + +} // anonymous namespace + namespace base { -File::File(const String& path) : Data(open(path.c_str(), O_RDONLY)) { +File::File(const Path& path) + : Data(open(path.c_str(), O_RDONLY | O_CLOEXEC)), path_(path) { if (!IsValid()) { GetLastError(&error_); return; } if (!IsFile(path)) { - error_ = path + " is a directory"; + error_ = path.string() + " is a directory"; Handle::Close(); return; } @@ -45,18 +74,16 @@ File::File(const String& path) : Data(open(path.c_str(), O_RDONLY)) { } // static -bool File::IsFile(const String& path, String* error) { - struct stat buffer; - if (stat(path.c_str(), &buffer)) { - GetLastError(error); +bool File::IsFile(const Path& path, String* error) { + std::experimental::filesystem::file_status status; + if (!GetStatus(path, &status, error)) { return false; } - - return (buffer.st_mode & S_IFDIR) == 0; + return std::experimental::filesystem::is_regular_file(status); } // static -bool File::IsExecutable(const String& path, String* error) { +bool File::IsExecutable(const Path& path, String* error) { if (!IsFile(path, error)) { return false; } @@ -69,29 +96,19 @@ bool File::IsExecutable(const String& path, String* error) { } // static -bool File::Exists(const String& path, String* error) { - if (access(path.c_str(), F_OK)) { - GetLastError(error); - return false; - } - - if (!IsFile(path, error)) { +bool File::Exists(const Path& path, String* error) { + std::experimental::filesystem::file_status status; + if (!GetStatus(path, &status, error)) { return false; } - - return true; + return std::experimental::filesystem::exists(status) && + std::experimental::filesystem::is_regular_file(status); } ui64 File::Size(String* error) const { DCHECK(IsValid()); - struct stat buffer; - if (fstat(native(), &buffer)) { - GetLastError(error); - return 0; - } - - return buffer.st_size; + return File::Size(path_, error); } bool File::Read(Immutable* output, String* error) { @@ -150,7 +167,7 @@ bool File::Hash(Immutable* output, const List& skip_list, return true; } -bool File::CopyInto(const String& dst_path, String* error) { +bool File::CopyInto(const Path& dst_path, String* error) { DCHECK(IsValid()); // Force unlinking of |dst|, since it may be hard-linked with other places. @@ -198,19 +215,20 @@ bool File::CopyInto(const String& dst_path, String* error) { } // static -ui64 File::Size(const String& path, String* error) { - File file(path); - - if (!file.IsValid()) { - file.GetCreationError(error); +ui64 File::Size(const Path& path, String* error) { + std::error_code ec; + const auto file_size = std::experimental::filesystem::file_size(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } return 0; } - - return file.Size(error); + return static_cast(file_size); } // static -bool File::Read(const String& path, Immutable* output, String* error) { +bool File::Read(const Path& path, Immutable* output, String* error) { File file(path); if (!file.IsValid()) { @@ -222,7 +240,7 @@ bool File::Read(const String& path, Immutable* output, String* error) { } // static -bool File::Hash(const String& path, Immutable* output, +bool File::Hash(const Path& path, Immutable* output, const List& skip_list, String* error) { File file(path); @@ -235,17 +253,19 @@ bool File::Hash(const String& path, Immutable* output, } // static -bool File::Delete(const String& path, String* error) { - if (unlink(path.c_str()) == -1) { - GetLastError(error); +bool File::Delete(const Path& path, String* error) { + std::error_code ec; + if (!std::experimental::filesystem::remove(path, ec)) { + if (error) { + *error = ec.message(); + } return false; } - return true; } // static -bool File::Write(const String& path, Immutable input, String* error) { +bool File::Write(const Path& path, Immutable input, String* error) { File dst(path, input.size()); if (!dst.IsValid()) { @@ -272,7 +292,7 @@ bool File::Write(const String& path, Immutable input, String* error) { } // static -bool File::Copy(const String& src_path, const String& dst_path, String* error) { +bool File::Copy(const Path& src_path, const Path& dst_path, String* error) { File src(src_path); if (!src.IsValid()) { @@ -284,51 +304,29 @@ bool File::Copy(const String& src_path, const String& dst_path, String* error) { } // static -bool File::Link(const String& src, const String& dst, String* error) { - auto Link = [&src, &dst]() -> int { -#if defined(OS_LINUX) - // Linux doesn't guarantee that |link()| do dereferences symlinks, thus - // we use |linkat()| which does for sure. - return linkat(AT_FDCWD, src.c_str(), AT_FDCWD, dst.c_str(), - AT_SYMLINK_FOLLOW); -#elif defined(OS_MACOSX) - return link(src.c_str(), dst.c_str()); -#else -#pragma message "This platform doesn't support hardlinks!" - errno = EACCES; - return -1; -#endif - }; - - // Try to create hard-link at first. - if (Link() == 0 || - (errno == EEXIST && unlink(dst.c_str()) == 0 && Link() == 0)) { - return true; - } - - return File::Copy(src, dst, error); -} - -// static -bool File::Move(const String& src, const String& dst, String* error) { - if (rename(src.c_str(), dst.c_str()) == -1) { - GetLastError(error); +bool File::Move(const Path& src, const Path& dst, String* error) { + std::error_code ec; + std::experimental::filesystem::rename(src, dst, ec); + if (ec) { + if (error) { + *error = ec.message(); + } return false; } - return true; } -File::File(const String& path, ui64 size) +File::File(const Path& path, ui64 size) : Data([=] { // We need write-access even on object files after introduction of the // "split-dwarf" option, see // https://sourceware.org/bugzilla/show_bug.cgi?id=971 const auto mode = mode_t(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); const auto flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC; - const String tmp_path = path + ".tmp"; + const Path tmp_path = AddTempEnding(path); return open(tmp_path.c_str(), flags, mode); }()), + path_(AddTempEnding(path)), move_on_close_(path) { if (!IsValid()) { GetLastError(&error_); @@ -372,12 +370,13 @@ bool File::Close(String* error) { return true; } - const String tmp_path = move_on_close_ + ".tmp"; + const Path tmp_path = AddTempEnding(move_on_close_); if (!Move(tmp_path, move_on_close_, error)) { Delete(tmp_path); return false; } + path_ = move_on_close_; return true; } diff --git a/src/base/file/file_test.cc b/src/base/file/file_test.cc index 2345fa90..a5f823b6 100644 --- a/src/base/file/file_test.cc +++ b/src/base/file/file_test.cc @@ -5,7 +5,9 @@ #include -#include +#include STL(fstream) +#include STL_EXPERIMENTAL(filesystem) +#include STL(system_error) namespace dist_clang { namespace base { @@ -13,13 +15,14 @@ namespace base { TEST(FileTest, Read) { const auto expected_content = "All your base are belong to us"_l; const base::TemporaryDir temp_dir; - const String file_path = String(temp_dir) + "/file"; + const Path temp_dir_path = temp_dir.GetPath(); + const Path file_path = temp_dir_path / "file"; - int fd = open(file_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0777); - ASSERT_NE(-1, fd); - int size = write(fd, expected_content, expected_content.size()); - ASSERT_EQ(expected_content.size(), static_cast(size)); - close(fd); + { + std::ofstream file(file_path, std::ios_base::out | std::ios_base::trunc); + ASSERT_TRUE(file.good()); + file << expected_content; + } Immutable content(true); // |content| is assignable since we use it many times during test. @@ -40,35 +43,32 @@ TEST(FileTest, Read) { TEST(FileTest, Write) { const auto expected_content = "All your base are belong to us"_l; const base::TemporaryDir temp_dir; - const String file_path = String(temp_dir) + "/file"; + const Path file_path = temp_dir.GetPath() / "file"; String error; EXPECT_TRUE(File::Write(file_path, expected_content, &error)) << error; char content[expected_content.size()]; - int fd = open(file_path.c_str(), O_RDONLY); - ASSERT_NE(-1, fd); - int size = read(fd, content, expected_content.size()); - ASSERT_EQ(expected_content.size(), static_cast(size)); - close(fd); + std::ifstream file(file_path); + ASSERT_TRUE(file.good()); + file.read(content, expected_content.size()); EXPECT_EQ(expected_content, String(content, expected_content.size())); - // Can't write to directory. EXPECT_FALSE(File::Write(temp_dir, expected_content)); } TEST(FileTest, Size) { const base::TemporaryDir temp_dir; - const String file_path = String(temp_dir) + "/file"; + const Path temp_dir_path = temp_dir.GetPath(); + const Path file_path = temp_dir_path / "file"; const String content = "1234567890"; - int fd = open(file_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0777); - ASSERT_NE(-1, fd); - int size = write(fd, content.data(), content.size()); - ASSERT_EQ(content.size(), static_cast(size)); - close(fd); - + { + std::ofstream file(file_path, std::ios_base::out | std::ios_base::trunc); + ASSERT_TRUE(file.good()); + file << content; + } EXPECT_EQ(content.size(), File::Size(file_path)); File file(file_path); @@ -83,13 +83,14 @@ TEST(FileTest, Hash) { const auto content = "All your base are belong to us"_l; const auto expected_hash = "c9e92e37df1e856cbd0abffe104225b8"_l; const base::TemporaryDir temp_dir; - const String file_path = String(temp_dir) + "/file"; + const Path temp_dir_path = temp_dir.GetPath(); + const String file_path = temp_dir_path / "file"; - int fd = open(file_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0777); - ASSERT_NE(-1, fd); - int size = write(fd, content, content.size()); - ASSERT_EQ(content.size(), static_cast(size)); - close(fd); + { + std::ofstream file(file_path, std::ios_base::out | std::ios_base::trunc); + ASSERT_TRUE(file.good()); + file << content; + } Immutable hash(true); // |hash| is assignable since we use it many times during test. @@ -116,81 +117,52 @@ TEST(FileTest, Copy) { const auto expected_content1 = "All your base are belong to us"_l; const auto expected_content2 = "Nothing lasts forever"_l; const TemporaryDir temp_dir; - const String file1 = String(temp_dir) + "/1"; - const String file2 = String(temp_dir) + "/2"; - const String file3 = String(temp_dir) + "/3"; + const Path temp_dir_path = temp_dir.GetPath(); + const Path file1 = temp_dir_path / "1"; + const Path file2 = temp_dir_path / "2"; + const Path file3 = temp_dir_path / "3"; ASSERT_TRUE(File::Write(file1, expected_content1)); ASSERT_TRUE(File::Copy(file1, file2)); - struct stat st; - const auto mode = mode_t(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + const auto permissions = Perms::owner_read | Perms::owner_write | + Perms::group_read | Perms::others_read; + + std::error_code ec; Immutable content(true); // |content| is assignable since we use it many times during test. - ASSERT_EQ(0, stat(file1.c_str(), &st)); - EXPECT_EQ(1u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); + EXPECT_EQ(1u, std::experimental::filesystem::hard_link_count(file1, ec)); + ASSERT_FALSE(ec); + + const auto& file1_status = std::experimental::filesystem::status(file1, ec); + ASSERT_FALSE(ec); + EXPECT_EQ(permissions, file1_status.permissions() & permissions); + + EXPECT_EQ(1u, std::experimental::filesystem::hard_link_count(file2, ec)); + ASSERT_FALSE(ec); + + auto file2_status = std::experimental::filesystem::status(file2, ec); + ASSERT_FALSE(ec); + EXPECT_EQ(permissions, file2_status.permissions() & permissions); String error; - ASSERT_EQ(0, stat(file2.c_str(), &st)); - EXPECT_EQ(1u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); ASSERT_TRUE(File::Read(file2, &content, &error)) << error; EXPECT_EQ(expected_content1, content); ASSERT_TRUE(File::Write(file3, expected_content2)); ASSERT_TRUE(File::Copy(file3, file2)); - ASSERT_EQ(0, stat(file2.c_str(), &st)); - EXPECT_EQ(1u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - ASSERT_TRUE(File::Read(file2, &content)); - EXPECT_EQ(expected_content2, content); -} + EXPECT_EQ(1u, std::experimental::filesystem::hard_link_count(file2, ec)); + ASSERT_FALSE(ec); -TEST(FileTest, Link) { - const auto expected_content = "All your base are belong to us"_l; - const TemporaryDir temp_dir; - const String file1 = String(temp_dir) + "/1"; - const String file2 = String(temp_dir) + "/2"; - const String file3 = String(temp_dir) + "/3"; - - ASSERT_TRUE(File::Write(file1, expected_content)); - ASSERT_TRUE(File::Link(file1, file2)); - - struct stat st; - const auto mode = mode_t(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); - - ASSERT_EQ(0, stat(file1.c_str(), &st)); - EXPECT_EQ(2u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - - auto inode = st.st_ino; - ASSERT_EQ(0, stat(file2.c_str(), &st)); - EXPECT_EQ(2u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - EXPECT_EQ(inode, st.st_ino); - - ASSERT_TRUE(File::Write(file3, expected_content)); - ASSERT_TRUE(File::Link(file3, file2)); - - ASSERT_EQ(0, stat(file1.c_str(), &st)); - EXPECT_EQ(1u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - - inode = st.st_ino; - ASSERT_EQ(0, stat(file2.c_str(), &st)); - EXPECT_EQ(2u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - EXPECT_NE(inode, st.st_ino); - - inode = st.st_ino; - ASSERT_EQ(0, stat(file3.c_str(), &st)); - EXPECT_EQ(2u, st.st_nlink); - EXPECT_EQ(mode, st.st_mode & mode); - EXPECT_EQ(inode, st.st_ino); + file2_status = std::experimental::filesystem::status(file2, ec); + ASSERT_FALSE(ec); + EXPECT_EQ(permissions, file2_status.permissions() & permissions); + + ASSERT_TRUE(File::Read(file2, &content, &error)) << error; + EXPECT_EQ(expected_content2, content); } } // namespace base diff --git a/src/base/file_utils.cc b/src/base/file_utils.cc new file mode 100644 index 00000000..02973019 --- /dev/null +++ b/src/base/file_utils.cc @@ -0,0 +1,111 @@ +#include + +#include +#include + +#include STL(chrono) +#include STL_EXPERIMENTAL(filesystem) +#include STL(map) +#include STL(regex) +#include STL(system_error) + +namespace dist_clang { +namespace base { + +void WalkDirectory( + const Path& path, + Fn visitor, + String* error) { + namespace fs = std::experimental::filesystem; + auto has_error = [&error](const std::error_code& ec) { + if (ec) { + if (error) { + *error = ec.message(); + } + return true; + } + return false; + }; + std::error_code ec; + // TODO(matthewtff): Consider following symlinks & skip permission denied + // directories. + fs::recursive_directory_iterator directories(path, ec); + + if (has_error(ec)) { + return; + } + + for (const auto& directory_entry : directories) { + // TODO(matthewtff): Reimplement using directory_entry::is_regular_file() + // as it gets implemented in libcxx. + const auto& entry_status = directory_entry.status(ec); + if (has_error(ec)) { + return; + } + + if (!fs::is_regular_file(entry_status)) { + continue; + } + + const auto& path = directory_entry.path(); + + const auto& last_write_time = fs::last_write_time(path, ec); + if (has_error(ec)) { + return; + } + + const auto& file_size = fs::file_size(path, ec); + if (has_error(ec)) { + return; + } + const auto seconds_since_epoch = std::chrono::duration_cast( + last_write_time.time_since_epoch()).count(); + visitor(path, seconds_since_epoch, file_size); + } +} + +ui64 CalculateDirectorySize(const Path& path, String* error) { + ui64 result = 0u; + + WalkDirectory(path, [&result](const Path&, ui64, ui64 size) { + result += size; + }, error); + + return result; +} + +bool RemoveEmptyDirectory(const Path& path) { + std::error_code ec; + return std::experimental::filesystem::remove(path, ec); +} + +bool ChangeOwner(const Path& path, ui32 uid, String* error) { +#if defined(OS_WIN) +#error NOTIMPLEMENTED +#else + if (lchown(path.c_str(), uid, getgid()) == -1) { + GetLastError(error); + return false; + } + return true; +#endif +} + +bool CreateDirectory(const Path& path, String* error) { + std::error_code ec; + std::experimental::filesystem::create_directories(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return false; + } + return true; +} + +Path AppendExtension(Path path, const char* extension) { + return path += extension; +} + +} // namespace base +} // namespace dist diff --git a/src/base/file_utils.h b/src/base/file_utils.h index 1dd996fc..add88fe4 100644 --- a/src/base/file_utils.h +++ b/src/base/file_utils.h @@ -1,43 +1,24 @@ #pragma once -#include - -#include +#include namespace dist_clang { namespace base { void WalkDirectory( - const String& path, - Fn visitor, + const Path& path, + Fn visitor, String* error = nullptr); -ui64 CalculateDirectorySize(const String& path, String* error = nullptr); - -Pair GetModificationTime( - const String& path, String* error = nullptr); - -// This function returns the path to an object with the least recent -// modification time. It may be regular file, directory or any other kind of -// file. The |path| should be a directory. If there is nothing inside the |path| -// or error has occured, the return value is |false|. -bool GetLeastRecentPath(const String& path, String& result, - String* error = nullptr); -bool GetLeastRecentPath(const String& path, String& result, const char* regex, - String* error = nullptr); - -inline bool RemoveEmptyDirectory(const String& path) { - return !rmdir(path.c_str()); -} - -inline bool ChangeOwner(const String& path, ui32 uid, String* error = nullptr) { - if (lchown(path.c_str(), uid, getgid()) == -1) { - GetLastError(error); - return false; - } - return true; -} - -bool CreateDirectory(const String& path, String* error = nullptr); + +ui64 CalculateDirectorySize(const Path& path, String* error = nullptr); + +bool RemoveEmptyDirectory(const Path& path); + +bool ChangeOwner(const Path& path, ui32 uid, String* error = nullptr); + +bool CreateDirectory(const Path& path, String* error = nullptr); + +Path AppendExtension(Path path, const char* extension); } // namespace base } // namespace dist_clang diff --git a/src/base/file_utils_posix.cc b/src/base/file_utils_posix.cc deleted file mode 100644 index 955d2d42..00000000 --- a/src/base/file_utils_posix.cc +++ /dev/null @@ -1,159 +0,0 @@ -#include - -#include -#include -#include - -#include STL(map) -#include STL(regex) - -#include -#include -#include -#include -#include -#include -#include - -namespace dist_clang { -namespace base { - -void WalkDirectory( - const String& path, - Fn visitor, - String* error) { - List paths; - paths.push_back(path); - - while (!paths.empty()) { - const String& path = paths.front(); - if (DIR* dir = opendir(path.c_str())) { - struct dirent* entry = nullptr; - - while ((entry = readdir(dir))) { - const String entry_name = entry->d_name; - const String new_path = path + "/" + entry_name; - - if (entry_name != "." && entry_name != "..") { - struct stat buffer; - if (!stat(new_path.c_str(), &buffer)) { - if (S_ISDIR(buffer.st_mode)) { - paths.push_back(new_path); - } else if (S_ISREG(buffer.st_mode)) { - struct timespec time_spec; -#if defined(OS_MACOSX) - time_spec = buffer.st_mtimespec; -#elif defined(OS_LINUX) - time_spec = buffer.st_mtim; -#else -#pragma message "Don't know how to get modification time on this platform!" - NOTREACHED(); -#endif - visitor(new_path, time_spec.tv_sec, buffer.st_size); - } - } else { - GetLastError(error); - break; - } - } - } - closedir(dir); - } else { - GetLastError(error); - break; - } - - paths.pop_front(); - } -} - -ui64 CalculateDirectorySize(const String& path, String* error) { - ui64 result = 0u; - - WalkDirectory(path, [&result](const String&, ui64, ui64 size) { - result += size; - }, error); - - return result; -} - -Pair GetModificationTime(const String& path, String* error) { - struct stat buffer; - if (stat(path.c_str(), &buffer) == -1) { - GetLastError(error); - return {0, 0}; - } - - struct timespec time_spec; -#if defined(OS_MACOSX) - time_spec = buffer.st_mtimespec; -#elif defined(OS_LINUX) - time_spec = buffer.st_mtim; -#else -#pragma message "Don't know how to get modification time on this platform!" - NOTREACHED(); -#endif - return {time_spec.tv_sec, time_spec.tv_nsec}; -} - -bool GetLeastRecentPath(const String& path, String& result, const char* regex, - String* error) { - DIR* dir = opendir(path.c_str()); - if (dir == nullptr) { - GetLastError(error); - return false; - } - - const Pair null_time = {0, 0}; - Pair mtime = null_time; - while (true) { - const struct dirent* entry = readdir(dir); - if (!entry) { - break; - } - - if (!std::regex_match(entry->d_name, std::regex(regex))) { - continue; - } - - const String entry_name = entry->d_name; - if (entry_name == "." || entry_name == "..") { - continue; - } - - const String new_path = path + "/" + entry_name; - auto current_mtime = GetModificationTime(new_path); - if (mtime == null_time || - (current_mtime != null_time && current_mtime < mtime)) { - mtime = current_mtime; - result = new_path; - } - } - closedir(dir); - - return mtime != null_time; -} - -bool GetLeastRecentPath(const String& path, String& result, String* error) { - return GetLeastRecentPath(path, result, ".*", error); -} - -bool CreateDirectory(const String& path, String* error) { - for (size_t i = 1; i < path.size(); ++i) { - if (path[i] == '/' && mkdir(path.substr(0, i).c_str(), 0755) == -1 && - errno != EEXIST) { - GetLastError(error); - return false; - } - } - - if (mkdir(path.c_str(), 0755) == -1 && errno != EEXIST) { - GetLastError(error); - return false; - } - - return true; -} - -} // namespace base -} // namespace dist diff --git a/src/base/file_utils_test.cc b/src/base/file_utils_test.cc index 04035965..38f62efa 100644 --- a/src/base/file_utils_test.cc +++ b/src/base/file_utils_test.cc @@ -1,116 +1,56 @@ #include +#include #include #include #include #include +#include STL(fstream) +#include STL(system_error) #include STL(thread) -#include -#include +#include STL_EXPERIMENTAL(filesystem) namespace dist_clang { namespace base { TEST(FileUtilsTest, CalculateDirectorySize) { const base::TemporaryDir temp_dir; - const String dir1 = String(temp_dir) + "/1"; - const String dir2 = String(temp_dir) + "/2"; - const String file1 = String(temp_dir) + "/file1"; - const String file2 = dir1 + "/file2"; - const String file3 = dir2 + "/file3"; + const Path temp_dir_path(temp_dir.GetPath()); + const Path dir1 = temp_dir_path / "1"; + const Path dir2 = temp_dir_path / "2"; + const Path file1 = temp_dir_path / "file1"; + const Path file2 = dir1 / "file2"; + const Path file3 = dir2 / "file3"; const String content1 = "a"; const String content2 = "ab"; const String content3 = "abc"; - ASSERT_NE(-1, mkdir(dir1.c_str(), 0777)); - ASSERT_NE(-1, mkdir(dir2.c_str(), 0777)); - int fd1 = open(file1.c_str(), O_CREAT | O_WRONLY); - int fd2 = open(file2.c_str(), O_CREAT | O_WRONLY); - int fd3 = open(file3.c_str(), O_CREAT | O_WRONLY); - ASSERT_TRUE(fd1 != -1 && fd2 != -1 && fd3 != -1); - ASSERT_EQ(content1.size(), - static_cast(write(fd1, content1.data(), content1.size()))); - ASSERT_EQ(content2.size(), - static_cast(write(fd2, content2.data(), content2.size()))); - ASSERT_EQ(content3.size(), - static_cast(write(fd3, content3.data(), content3.size()))); - close(fd1); - close(fd2); - close(fd3); + std::error_code ec; - String error; - EXPECT_EQ(content1.size() + content2.size() + content3.size(), - CalculateDirectorySize(temp_dir, &error)) - << error; -} + std::experimental::filesystem::create_directory(dir1, ec); + ASSERT_FALSE(ec) << ec.message(); + std::experimental::filesystem::create_directory(dir2, ec); + ASSERT_FALSE(ec) << ec.message(); -TEST(FileUtilsTest, LeastRecentPath) { - const base::TemporaryDir temp_dir; - const String dir = String(temp_dir) + "/1"; - const String file1 = String(temp_dir) + "/2"; - const String file2 = dir + "/3"; - const String file3 = dir + "/4"; - - ASSERT_NE(-1, mkdir(dir.c_str(), 0777)); - - std::this_thread::sleep_for(Seconds(1)); - int fd = open(file1.c_str(), O_CREAT, 0777); - ASSERT_NE(-1, fd); - close(fd); - - String path; - EXPECT_TRUE(GetLeastRecentPath(temp_dir, path)); - EXPECT_EQ(dir, path) << "dir mtime is " << GetModificationTime(dir).first - << ":" << GetModificationTime(dir).second - << " ; path mtime is " << GetModificationTime(path).first - << ":" << GetModificationTime(path).second; - - std::this_thread::sleep_for(Seconds(1)); - fd = open(file2.c_str(), O_CREAT, 0777); - ASSERT_NE(-1, fd); - close(fd); - - EXPECT_TRUE(GetLeastRecentPath(temp_dir, path)); - EXPECT_EQ(file1, path) << "file1 mtime is " - << GetModificationTime(file1).first << ":" - << GetModificationTime(file1).second - << " ; path mtime is " - << GetModificationTime(path).first << ":" - << GetModificationTime(path).second; - - std::this_thread::sleep_for(Seconds(1)); - fd = open(file3.c_str(), O_CREAT, 0777); - ASSERT_NE(-1, fd); - close(fd); - - EXPECT_TRUE(GetLeastRecentPath(dir, path)); - EXPECT_EQ(file2, path) << "file2 mtime is " - << GetModificationTime(file2).first << ":" - << GetModificationTime(file2).second - << " ; path mtime is " - << GetModificationTime(path).first << ":" - << GetModificationTime(path).second; -} + std::ofstream f1(file1); + std::ofstream f2(file2); + std::ofstream f3(file3); + ASSERT_TRUE(f1.good() && f2.good() && f3.good()); -TEST(FileUtilsTest, LeastRecentPathWithRegex) { - const base::TemporaryDir temp_dir; - const String file1 = String(temp_dir) + "/1"; - const String file2 = String(temp_dir) + "/2"; - - int fd = open(file1.c_str(), O_CREAT, 0777); - ASSERT_NE(-1, fd); - close(fd); + f1 << content1; + f2 << content2; + f3 << content3; - std::this_thread::sleep_for(Seconds(1)); - fd = open(file2.c_str(), O_CREAT, 0777); - ASSERT_NE(-1, fd); - close(fd); + f1.close(); + f2.close(); + f3.close(); - String path; - EXPECT_TRUE(GetLeastRecentPath(temp_dir, path, "2")); - EXPECT_EQ(file2, path); + String error; + EXPECT_EQ(content1.size() + content2.size() + content3.size(), + CalculateDirectorySize(temp_dir, &error)) + << error; } TEST(FileUtilsTest, TempFile) { @@ -126,15 +66,13 @@ TEST(FileUtilsTest, TempFile) { TEST(FileUtilsTest, CreateDirectory) { String error; const base::TemporaryDir temp_dir; - const String temp = String(temp_dir) + "/1/2/3"; + const Path& temp = temp_dir.GetPath() / "1" / "2" / "3"; - ASSERT_TRUE(CreateDirectory(temp, &error)) << error; + ASSERT_TRUE(CreateDirectory(temp.string(), &error)) << error; - DIR* dir = opendir(temp.c_str()); - EXPECT_TRUE(dir); - if (dir) { - closedir(dir); - } + std::error_code ec; + EXPECT_TRUE(std::experimental::filesystem::exists(temp, ec)); + ASSERT_FALSE(ec) << ec.message(); } } // namespace base diff --git a/src/base/process.cc b/src/base/process.cc index 25d337ef..f11b9b9d 100644 --- a/src/base/process.cc +++ b/src/base/process.cc @@ -3,7 +3,7 @@ namespace dist_clang { namespace base { -Process::Process(const String& exec_path, Immutable cwd_path, ui32 uid) +Process::Process(const Path& exec_path, const Path& cwd_path, ui32 uid) : exec_path_(exec_path), cwd_path_(cwd_path), uid_(uid) { } diff --git a/src/base/process.h b/src/base/process.h index bd6041a8..fb775228 100644 --- a/src/base/process.h +++ b/src/base/process.h @@ -20,6 +20,8 @@ FORWARD_TEST(ClientTest, SendPluginPath); } // namespace client namespace daemon { +FORWARD_TEST(AbsorberTest, OverloadOnQueueSizeExceed); +FORWARD_TEST(AbsorberTest, RestoreFromCacheOnOverload); FORWARD_TEST(AbsorberTest, StoreLocalCacheWithoutBlacklist); FORWARD_TEST(AbsorberTest, StoreLocalCacheWithBlacklist); FORWARD_TEST(AbsorberTest, StoreLocalCacheWithAndWithoutBlacklist); @@ -50,12 +52,12 @@ namespace base { class ProcessImpl; class Process - : public Testable { + : public Testable { public: enum : ui16 { UNLIMITED = 0 }; enum : ui32 { SAME_UID = 0 }; - explicit Process(const String& exec_path, Immutable cwd_path = Immutable(), + explicit Process(const Path& exec_path, const Path& cwd_path = Path(), ui32 uid = SAME_UID); virtual ~Process() {} @@ -84,7 +86,7 @@ class Process String* error = nullptr) = 0; protected: - Immutable exec_path_, cwd_path_; + Path exec_path_, cwd_path_; List args_, envs_; Immutable stdout_, stderr_; const ui32 uid_; @@ -98,6 +100,8 @@ class Process FRIEND_TEST(client::ClientTest, SuccessfulCompilation); FRIEND_TEST(client::ClientTest, FailedCompilation); FRIEND_TEST(client::ClientTest, SendPluginPath); + FRIEND_TEST(daemon::AbsorberTest, OverloadOnQueueSizeExceed); + FRIEND_TEST(daemon::AbsorberTest, RestoreFromCacheOnOverload); FRIEND_TEST(daemon::AbsorberTest, StoreLocalCacheWithoutBlacklist); FRIEND_TEST(daemon::AbsorberTest, StoreLocalCacheWithBlacklist); FRIEND_TEST(daemon::AbsorberTest, StoreLocalCacheWithAndWithoutBlacklist); diff --git a/src/base/process_impl.cc b/src/base/process_impl.cc index bb25d6ed..f644453f 100644 --- a/src/base/process_impl.cc +++ b/src/base/process_impl.cc @@ -11,7 +11,7 @@ namespace dist_clang { namespace base { -ProcessImpl::ProcessImpl(const String& exec_path, Immutable cwd_path, ui32 uid) +ProcessImpl::ProcessImpl(const Path& exec_path, const Path& cwd_path, ui32 uid) : Process(exec_path, cwd_path, uid), killed_(false) {} // This method contains code between |fork()| and |exec()|. Since we're in a @@ -37,7 +37,7 @@ bool ProcessImpl::RunChild(Pipe& out, Pipe& err, Pipe* in) { err[0].Close(); if (!cwd_path_.empty() && !ChangeCurrentDir(cwd_path_)) { - std::cerr << "Can't change current directory to " << cwd_path_.c_str() + std::cerr << "Can't change current directory to " << cwd_path_ << std::endl; exit(1); } @@ -68,7 +68,7 @@ bool ProcessImpl::RunChild(Pipe& out, Pipe& err, Pipe* in) { execve(exec_path_.c_str(), const_cast(argv), const_cast(env)) == -1) || execv(exec_path_.c_str(), const_cast(argv)) == -1) { - std::cerr << "Failed to execute " << exec_path_.c_str() << ": " + std::cerr << "Failed to execute " << exec_path_ << ": " << strerror(errno) << std::endl; exit(1); } diff --git a/src/base/process_impl.h b/src/base/process_impl.h index 3d9de722..7b2050d4 100644 --- a/src/base/process_impl.h +++ b/src/base/process_impl.h @@ -17,8 +17,8 @@ class ProcessImpl : public Process { private: friend class DefaultFactory; - explicit ProcessImpl(const String& exec_path, - Immutable cwd_path = Immutable(), ui32 uid = SAME_UID); + explicit ProcessImpl(const Path& exec_path, + const Path& cwd_path = Path(), ui32 uid = SAME_UID); bool RunChild(Pipe& out, Pipe& err, Pipe* in); bool WaitPid(int pid, ui64 sec_timeout, String* error = nullptr); diff --git a/src/base/process_test.cc b/src/base/process_test.cc index 9aa07f4a..e06ddcd2 100644 --- a/src/base/process_test.cc +++ b/src/base/process_test.cc @@ -30,25 +30,25 @@ class ProcessTest : public ::testing::Test { TEST_F(ProcessTest, CheckExitCode) { const int exit_code = 1; - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l) .AppendArg(Immutable("exit " + std::to_string(exit_code))); ASSERT_FALSE(process->Run(1)); } TEST_F(ProcessTest, ChangeCurrentDir) { - const auto dir = "/usr"_l; + const String dir = "/usr"; ASSERT_NE(dir, GetCurrentDir()) << "Don't run this test from " << dir; ProcessPtr process = Process::Create(sh, dir, Process::SAME_UID); process->AppendArg("-c"_l).AppendArg("pwd"_l); ASSERT_TRUE(process->Run(1)); - EXPECT_EQ(Immutable(dir) + "\n"_l, process->stdout()); + EXPECT_EQ(dir + "\n", process->stdout().string_copy()); EXPECT_TRUE(process->stderr().empty()); } TEST_F(ProcessTest, ReadStderr) { const String test_data(10, 'a'); - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l) .AppendArg(Immutable("echo " + test_data + " 1>&2")); ASSERT_TRUE(process->Run(1)); @@ -58,7 +58,7 @@ TEST_F(ProcessTest, ReadStderr) { TEST_F(ProcessTest, ReadSmallOutput) { const String test_data(10, 'a'); - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l).AppendArg(Immutable("echo " + test_data)); ASSERT_TRUE(process->Run(1)); EXPECT_EQ(Immutable(test_data) + "\n"_l, process->stdout()); @@ -67,7 +67,7 @@ TEST_F(ProcessTest, ReadSmallOutput) { TEST_F(ProcessTest, ReadLargeOutput) { const String test_data(67000, 'a'); - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l).AppendArg(Immutable("echo " + test_data)); ASSERT_TRUE(process->Run(1)); EXPECT_EQ(Immutable(test_data) + "\n"_l, process->stdout()); @@ -76,7 +76,7 @@ TEST_F(ProcessTest, ReadLargeOutput) { TEST_F(ProcessTest, EchoSmallInput) { const String test_data(10, 'a'); - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l).AppendArg("cat"_l); ASSERT_TRUE(process->Run(1, Immutable(test_data))); EXPECT_EQ(Immutable(test_data), process->stdout()); @@ -85,7 +85,7 @@ TEST_F(ProcessTest, EchoSmallInput) { TEST_F(ProcessTest, EchoLargeInput) { const String test_data(67000, 'a'); - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l).AppendArg("cat"_l); ASSERT_TRUE(process->Run(1, Immutable(test_data))); EXPECT_EQ(Immutable(test_data), process->stdout()); @@ -93,13 +93,13 @@ TEST_F(ProcessTest, EchoLargeInput) { } TEST_F(ProcessTest, ReadTimeout) { - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AppendArg("-c"_l).AppendArg("sleep 2"_l); ASSERT_FALSE(process->Run(1)); } TEST_F(ProcessTest, TooManyArgs) { - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); for (auto i = 0u; i < ProcessImpl::MAX_ARGS + 2; ++i) { process->AppendArg("yes"_l); } @@ -108,7 +108,7 @@ TEST_F(ProcessTest, TooManyArgs) { TEST_F(ProcessTest, RunWithEnvironment) { const auto expected_value = "some_value"_l; - ProcessPtr process = Process::Create(sh, String(), Process::SAME_UID); + ProcessPtr process = Process::Create(sh, Path(), Process::SAME_UID); process->AddEnv("ENV", expected_value) .AppendArg("-c"_l) .AppendArg("echo -n $ENV"_l); diff --git a/src/base/protobuf_utils.cc b/src/base/protobuf_utils.cc index 63fb9ee6..3dfcc0b4 100644 --- a/src/base/protobuf_utils.cc +++ b/src/base/protobuf_utils.cc @@ -20,7 +20,7 @@ Log& Log::operator<<(const google::protobuf::Message& info) { return *this; } -bool LoadFromFile(const String& path, google::protobuf::Message* message, +bool LoadFromFile(const Path& path, google::protobuf::Message* message, String* error) { Immutable contents; if (!File::Read(path, &contents, error) || @@ -36,7 +36,7 @@ bool LoadFromFile(const String& path, google::protobuf::Message* message, return true; } -bool SaveToFile(const String& path, const google::protobuf::Message& message, +bool SaveToFile(const Path& path, const google::protobuf::Message& message, String* error) { String output; if (!google::protobuf::TextFormat::PrintToString(message, &output) || diff --git a/src/base/protobuf_utils.h b/src/base/protobuf_utils.h index 42d0f925..e89b29f0 100644 --- a/src/base/protobuf_utils.h +++ b/src/base/protobuf_utils.h @@ -17,9 +17,9 @@ namespace base { template <> Log& Log::operator<<(const google::protobuf::Message& info); -bool LoadFromFile(const String& path, google::protobuf::Message* message, +bool LoadFromFile(const Path& path, google::protobuf::Message* message, String* error = nullptr); -bool SaveToFile(const String& path, const google::protobuf::Message& message, +bool SaveToFile(const Path& path, const google::protobuf::Message& message, String* error = nullptr); } // namespace base diff --git a/src/base/temporary_dir.h b/src/base/temporary_dir.h index 44b66625..f36b90bd 100644 --- a/src/base/temporary_dir.h +++ b/src/base/temporary_dir.h @@ -10,14 +10,16 @@ class TemporaryDir { TemporaryDir(); ~TemporaryDir(); - inline const String& GetPath() const { return path_; } + inline const Path& GetPath() const { return path_; } inline const String& GetError() const { return error_; } - inline operator bool() const { return !path_.empty(); } + inline operator bool() const { return path_.empty(); } + inline operator Path() const { return path_; } inline operator String() const { return path_; } private: - String path_, error_; + Path path_; + String error_; }; } // namespace base diff --git a/src/base/temporary_dir_posix.cc b/src/base/temporary_dir_posix.cc index d6524a7c..3c1b415c 100644 --- a/src/base/temporary_dir_posix.cc +++ b/src/base/temporary_dir_posix.cc @@ -37,7 +37,7 @@ TemporaryDir::TemporaryDir() { GetLastError(&error_); return; } - path_ = buf; + path_ = Path(buf); } TemporaryDir::~TemporaryDir() { diff --git a/src/base/test_process.cc b/src/base/test_process.cc index 896bccca..bc20df04 100644 --- a/src/base/test_process.cc +++ b/src/base/test_process.cc @@ -3,14 +3,14 @@ namespace dist_clang { namespace base { -UniquePtr TestProcess::Factory::Create(const String& exec_path, - Immutable cwd_path, ui32 uid) { +UniquePtr TestProcess::Factory::Create( + const Path& exec_path, const Path& cwd_path, ui32 uid) { auto new_t = new TestProcess(exec_path, cwd_path, uid); on_create_(new_t); return UniquePtr(new_t); } -TestProcess::TestProcess(const String& exec_path, Immutable cwd_path, ui32 uid) +TestProcess::TestProcess(const Path& exec_path, const Path& cwd_path, ui32 uid) : Process(exec_path, cwd_path, uid) { } diff --git a/src/base/test_process.h b/src/base/test_process.h index 7ed928e2..fcdd8c71 100644 --- a/src/base/test_process.h +++ b/src/base/test_process.h @@ -16,8 +16,8 @@ class TestProcess : public Process { public: using OnCreateCallback = Fn; - virtual UniquePtr Create(const String& exec_path, - Immutable cwd_path, ui32 uid) override; + virtual UniquePtr Create(const Path& exec_path, + const Path& cwd_path, ui32 uid) override; inline void CallOnCreate(OnCreateCallback callback) { on_create_ = callback; @@ -36,7 +36,7 @@ class TestProcess : public Process { String PrintArgs() const; // helper for gtest assertions. private: - TestProcess(const String& exec_path, Immutable cwd_path, ui32 uid); + TestProcess(const Path& exec_path, const Path& cwd_path, ui32 uid); OnRunCallback on_run_ = EmptyLambda(false); Atomic* run_attempts_ = nullptr; diff --git a/src/cache/database_leveldb.cc b/src/cache/database_leveldb.cc index f5aeaba9..79e8e305 100644 --- a/src/cache/database_leveldb.cc +++ b/src/cache/database_leveldb.cc @@ -10,8 +10,8 @@ namespace dist_clang { namespace cache { -LevelDB::LevelDB(const String& path, const String& name) - : path_(path + "/leveldb_" + name) { +LevelDB::LevelDB(const Path& path, const String& name) + : path_(base::AppendExtension(path / "leveldb_", name.c_str())) { using namespace leveldb; Options options; diff --git a/src/cache/database_leveldb.h b/src/cache/database_leveldb.h index cacb7fe8..420a50ab 100644 --- a/src/cache/database_leveldb.h +++ b/src/cache/database_leveldb.h @@ -14,7 +14,7 @@ namespace cache { class LevelDB : public Database { public: - LevelDB(const String& path, const String& name); + LevelDB(const Path& path, const String& name); ~LevelDB(); bool Set(const String& key, const Immutable& value) override THREAD_SAFE; @@ -30,7 +30,7 @@ class LevelDB : public Database { private: leveldb::DB* db_ = nullptr; - const String path_; + const Path path_; }; } // namespace cache diff --git a/src/cache/database_sqlite.cc b/src/cache/database_sqlite.cc index c9a7ad76..9cdbce73 100644 --- a/src/cache/database_sqlite.cc +++ b/src/cache/database_sqlite.cc @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -61,8 +62,8 @@ SQLite::SQLite() : path_(":memory:") { LOG(DB_INFO) << "SQLite database is created in-memory"; } -SQLite::SQLite(const String& path, const String& name) - : path_(path + "/" + name + ".sqlite") { +SQLite::SQLite(const Path& path, const String& name) + : path_(base::AppendExtension(path / name, ".sqlite")) { auto result = sqlite3_open(path_.c_str(), &db_); CHECK(result == SQLITE_OK) << "Failed to open " << path_ << ": " << sqlite3_errstr(result); diff --git a/src/cache/database_sqlite.h b/src/cache/database_sqlite.h index eee6d5e4..cb1bfc24 100644 --- a/src/cache/database_sqlite.h +++ b/src/cache/database_sqlite.h @@ -24,7 +24,7 @@ class SQLite : public Database +#include #include #include #include @@ -11,8 +12,10 @@ #include +#if defined(OS_LINUX) || defined(OS_MACOSX) #include #include +#endif #include @@ -20,10 +23,13 @@ namespace dist_clang { namespace { -String ReplaceTildeInPath(const String& path) { - if (path[0] == '~' && path[1] == '/') { - return String(base::GetHomeDir()) + path.substr(1); +Path ReplaceTildeInPath(const Path& path) { +#if defined(OS_LINUX) || defined(OS_MACOSX) + const String& path_string = path.string(); + if (path_string[0] == '~' && path_string[1] == '/') { + return Path(base::GetHomeDir().c_str()) / path_string.substr(1); } +#endif return path; } @@ -45,14 +51,14 @@ String HashCombine(const Immutable& source, const cache::ExtraFiles& files) { namespace cache { -FileCache::FileCache(const String& path, ui64 size, bool snappy, +FileCache::FileCache(const Path& path, ui64 size, bool snappy, bool store_index) : path_(ReplaceTildeInPath(path)), snappy_(snappy), store_index_(store_index), max_size_(size) {} -FileCache::FileCache(const String& path) +FileCache::FileCache(const Path& path) : FileCache(path, UNLIMITED, false, false) {} FileCache::~FileCache() { @@ -78,7 +84,7 @@ bool FileCache::Run(ui64 clean_period) { CHECK(clean_period > 0); entries_->BeginTransaction(); - base::WalkDirectory(path_, [this](const String& file_path, ui64 mtime, ui64) { + base::WalkDirectory(path_, [this](const Path& file_path, ui64 mtime, ui64) { std::regex regex("([a-f0-9]{32}-[a-f0-9]{8}-[a-f0-9]{8})\\.manifest$"); std::cmatch match; if (!std::regex_search(file_path.c_str(), match, regex) || @@ -147,11 +153,12 @@ UnhandledHash FileCache::Hash(UnhandledSource code, bool FileCache::Find(UnhandledSource code, const ExtraFiles& extra_files, CommandLine command_line, Version version, - const String& current_dir, Entry* entry) const { + const Path& current_dir, Entry* entry) const { DCHECK(entry); auto unhandled_hash = Hash(code, extra_files, command_line, version); - const String manifest_path = CommonPath(unhandled_hash) + ".manifest"; + const Path manifest_path = + base::AppendExtension(CommonPath(unhandled_hash), ".manifest"); const ReadLock lock(this, manifest_path); if (!lock) { @@ -169,9 +176,10 @@ bool FileCache::Find(UnhandledSource code, const ExtraFiles& extra_files, Immutable::Rope hash_rope = {unhandled_hash}; for (const auto& header : manifest.direct().headers()) { Immutable header_hash; - const String header_path = - header[0] == '/' ? header : current_dir + "/" + header; - if (!base::File::Hash(header_path, &header_hash)) { + const Path header_path(header); + const Path header_absolute_path = + header_path.is_absolute() ? header_path : current_dir / header; + if (!base::File::Hash(header_absolute_path, &header_hash)) { return false; } hash_rope.push_back(header_hash); @@ -190,7 +198,8 @@ bool FileCache::Find(UnhandledSource code, const ExtraFiles& extra_files, bool FileCache::Find(HandledHash hash, Entry* entry) const { DCHECK(entry); - const String manifest_path = CommonPath(hash) + ".manifest"; + const Path manifest_path = + base::AppendExtension(CommonPath(hash), ".manifest"); const ReadLock lock(this, manifest_path); if (!lock) { @@ -208,7 +217,7 @@ bool FileCache::Find(HandledHash hash, Entry* entry) const { ui64 size = 0; if (manifest.v1().err()) { - const String stderr_path = CommonPath(hash) + ".stderr"; + const Path stderr_path = base::AppendExtension(CommonPath(hash), ".stderr"); if (!base::File::Read(stderr_path, &entry->stderr)) { return false; } @@ -216,7 +225,7 @@ bool FileCache::Find(HandledHash hash, Entry* entry) const { } if (manifest.v1().obj()) { - const String object_path = CommonPath(hash) + ".o"; + const Path object_path = base::AppendExtension(CommonPath(hash), ".o"); if (manifest.v1().snappy()) { String error; @@ -245,7 +254,7 @@ bool FileCache::Find(HandledHash hash, Entry* entry) const { } if (manifest.v1().dep()) { - const String deps_path = CommonPath(hash) + ".d"; + const Path deps_path = base::AppendExtension(CommonPath(hash), ".d"); if (!base::File::Read(deps_path, &entry->deps)) { return false; } @@ -265,7 +274,7 @@ void FileCache::Store(UnhandledSource code, const ExtraFiles& extra_files, } void FileCache::Store(string::HandledHash hash, Entry entry) { - auto manifest_path = CommonPath(hash) + ".manifest"; + auto manifest_path = base::AppendExtension(CommonPath(hash), ".manifest"); WriteLock lock(this, manifest_path); String error; @@ -283,7 +292,7 @@ void FileCache::Store(string::HandledHash hash, Entry entry) { manifest.mutable_v1()->set_err(!entry.stderr.empty()); if (!entry.stderr.empty()) { - const String stderr_path = CommonPath(hash) + ".stderr"; + const Path stderr_path = base::AppendExtension(CommonPath(hash), ".stderr"); if (!base::File::Write(stderr_path, entry.stderr, &error)) { RemoveEntry(hash); @@ -297,7 +306,7 @@ void FileCache::Store(string::HandledHash hash, Entry entry) { manifest.mutable_v1()->set_obj(!entry.object.empty()); if (!entry.object.empty()) { - const String object_path = CommonPath(hash) + ".o"; + const Path object_path = base::AppendExtension(CommonPath(hash), ".o"); if (!snappy_) { object_size = entry.object.size(); @@ -332,7 +341,7 @@ void FileCache::Store(string::HandledHash hash, Entry entry) { manifest.mutable_v1()->set_dep(!entry.deps.empty()); if (!entry.deps.empty()) { - const String deps_path = CommonPath(hash) + ".d"; + const Path deps_path = base::AppendExtension(CommonPath(hash), ".d"); if (!base::File::Write(deps_path, entry.deps, &error)) { RemoveEntry(hash); @@ -361,8 +370,8 @@ void FileCache::Store(string::HandledHash hash, Entry entry) { bool FileCache::GetEntrySize(string::Hash hash, ui64* size) const { DCHECK(size); - const String common_path = CommonPath(hash); - const String manifest_path = common_path + ".manifest"; + const Path common_path = CommonPath(hash); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); SQLite::Value entry; if (entries_ && entries_->Get(hash.str, &entry)) { @@ -385,11 +394,11 @@ bool FileCache::RemoveEntry(string::Hash hash) { String error; SQLite::Value entry; bool has_entry = entries_->Get(hash.str, &entry); - const String common_path = CommonPath(hash); - const String manifest_path = common_path + ".manifest"; - const String object_path = common_path + ".o"; - const String deps_path = common_path + ".d"; - const String stderr_path = common_path + ".stderr"; + const Path common_path = CommonPath(hash); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); + const Path object_path = base::AppendExtension(common_path, ".o"); + const Path deps_path = base::AppendExtension(common_path, ".d"); + const Path stderr_path = base::AppendExtension(common_path, ".stderr"); bool result = true; auto entry_size = has_entry ? std::get(entry) : 0u; @@ -445,7 +454,8 @@ void FileCache::DoStore(UnhandledHash orig_hash, const List& headers, // headers, while checking the direct cache. Such approach has a little // drawback, because the changes in the dependent headers will make a // false-positive direct cache hit, followed by true cache miss. - const String manifest_path = CommonPath(orig_hash) + ".manifest"; + const Path manifest_path = + base::AppendExtension(CommonPath(orig_hash), ".manifest"); WriteLock lock(this, manifest_path); if (!lock) { @@ -468,9 +478,11 @@ void FileCache::DoStore(UnhandledHash orig_hash, const List& headers, for (const String& header : headers) { String error; Immutable header_hash; - const String header_path = - header[0] == '/' ? header : current_dir + "/" + header; - if (!base::File::Hash(header_path, &header_hash, skip_list, &error)) { + const Path header_path(header); + const Path header_absolute_path = + header_path.is_absolute() ? header_path : current_dir / header_path; + if (!base::File::Hash(header_absolute_path, &header_hash, + skip_list, &error)) { LOG(CACHE_ERROR) << "Failed to hash " << header_path << ": " << error; return; } @@ -535,7 +547,7 @@ void FileCache::Clean(UniquePtr list) { SQLite::Value entry; CHECK(entries_->First(&hash.str, &entry)); - auto manifest_path = CommonPath(hash) + ".manifest"; + auto manifest_path = base::AppendExtension(CommonPath(hash), ".manifest"); WriteLock lock(this, manifest_path); if (lock) { LOG(CACHE_VERBOSE) << "Cache overuse is " << (cache_size_ - max_size_) @@ -546,7 +558,7 @@ void FileCache::Clean(UniquePtr list) { entries_->EndTransaction(); } -FileCache::ReadLock::ReadLock(const FileCache* file_cache, const String& path) +FileCache::ReadLock::ReadLock(const FileCache* file_cache, const Path& path) : cache_(file_cache), path_(path) { if (!base::File::Exists(path)) { return; @@ -583,7 +595,7 @@ void FileCache::ReadLock::Unlock() { } } -FileCache::WriteLock::WriteLock(const FileCache* file_cache, const String& path) +FileCache::WriteLock::WriteLock(const FileCache* file_cache, const Path& path) : cache_(file_cache), path_(path) { std::lock_guard lock(cache_->locks_mutex_); diff --git a/src/cache/file_cache.h b/src/cache/file_cache.h index 37ea80af..31be0dfc 100644 --- a/src/cache/file_cache.h +++ b/src/cache/file_cache.h @@ -96,8 +96,8 @@ class FileCache { Immutable stderr; }; - FileCache(const String& path, ui64 size, bool snappy, bool store_index); - explicit FileCache(const String& path); + FileCache(const Path& path, ui64 size, bool snappy, bool store_index); + explicit FileCache(const Path& path); ~FileCache(); bool Run(ui64 clean_period); @@ -115,7 +115,7 @@ class FileCache { bool Find(string::UnhandledSource code, const ExtraFiles& extra_files, string::CommandLine command_line, string::Version version, - const String& current_dir, Entry* entry) const; + const Path& current_dir, Entry* entry) const; bool Find(string::HandledHash hash, Entry* entry) const; @@ -143,7 +143,7 @@ class FileCache { class ReadLock { public: - ReadLock(const FileCache* WEAK_PTR cache, const String& path); + ReadLock(const FileCache* WEAK_PTR cache, const Path& path); ~ReadLock() { Unlock(); } inline operator bool() const { return locked_; } @@ -151,13 +151,13 @@ class FileCache { private: const FileCache* cache_; - const String path_; + const Path path_; bool locked_ = false; }; class WriteLock { public: - WriteLock(const FileCache* WEAK_PTR cache, const String& path); + WriteLock(const FileCache* WEAK_PTR cache, const Path& path); ~WriteLock() { Unlock(); } inline operator bool() const { return locked_; } @@ -165,20 +165,20 @@ class FileCache { private: const FileCache* cache_; - const String path_; + const Path path_; bool locked_ = false; }; friend class ReadLock; friend class WriteLock; - inline String SecondPath(string::Hash hash) const { + inline Path SecondPath(string::Hash hash) const { DCHECK(hash.str.size() >= 2); - return path_ + "/" + hash.str[0] + "/" + hash.str[1]; + return path_ / String(1, hash.str[0]) / String(1, hash.str[1]); } - inline String CommonPath(string::Hash hash) const { - return SecondPath(hash) + "/" + hash.str.string_copy(); + inline Path CommonPath(string::Hash hash) const { + return SecondPath(hash) / hash.str.string_copy(); } void DoStore(string::UnhandledHash orig_hash, const List& headers, @@ -204,7 +204,7 @@ class FileCache { mutable HashMap read_locks_; mutable HashSet write_locks_; - const String path_; + const Path path_; bool snappy_, store_index_; UniquePtr database_; UniquePtr entries_; diff --git a/src/cache/file_cache_migrator_test.cc b/src/cache/file_cache_migrator_test.cc index a98cc9ab..d64ebdd5 100644 --- a/src/cache/file_cache_migrator_test.cc +++ b/src/cache/file_cache_migrator_test.cc @@ -14,12 +14,14 @@ TEST(FileCacheMigratorTest, Version_0_to_1_Simple) { const base::TemporaryDir tmp_dir; string::Hash hash{"12345678901234567890123456789012-12345678-00000001"_l}; FileCache cache(tmp_dir); - const String common_path = cache.CommonPath(hash); - const String manifest_path = common_path + ".manifest"; + const Path common_path = cache.CommonPath(hash); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); ASSERT_TRUE(base::CreateDirectory(cache.SecondPath(hash))); - ASSERT_TRUE(base::File::Write(common_path + ".o", "12345"_l)); - ASSERT_TRUE(base::File::Write(common_path + ".d", "12345"_l)); + ASSERT_TRUE( + base::File::Write(base::AppendExtension(common_path, ".o"), "12345"_l)); + ASSERT_TRUE( + base::File::Write(base::AppendExtension(common_path, ".d"), "12345"_l)); proto::Manifest manifest; @@ -48,7 +50,8 @@ TEST(FileCacheMigratorTest, Version_0_to_1_Direct) { const base::TemporaryDir tmp_dir; string::Hash hash{"12345678901234567890123456789012-12345678-00000001"_l}; FileCache cache(tmp_dir); - const String manifest_path = cache.CommonPath(hash) + ".manifest"; + const String manifest_path = + base::AppendExtension(cache.CommonPath(hash), ".manifest"); proto::Manifest manifest; manifest.add_headers()->assign("test.h"); @@ -104,7 +107,8 @@ TEST(FileCacheMigratorTest, Version_1_to_2_Direct) { const base::TemporaryDir tmp_dir; string::Hash hash{"12345678901234567890123456789012-12345678-00000001"_l}; FileCache cache(tmp_dir); - const String manifest_path = cache.CommonPath(hash) + ".manifest"; + const String manifest_path = + base::AppendExtension(cache.CommonPath(hash), ".manifest"); proto::Manifest manifest; manifest.set_version(1); diff --git a/src/cache/file_cache_test.cc b/src/cache/file_cache_test.cc index d459bcc9..e52d6145 100644 --- a/src/cache/file_cache_test.cc +++ b/src/cache/file_cache_test.cc @@ -43,7 +43,7 @@ TEST(FileCacheTest, ExtraFilesGiveDifferentHash) { TEST(FileCacheTest, LockNonExistentFile) { const base::TemporaryDir tmp_dir; - const String absent_path = String(tmp_dir) + "/absent_file"; + const Path absent_path = tmp_dir.GetPath() / "absent_file"; FileCache cache(tmp_dir); { @@ -58,7 +58,7 @@ TEST(FileCacheTest, LockNonExistentFile) { TEST(FileCacheTest, DoubleLocks) { const base::TemporaryDir tmp_dir; - const String file_path = String(tmp_dir) + "/file"; + const Path file_path = tmp_dir.GetPath() / "file"; FileCache cache(tmp_dir); ASSERT_TRUE(base::File::Write(file_path, "1"_l)); @@ -95,11 +95,11 @@ TEST(FileCacheTest, RemoveEntry) { string::Hash hash1{"12345678901234567890123456789012-12345678-00000001"_l}; { ASSERT_TRUE(base::CreateDirectory(cache.SecondPath(hash1))); - const String common_path = cache.CommonPath(hash1); - const String manifest_path = common_path + ".manifest"; - const String object_path = common_path + ".o"; - const String deps_path = common_path + ".d"; - const String stderr_path = common_path + ".stderr"; + const Path common_path = cache.CommonPath(hash1); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); + const Path object_path = base::AppendExtension(common_path, ".o"); + const Path deps_path = base::AppendExtension(common_path, ".d"); + const Path stderr_path = base::AppendExtension(common_path, ".stderr"); proto::Manifest manifest; manifest.set_version(1); @@ -115,10 +115,10 @@ TEST(FileCacheTest, RemoveEntry) { string::Hash hash2{"12345678901234567890123456789012-12345678-00000002"_l}; { ASSERT_TRUE(base::CreateDirectory(cache.SecondPath(hash2))); - const String common_path = cache.CommonPath(hash2); - const String manifest_path = common_path + ".manifest"; - const String object_path = common_path + ".o"; - const String deps_path = common_path + ".d"; + const Path common_path = cache.CommonPath(hash2); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); + const Path object_path = base::AppendExtension(common_path, ".o"); + const Path deps_path = base::AppendExtension(common_path, ".d"); proto::Manifest manifest; manifest.set_version(1); @@ -133,10 +133,10 @@ TEST(FileCacheTest, RemoveEntry) { string::Hash hash3{"12345678901234567890123456789012-12345678-00000003"_l}; { ASSERT_TRUE(base::CreateDirectory(cache.SecondPath(hash3))); - const String common_path = cache.CommonPath(hash3); - const String manifest_path = common_path + ".manifest"; - const String object_path = common_path + ".o"; - const String deps_path = common_path + ".d"; + const Path common_path = cache.CommonPath(hash3); + const Path manifest_path = base::AppendExtension(common_path, ".manifest"); + const Path object_path = base::AppendExtension(common_path, ".o"); + const Path deps_path = base::AppendExtension(common_path, ".d"); ASSERT_TRUE(base::File::Write(manifest_path, "1"_l)); ASSERT_TRUE(base::File::Write(object_path, "1"_l)); @@ -159,13 +159,12 @@ TEST(FileCacheTest, RemoveEntry) { TEST(FileCacheTest, RestoreSingleEntry) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(path); + FileCache cache(tmp_dir.GetPath()); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -195,13 +194,12 @@ TEST(FileCacheTest, RestoreSingleEntry) { TEST(FileCacheTest, RestoreSingleEntryWithExtraFile) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(path); + FileCache cache(tmp_dir.GetPath()); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -238,13 +236,12 @@ TEST(FileCacheTest, RestoreSingleEntryWithExtraFile) { TEST(FileCacheTest, RestoreEntryWithMissingFile) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(path); + FileCache cache(tmp_dir.GetPath()); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -263,7 +260,7 @@ TEST(FileCacheTest, RestoreEntryWithMissingFile) { // Store the entry. cache.Store(hash, entry1); - base::File::Delete(cache.CommonPath(hash) + ".d"); + base::File::Delete(base::AppendExtension(cache.CommonPath(hash), ".d")); // Restore the entry. ASSERT_FALSE(cache.Find(hash, &entry2)); @@ -307,8 +304,7 @@ TEST(FileCacheTest, DISABLED_BadInitialCacheSize) { TEST(FileCacheTest, ExceedCacheSize) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String cache_path = path + "/cache"; + const Path cache_path = tmp_dir.GetPath() / "cache"; const Literal obj_content[] = {"22"_l, "333"_l, "4444"_l}; const HandledSource code[] = {HandledSource("int main() { return 0; }"_l), HandledSource("int main() { return 1; }"_l), @@ -354,16 +350,15 @@ TEST(FileCacheTest, ExceedCacheSize) { TEST(FileCacheTest, RestoreDirectEntry) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; - const String header2_rel_path = "test2.h"; + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path header1_path = tmp_dir.GetPath() / "test1.h"; + const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(path); + FileCache cache(tmp_dir.GetPath()); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -387,10 +382,11 @@ TEST(FileCacheTest, RestoreDirectEntry) { // Store the direct entry. const UnhandledSource orig_code("int main() {}"_l); const List headers = {header1_path, header2_rel_path}; - cache.Store(orig_code, {}, cl, version, headers, {}, path, hash); + cache.Store(orig_code, {}, cl, version, headers, {}, tmp_dir.GetPath(), hash); // Restore the entry. - ASSERT_TRUE(cache.Find(orig_code, {}, cl, version, path, &entry2)); + ASSERT_TRUE(cache.Find( + orig_code, {}, cl, version, tmp_dir.GetPath(), &entry2)); EXPECT_EQ(expected_object_code, entry2.object); EXPECT_EQ(expected_deps, entry2.deps); EXPECT_EQ(expected_stderr, entry2.stderr); @@ -398,12 +394,12 @@ TEST(FileCacheTest, RestoreDirectEntry) { TEST(FileCacheTest, RestoreDirectEntryWithExtraFile) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; - const String header2_rel_path = "test2.h"; + const Path path = tmp_dir.GetPath(); + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path header1_path = tmp_dir.GetPath() / "test1.h"; + const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; @@ -452,16 +448,15 @@ TEST(FileCacheTest, RestoreDirectEntryWithExtraFile) { TEST(FileCacheTest, DirectEntry_ChangedHeaderContents) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; - const String header2_rel_path = "test2.h"; + const Path object_path = tmp_dir.GetPath() / "test.o"; + const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path header1_path = tmp_dir.GetPath() / "test1.h"; + const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(path); + FileCache cache(tmp_dir.GetPath()); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry; @@ -485,23 +480,24 @@ TEST(FileCacheTest, DirectEntry_ChangedHeaderContents) { // Store the direct entry. const UnhandledSource orig_code("int main() {}"_l); const List headers = {header1_path, header2_rel_path}; - cache.Store(orig_code, {}, cl, version, headers, {}, path, hash); + cache.Store(orig_code, {}, cl, version, headers, {}, tmp_dir.GetPath(), hash); // Change header contents. ASSERT_TRUE(base::File::Write(header2_path, "#define C"_l)); // Restore the entry. - EXPECT_FALSE(cache.Find(orig_code, {}, cl, version, path, &entry)); + EXPECT_FALSE(cache.Find( + orig_code, {}, cl, version, tmp_dir.GetPath(), &entry)); } TEST(FileCacheTest, DirectEntry_ChangedPreprocessedHeaderContents) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; - const String preprocessed_header_path = path + "/precompile.h.pth"; + const Path path = tmp_dir.GetPath(); + const Path object_path = path / "test.o"; + const Path deps_path = path / "test.d"; + const Path header1_path = path / "test1.h"; + const Path header2_path = path / "test2.h"; + const Path preprocessed_header_path = path / "precompile.h.pth"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; @@ -544,11 +540,11 @@ TEST(FileCacheTest, DirectEntry_ChangedPreprocessedHeaderContents) { TEST(FileCacheTest, DirectEntry_RewriteManifest) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; + const Path path = tmp_dir.GetPath(); + const Path object_path = path / "test.o"; + const Path deps_path = path / "test.d"; + const Path header1_path = path / "test1.h"; + const Path header2_path = path / "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; @@ -590,11 +586,11 @@ TEST(FileCacheTest, DirectEntry_RewriteManifest) { TEST(FileCacheTest, DirectEntry_ChangedOriginalCode) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; + const Path path = tmp_dir.GetPath(); + const Path object_path = path / "test.o"; + const Path deps_path = path / "test.d"; + const Path header1_path = path / "test1.h"; + const Path header2_path = path / "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; @@ -631,11 +627,11 @@ TEST(FileCacheTest, DirectEntry_ChangedOriginalCode) { TEST(FileCacheTest, DirectEntry_ChangedExtraFile) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; - const String header1_path = path + "/test1.h"; - const String header2_path = path + "/test2.h"; + const Path path = tmp_dir.GetPath(); + const Path object_path = path / "test.o"; + const Path deps_path = path / "test.d"; + const Path header1_path = path / "test1.h"; + const Path header2_path = path / "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; @@ -677,9 +673,9 @@ TEST(FileCacheTest, DirectEntry_ChangedExtraFile) { TEST(FileCacheTest, RestoreAndMigrateSnappyEntry) { const base::TemporaryDir tmp_dir; - const String path = tmp_dir; - const String object_path = path + "/test.o"; - const String deps_path = path + "/test.d"; + const Path path = tmp_dir.GetPath(); + const Path object_path = path / "test.o"; + const Path deps_path = path / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; @@ -734,7 +730,8 @@ TEST(FileCacheTest, UseIndexFromDisk) { { FileCache cache(tmp_dir, FileCache::UNLIMITED, false, true); - const String manifest_path = cache.CommonPath(hash) + ".manifest"; + const Path manifest_path = + base::AppendExtension(cache.CommonPath(hash), ".manifest"); ASSERT_TRUE(base::CreateDirectory(cache.SecondPath(hash))); @@ -752,7 +749,8 @@ TEST(FileCacheTest, UseIndexFromDisk) { { FileCache cache(tmp_dir, FileCache::UNLIMITED, false, true); - const String manifest_path = cache.CommonPath(hash) + ".manifest"; + const Path manifest_path = + base::AppendExtension(cache.CommonPath(hash), ".manifest"); ASSERT_TRUE(base::File::Write(manifest_path, "1"_l)); EXPECT_TRUE(cache.Run(1)); diff --git a/src/client/clang.cc b/src/client/clang.cc index ead05dfe..5fe477ec 100644 --- a/src/client/clang.cc +++ b/src/client/clang.cc @@ -18,7 +18,7 @@ namespace dist_clang { namespace client { bool DoMain(int argc, const char* const argv[], Immutable socket_path, - Immutable clang_path, Immutable version, ui32 connect_timeout_secs, + const Path& clang_path, Immutable version, ui32 connect_timeout_secs, ui32 read_timeout_secs, ui32 send_timeout_secs, ui32 read_min_bytes, const HashMap& plugins, bool disabled) { if (clang_path.empty() || disabled) { diff --git a/src/client/clang.h b/src/client/clang.h index b243fa9c..805717be 100644 --- a/src/client/clang.h +++ b/src/client/clang.h @@ -6,7 +6,7 @@ namespace dist_clang { namespace client { bool DoMain(int argc, const char* const argv[], Immutable socket_path, - Immutable clang_path, Immutable version, ui32 connect_timeout_secs, + const Path& clang_path, Immutable version, ui32 connect_timeout_secs, ui32 read_timeout_secs, ui32 send_timeout_secs, ui32 read_min_bytes, const HashMap& plugins, bool disabled); diff --git a/src/client/clang_command.cc b/src/client/clang_command.cc index a3324167..7c58ebaa 100644 --- a/src/client/clang_command.cc +++ b/src/client/clang_command.cc @@ -26,7 +26,7 @@ ClangCommand::ClangCommand(llvm::ArrayRef args, }()), opts_(opts) {} -base::ProcessPtr ClangCommand::CreateProcess(Immutable current_dir, +base::ProcessPtr ClangCommand::CreateProcess(const Path& current_dir, ui32 user_id) const { NOTREACHED(); return base::ProcessPtr(); @@ -46,7 +46,7 @@ String ClangCommand::RenderAllArgs() const { } bool ClangCommand::FillFlags(base::proto::Flags* flags, - const String& clang_path, + const Path& clang_path, const String& clang_major_version) const { if (!flags) { return true; @@ -141,8 +141,7 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags, auto pos = replaced_command.find(self_path); if (pos != String::npos) { replaced_command.replace( - pos, self_path.size(), - clang_path.substr(0, clang_path.find_last_of('/'))); + pos, self_path.size(), clang_path.parent_path()); non_direct_list.push_back(arg->getSpelling().data()); non_direct_list.push_back(tmp_list.MakeArgString(replaced_command)); LOG(VERBOSE) << "Replaced command: " << non_direct_list.back(); diff --git a/src/client/clang_command.hh b/src/client/clang_command.hh index a75d3df8..0d8925e8 100644 --- a/src/client/clang_command.hh +++ b/src/client/clang_command.hh @@ -13,12 +13,12 @@ class ClangCommand : public Command { ClangCommand(llvm::ArrayRef args, SharedPtr opts); - base::ProcessPtr CreateProcess(Immutable current_dir, + base::ProcessPtr CreateProcess(const Path& current_dir, ui32 user_id) const override; String GetExecutable() const override; String RenderAllArgs() const override; bool CanFillFlags() const override { return true; } - bool FillFlags(base::proto::Flags* flags, const String& clang_path, + bool FillFlags(base::proto::Flags* flags, const Path& clang_path, const String& clang_major_version) const override; private: diff --git a/src/client/clang_test.cc b/src/client/clang_test.cc index 97d04d36..3299da0f 100644 --- a/src/client/clang_test.cc +++ b/src/client/clang_test.cc @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -78,7 +79,7 @@ class ClientTest : public ::testing::Test { namespace { -Literal clang_path = "fake_clang++"_l; +const Path clang_path = "fake_clang++"; Literal version = "fake_version"_l; } // namespace @@ -101,7 +102,7 @@ TEST_F(ClientTest, EmptyClangPath) { const char* argv[] = {"clang++", "-c", "/tmp/test_file.cc", nullptr}; const int argc = 3; - EXPECT_TRUE(client::DoMain(argc, argv, Immutable(), Immutable(), Immutable(), + EXPECT_TRUE(client::DoMain(argc, argv, Immutable(), Path(), Immutable(), 0, 0, 0, 0, HashMap(), false)); EXPECT_TRUE(weak_ptr.expired()); EXPECT_EQ(0u, send_count); @@ -179,7 +180,7 @@ TEST_F(ClientTest, CannotReadMessage) { const auto& extension = message.GetExtension(base::proto::Local::extension); - EXPECT_EQ(base::GetCurrentDir(), Immutable(extension.current_dir())); + EXPECT_EQ(base::GetCurrentDir(), extension.current_dir()); ASSERT_TRUE(extension.has_flags()); const auto& cc_flags = extension.flags(); diff --git a/src/client/clean_command.cc b/src/client/clean_command.cc index 386ff53c..9817fc8b 100644 --- a/src/client/clean_command.cc +++ b/src/client/clean_command.cc @@ -5,7 +5,9 @@ namespace dist_clang { namespace client { -base::ProcessPtr CleanCommand::CreateProcess(Immutable current_dir, +const char* CleanCommand::rm_path = "/bin/rm"; + +base::ProcessPtr CleanCommand::CreateProcess(const Path& current_dir, ui32 user_id) const { auto process = base::Process::Create(rm_path, current_dir, user_id); for (const auto& it : temp_files_) { diff --git a/src/client/clean_command.hh b/src/client/clean_command.hh index d5c4a673..3a3a9178 100644 --- a/src/client/clean_command.hh +++ b/src/client/clean_command.hh @@ -12,13 +12,13 @@ class CleanCommand : public Command { CleanCommand(const llvm::opt::ArgStringList& temp_files) : temp_files_(temp_files) {} - base::ProcessPtr CreateProcess(Immutable current_dir, + base::ProcessPtr CreateProcess(const Path& current_dir, ui32 user_id) const override; String GetExecutable() const override { return rm_path; } String RenderAllArgs() const override; private: - static constexpr const char* rm_path = "/bin/rm"; + static const char* rm_path; const llvm::opt::ArgStringList& temp_files_; }; diff --git a/src/client/command.hh b/src/client/command.hh index c3c1ad85..2939b4c3 100644 --- a/src/client/command.hh +++ b/src/client/command.hh @@ -22,7 +22,7 @@ class Command { virtual ~Command() {} - virtual base::ProcessPtr CreateProcess(Immutable current_dir, + virtual base::ProcessPtr CreateProcess(const Path& current_dir, ui32 user_id) const = 0; virtual String GetExecutable() const = 0; virtual String RenderAllArgs() const = 0; // For testing. @@ -33,7 +33,7 @@ class Command { } virtual bool FillFlags(base::proto::Flags* flags, - const String& clang_path, + const Path& clang_path, const String& clang_major_version) const { NOTREACHED(); return false; diff --git a/src/client/command_test.cc b/src/client/command_test.cc index a1101e6d..5f5f846b 100644 --- a/src/client/command_test.cc +++ b/src/client/command_test.cc @@ -1,4 +1,5 @@ #include +#include #include #include diff --git a/src/client/configuration.cc b/src/client/configuration.cc index 95fa4058..8a602059 100644 --- a/src/client/configuration.cc +++ b/src/client/configuration.cc @@ -15,12 +15,12 @@ namespace client { Configuration::Configuration() { // Try to load config file first. - String current_dir = base::GetCurrentDir(); + Path current_dir = base::GetCurrentDir(); do { - String config_path = current_dir + "/.distclang"; + const Path config_path = current_dir / ".distclang"; if (base::LoadFromFile(config_path, &config_)) { - if (config_.path()[0] != '/') { - config_.set_path(current_dir + "/" + config_.path()); + if (!config_path.is_absolute()) { + config_.set_path(current_dir / config_.path()); } LOG(VERBOSE) << "Took compiler path from " << config_path << " : " << config_.path(); @@ -48,8 +48,8 @@ Configuration::Configuration() { continue; } - if (plugin->path()[0] != '/') { - plugin->set_path(current_dir + "/" + plugin->path()); + if (!Path(plugin->path()).is_absolute()) { + plugin->set_path(current_dir / plugin->path()); } LOG(VERBOSE) << "Took plugin from " << config_path << " : " @@ -59,7 +59,7 @@ Configuration::Configuration() { break; } - current_dir = current_dir.substr(0, current_dir.find_last_of("/")); + current_dir = current_dir.parent_path(); } while (!current_dir.empty()); // Environment variables prevail over config file. @@ -110,9 +110,10 @@ Configuration::Configuration() { } for (const auto& dir : path_dirs) { - // TODO: convert |dir + "/clang"| to canonical path. - if (base::File::IsExecutable(dir + "/clang") && dir != self_path) { - config_.set_path(dir + "/clang"); + Path clang_path(dir); + clang_path /= "clang"; + if (base::File::IsExecutable(clang_path) && dir != self_path) { + config_.set_path(clang_path); break; } } diff --git a/src/client/configuration_test.cc b/src/client/configuration_test.cc index 2fe87799..23d75681 100644 --- a/src/client/configuration_test.cc +++ b/src/client/configuration_test.cc @@ -1,18 +1,22 @@ #include +#include #include #include #include #include +#include STL_EXPERIMENTAL(filesystem) + namespace dist_clang { namespace client { TEST(ClientConfigurationTest, ConfigFileInParentDir) { const base::TemporaryDir tmp_dir; - const String child_dir = String(tmp_dir) + "/1"; - const String config_path = String(tmp_dir) + "/.distclang"; + const Path temp_dir_path = tmp_dir.GetPath(); + const Path child_dir = temp_dir_path / "1"; + const Path config_path = temp_dir_path / ".distclang"; ASSERT_TRUE(base::CreateDirectory(child_dir)); @@ -32,10 +36,10 @@ TEST(ClientConfigurationTest, ConfigFileInParentDir) { ASSERT_TRUE(base::SaveToFile(config_path, config)); auto old_cwd = base::GetCurrentDir(); - ASSERT_TRUE(base::ChangeCurrentDir(Immutable::WrapString(child_dir))); + ASSERT_TRUE(base::ChangeCurrentDir(child_dir)); Configuration configuration; EXPECT_EQ(1, configuration.config().plugins_size()); - EXPECT_EQ('/', configuration.config().plugins(0).path()[0]) + EXPECT_TRUE(Path(configuration.config().plugins(0).path()).is_absolute()) << configuration.config().plugins(0).path(); base::ChangeCurrentDir(old_cwd); } diff --git a/src/client/driver_command.cc b/src/client/driver_command.cc index ccd268d9..fb384eb2 100644 --- a/src/client/driver_command.cc +++ b/src/client/driver_command.cc @@ -6,7 +6,7 @@ namespace dist_clang { namespace client { -base::ProcessPtr DriverCommand::CreateProcess(Immutable current_dir, +base::ProcessPtr DriverCommand::CreateProcess(const Path& current_dir, ui32 user_id) const { CHECK(!clang_); auto process = diff --git a/src/client/driver_command.hh b/src/client/driver_command.hh index 5fb6c52c..c9247b69 100644 --- a/src/client/driver_command.hh +++ b/src/client/driver_command.hh @@ -25,14 +25,14 @@ class DriverCommand : public Command { driver_(driver), clang_(new ClangCommand(driver_command.getArguments(), opts)) {} - base::ProcessPtr CreateProcess(Immutable current_dir, + base::ProcessPtr CreateProcess(const Path& current_dir, ui32 user_id) const override; String GetExecutable() const override; String RenderAllArgs() const override; bool CanFillFlags() const override { return !!clang_; } - inline bool FillFlags(base::proto::Flags* flags, const String& clang_path, + inline bool FillFlags(base::proto::Flags* flags, const Path& clang_path, const String& clang_major_version) const override { DCHECK(CanFillFlags()); return clang_->FillFlags(flags, clang_path, clang_major_version); diff --git a/src/daemon/absorber.cc b/src/daemon/absorber.cc index 84a7195a..63f136f6 100644 --- a/src/daemon/absorber.cc +++ b/src/daemon/absorber.cc @@ -101,14 +101,14 @@ cache::ExtraFiles Absorber::GetExtraFiles(const proto::Remote* message) { } bool Absorber::PrepareExtraFilesForCompiler( - const cache::ExtraFiles& extra_files, const String& temp_dir_path, + const cache::ExtraFiles& extra_files, const Path& temp_dir_path, base::proto::Flags* flags, net::proto::Status* status) { DCHECK(flags); DCHECK(status); auto sanitize_blacklist = extra_files.find(cache::SANITIZE_BLACKLIST); if (sanitize_blacklist != extra_files.end()) { - String sanitize_blacklist_file = temp_dir_path + "/sanitize_blacklist"; + String sanitize_blacklist_file = temp_dir_path / "sanitize_blacklist"; if (!base::File::Write(sanitize_blacklist_file, sanitize_blacklist->second)) { LOG(WARNING) << "Failed to write sanitize blacklist file " diff --git a/src/daemon/absorber.h b/src/daemon/absorber.h index a98a1b3a..9c9cdafe 100644 --- a/src/daemon/absorber.h +++ b/src/daemon/absorber.h @@ -26,7 +26,7 @@ class Absorber : public CompilationDaemon { cache::ExtraFiles GetExtraFiles(const proto::Remote* message); bool PrepareExtraFilesForCompiler(const cache::ExtraFiles& extra_files, - const String& temp_dir_path, + const Path& temp_dir_path, base::proto::Flags* flags, net::proto::Status* status); diff --git a/src/daemon/compilation_daemon.cc b/src/daemon/compilation_daemon.cc index 9dffb5a6..983b7ca3 100644 --- a/src/daemon/compilation_daemon.cc +++ b/src/daemon/compilation_daemon.cc @@ -88,8 +88,8 @@ bool ParseDeps(String deps, List& headers) { return true; } -inline String GetFullPath(const String& current_dir, const String& path) { - return path[0] == '/' ? path : current_dir + "/" + path; +inline String GetFullPath(const Path& current_dir, const Path& path) { + return path.is_absolute() ? path : current_dir / path; } } // namespace @@ -220,7 +220,7 @@ bool CompilationDaemon::SetupCompiler(base::proto::Flags* flags, } bool CompilationDaemon::ReadExtraFiles(const base::proto::Flags& flags, - const String& current_dir, + const Path& current_dir, ExtraFiles* extra_files) const { DCHECK(extra_files); DCHECK(extra_files->empty()); @@ -230,7 +230,7 @@ bool CompilationDaemon::ReadExtraFiles(const base::proto::Flags& flags, } Immutable sanitize_blacklist_contents; - const String sanitize_blacklist = + const Path sanitize_blacklist = GetFullPath(current_dir, flags.sanitize_blacklist()); if (!base::File::Read(sanitize_blacklist, &sanitize_blacklist_contents)) { LOG(CACHE_ERROR) << "Failed to read sanitize blacklist file" @@ -252,7 +252,7 @@ bool CompilationDaemon::SearchSimpleCache( } bool CompilationDaemon::SearchDirectCache( - const base::proto::Flags& flags, const String& current_dir, + const base::proto::Flags& flags, const Path& current_dir, cache::FileCache::Entry* entry) const { auto conf = this->conf(); @@ -267,7 +267,7 @@ bool CompilationDaemon::SearchDirectCache( Counter counter(Metric::DIRECT_CACHE_LOOKUP_TIME); const Version version(flags.compiler().version()); - const String input = GetFullPath(current_dir, flags.input()); + const Path input = GetFullPath(current_dir, flags.input()); const CommandLine command_line(CommandLineForDirectCache(current_dir, flags)); UnhandledSource code; @@ -363,7 +363,7 @@ bool CompilationDaemon::Check(const Configuration& conf) const { // static base::ProcessPtr CompilationDaemon::CreateProcess( - const base::proto::Flags& flags, ui32 user_id, Immutable cwd_path) { + const base::proto::Flags& flags, ui32 user_id, const Path& cwd_path) { DCHECK(flags.compiler().has_path()); base::ProcessPtr process = base::Process::Create(flags.compiler().path(), cwd_path, user_id); @@ -404,7 +404,7 @@ base::ProcessPtr CompilationDaemon::CreateProcess( // static base::ProcessPtr CompilationDaemon::CreateProcess( - const base::proto::Flags& flags, Immutable cwd_path) { + const base::proto::Flags& flags, const Path& cwd_path) { return CreateProcess(flags, base::Process::SAME_UID, cwd_path); } diff --git a/src/daemon/compilation_daemon.h b/src/daemon/compilation_daemon.h index 04588b0f..8be889dd 100644 --- a/src/daemon/compilation_daemon.h +++ b/src/daemon/compilation_daemon.h @@ -13,9 +13,9 @@ class CompilationDaemon : public BaseDaemon { static base::ProcessPtr CreateProcess(const base::proto::Flags& flags, ui32 user_id, - Immutable cwd_path = Immutable()); + const Path& cwd_path = Path()); static base::ProcessPtr CreateProcess(const base::proto::Flags& flags, - Immutable cwd_path = Immutable()); + const Path& cwd_path = Path()); static cache::string::HandledHash GenerateHash( const base::proto::Flags& flags, const cache::string::HandledSource& code, @@ -28,14 +28,14 @@ class CompilationDaemon : public BaseDaemon { net::proto::Status* status) const; bool ReadExtraFiles(const base::proto::Flags& flags, - const String& current_dir, + const Path& current_dir, cache::ExtraFiles* extra_files) const; bool SearchSimpleCache(const cache::string::HandledHash& hash, cache::FileCache::Entry* entry) const; bool SearchDirectCache(const base::proto::Flags& flags, - const String& current_dir, + const Path& current_dir, cache::FileCache::Entry* entry) const; void UpdateSimpleCache(const cache::string::HandledHash& hash, diff --git a/src/daemon/emitter.cc b/src/daemon/emitter.cc index d48beb57..26a5ac57 100644 --- a/src/daemon/emitter.cc +++ b/src/daemon/emitter.cc @@ -22,21 +22,23 @@ using Counter = perf::Counter; namespace { -inline String GetOutputPath(const base::proto::Local* WEAK_PTR message) { +inline Path GetOutputPath(const base::proto::Local* WEAK_PTR message) { DCHECK(message); - if (message->flags().output()[0] == '/') { + const Path output_path(message->flags().output()); + if (output_path.is_absolute()) { return message->flags().output(); } else { - return message->current_dir() + "/" + message->flags().output(); + return Path(message->current_dir()) / output_path; } } inline String GetDepsPath(const base::proto::Local* WEAK_PTR message) { DCHECK(message); - if (message->flags().deps_file()[0] == '/') { + const Path deps_path(message->flags().deps_file()); + if (deps_path.is_absolute()) { return message->flags().deps_file(); } else { - return message->current_dir() + "/" + message->flags().deps_file(); + return Path(message->current_dir()) / deps_path; } } @@ -60,10 +62,10 @@ inline bool GenerateSource(const base::proto::Local* WEAK_PTR message, base::ProcessPtr process; if (message->has_user_id()) { process = daemon::CompilationDaemon::CreateProcess( - pp_flags, message->user_id(), Immutable(message->current_dir())); + pp_flags, message->user_id(), message->current_dir()); } else { process = daemon::CompilationDaemon::CreateProcess( - pp_flags, Immutable(message->current_dir())); + pp_flags, message->current_dir()); } if (!process->Run(10)) { @@ -231,7 +233,7 @@ void Emitter::DoCheckCache(const base::WorkerPool& pool) { auto RestoreFromCache = [&](const HandledSource& source, const cache::ExtraFiles& extra_files) { String error; - const String output_path = GetOutputPath(incoming); + const Path output_path = GetOutputPath(incoming); if (!base::File::Write(output_path, entry.object, &error)) { LOG(ERROR) << "Failed to write file from cache: " << output_path @@ -344,7 +346,7 @@ void Emitter::DoLocalExecute(const base::WorkerPool& pool) { incoming->has_user_id() ? incoming->user_id() : base::Process::SAME_UID; Counter<> counter(Metric::LOCAL_COMPILATION_TIME); base::ProcessPtr process = CreateProcess( - incoming->flags(), uid, Immutable(incoming->current_dir())); + incoming->flags(), uid, incoming->current_dir()); if (!process->Run(base::Process::UNLIMITED, &error)) { status.set_code(net::proto::Status::EXECUTION); if (!process->stderr().empty()) { @@ -518,7 +520,7 @@ void Emitter::DoRemoteExecute(const base::WorkerPool& pool, ResolveFn resolver, } } - const String output_path = GetOutputPath(incoming); + const Path output_path = GetOutputPath(incoming); if (reply->HasExtension(proto::Result::extension)) { auto* result = reply->MutableExtension(proto::Result::extension); if (result->has_from_cache() && result->from_cache()) { diff --git a/src/daemon/emitter_test.cc b/src/daemon/emitter_test.cc index 99f1cd9a..242dc20e 100644 --- a/src/daemon/emitter_test.cc +++ b/src/daemon/emitter_test.cc @@ -1536,7 +1536,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResult) { const auto plugin_name = "fake_plugin"_l; const auto plugin_path = "fake_plugin_path"_l; - const String output_path2 = String(temp_dir) + "/test2.o"; + const Path output_path2 = temp_dir.GetPath() / "test2.o"; // |output_path2| checks that everything works fine with absolute paths. conf.mutable_cache()->set_path(temp_dir); @@ -1571,7 +1571,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResult) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, "-x"_l, language, "-o"_l, output_path1, input_path1}), process->args_); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path1, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path1.c_str(), object_code)); } else if (run_count == 3) { EXPECT_EQ((Immutable::Rope{"-E"_l, "-x"_l, language, "-o"_l, "-"_l, @@ -1683,7 +1683,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForRemoteResult) { const auto plugin_path = "fake_plugin_path"_l; const auto remote_compilation_time = std::chrono::milliseconds(10); - const String output_path2 = String(temp_dir) + "/test2.o"; + const Path output_path2 = temp_dir.GetPath() / "test2.o"; // |output_path2| checks that everything works fine with absolute paths. conf.mutable_emitter()->set_only_failed(true); @@ -1939,7 +1939,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteFail) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, "-x"_l, language, "-o"_l, output_path1, input_path1}), process->args_); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path1, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path1.c_str(), object_code)); } }; @@ -2085,7 +2085,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteRejects) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, "-x"_l, language, "-o"_l, output_path1, input_path1}), process->args_); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path1, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path1.c_str(), object_code)); } }; @@ -2303,10 +2303,10 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { const auto header2_path = path + "/header2.h"_l; const auto source_code = "int main() {}"_l; - ASSERT_TRUE(base::File::Write(input1_path, source_code)); - ASSERT_TRUE(base::File::Write(input2_path, source_code)); - ASSERT_TRUE(base::File::Write(header1_path, "#define A"_l)); - ASSERT_TRUE(base::File::Write(header2_path, "#define B"_l)); + ASSERT_TRUE(base::File::Write(input1_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(input2_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header1_path.string_copy(), "#define A"_l)); + ASSERT_TRUE(base::File::Write(header2_path.string_copy(), "#define B"_l)); // Prepare configuration. const String compiler_version = "fake_compiler_version"; @@ -2356,7 +2356,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { "-x"_l, language, "-o"_l, "-"_l, input1_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps1_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps1_path.c_str(), deps_contents)); } else if (run_count == 2) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, @@ -2364,7 +2364,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { language, "-o"_l, output1_path, input1_path}), process->args_) << process->PrintArgs(); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output1_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output1_path.c_str(), object_code)); } }; @@ -2436,13 +2436,13 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { EXPECT_EQ(1u, metric.value()); Immutable cache_output; - EXPECT_TRUE(base::File::Exists(output2_path)); - EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); + EXPECT_TRUE(base::File::Exists(output2_path.string_copy())); + EXPECT_TRUE(base::File::Read(output2_path.string_copy(), &cache_output)); EXPECT_EQ(object_code, cache_output); Immutable cache_deps; - EXPECT_TRUE(base::File::Exists(deps2_path)); - EXPECT_TRUE(base::File::Read(deps2_path, &cache_deps)); + EXPECT_TRUE(base::File::Exists(deps2_path.string_copy())); + EXPECT_TRUE(base::File::Read(deps2_path.string_copy(), &cache_deps)); EXPECT_EQ(deps_contents, cache_deps); EXPECT_EQ(2u, run_count); @@ -2486,12 +2486,12 @@ TEST_F(EmitterTest, Immutable("test.o: test.cc header1.h header2.h "_l) + header1_path; const auto preprocessed_contents = "Any content should work"_l; - ASSERT_TRUE( - base::File::Write(preprocessed_header_path, preprocessed_contents)); + ASSERT_TRUE( base::File::Write(preprocessed_header_path.string_copy(), + preprocessed_contents)); - ASSERT_TRUE(base::File::Write(input_path, source_code)); - ASSERT_TRUE(base::File::Write(header1_path, "#define A"_l)); - ASSERT_TRUE(base::File::Write(header2_path, "#define B"_l)); + ASSERT_TRUE(base::File::Write(input_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header1_path.string_copy(), "#define A"_l)); + ASSERT_TRUE(base::File::Write(header2_path.string_copy(), "#define B"_l)); // Prepare configuration. const String compiler_version = "fake_compiler_version"; @@ -2528,7 +2528,7 @@ TEST_F(EmitterTest, "-x"_l, language, "-o"_l, "-"_l, input_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps_path.c_str(), deps_contents)); } else if (run_count == 2) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, @@ -2536,7 +2536,7 @@ TEST_F(EmitterTest, language, "-o"_l, output_path, input_path}), process->args_) << process->PrintArgs(); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path.c_str(), object_code)); } else if (run_count == 3) { EXPECT_EQ( @@ -2545,7 +2545,7 @@ TEST_F(EmitterTest, "-o"_l, "-"_l, input_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps_path.c_str(), preprocessed_deps_contents)); } }; @@ -2669,10 +2669,10 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { const auto header2_path = path + "/header2.h"_l; const auto source_code = "int main() {}"_l; - ASSERT_TRUE(base::File::Write(input1_path, source_code)); - ASSERT_TRUE(base::File::Write(input2_path, source_code)); - ASSERT_TRUE(base::File::Write(header1_path, "#define A"_l)); - ASSERT_TRUE(base::File::Write(header2_path, "#define B"_l)); + ASSERT_TRUE(base::File::Write(input1_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(input2_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header1_path.string_copy(), "#define A"_l)); + ASSERT_TRUE(base::File::Write(header2_path.string_copy(), "#define B"_l)); // Prepare configuration. const String compiler_version = "fake_compiler_version"; @@ -2758,7 +2758,7 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { language, "-o"_l, "-"_l, input1_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps1_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps1_path.c_str(), "test1.o: test1.cc header1.h header2.h"_l)); }; @@ -2828,8 +2828,8 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { EXPECT_EQ(1u, metric.value()); Immutable cache_output; - EXPECT_TRUE(base::File::Exists(output2_path)); - EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); + EXPECT_TRUE(base::File::Exists(output2_path.string_copy())); + EXPECT_TRUE(base::File::Read(output2_path.string_copy(), &cache_output)); EXPECT_EQ(object_code, cache_output); EXPECT_EQ(1u, run_count); @@ -2978,12 +2978,12 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { const auto preprocessed_contents = "Any content should work"_l; const auto sanitize_blacklist_contents = "fun:main"_l; - ASSERT_TRUE(base::File::Write(input_path, source_code)); - ASSERT_TRUE(base::File::Write(header_path, header_contents)); - ASSERT_TRUE( - base::File::Write(preprocessed_header_path, preprocessed_contents)); - ASSERT_TRUE( - base::File::Write(sanitize_blacklist_path, sanitize_blacklist_contents)); + ASSERT_TRUE(base::File::Write(input_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header_path.string_copy(), header_contents)); + ASSERT_TRUE(base::File::Write(preprocessed_header_path.string_copy(), + preprocessed_contents)); + ASSERT_TRUE(base::File::Write(sanitize_blacklist_path.string_copy(), + sanitize_blacklist_contents)); // Prepare configuration. const String compiler_version = "fake_compiler_version"; @@ -3034,7 +3034,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { "-o"_l, "-"_l, input_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps_path.c_str(), deps_contents)); } else if (run_count == 2) { EXPECT_EQ((Immutable::Rope{ @@ -3045,7 +3045,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { "-o"_l, output_path, input_path}), process->args_) << process->PrintArgs(); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path.c_str(), object_code)); } }; @@ -3095,9 +3095,9 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { const auto header_path = path + "/header.h"_l; const auto sanitize_blacklist_path = path + "/asan-blacklist.txt"_l; - ASSERT_TRUE(base::File::Write(input_path, source_code)); - ASSERT_TRUE(base::File::Write(header_path, header_contents)); - ASSERT_TRUE(base::File::Write(sanitize_blacklist_path, + ASSERT_TRUE(base::File::Write(input_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header_path.string_copy(), header_contents)); + ASSERT_TRUE(base::File::Write(sanitize_blacklist_path.string_copy(), sanitize_blacklist_contents)); net::Connection::ScopedMessage message(new net::Connection::Message); @@ -3135,13 +3135,13 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { EXPECT_EQ(1u, metric.value()); Immutable cache_output; - const auto output2_path = String(temp_dir2) + "/" + String(output_path); + const auto output2_path = temp_dir2.GetPath() / output_path.c_str(); EXPECT_TRUE(base::File::Exists(output2_path)); EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); EXPECT_EQ(object_code, cache_output); Immutable cache_deps; - const auto deps2_path = String(temp_dir2) + "/" + String(deps_path); + const auto deps2_path = temp_dir2.GetPath() / deps_path.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path)); EXPECT_TRUE(base::File::Read(deps2_path, &cache_deps)); EXPECT_EQ(deps_contents, cache_deps); @@ -3168,7 +3168,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { const base::TemporaryDir temp_dir; const auto path = Immutable(String(temp_dir)); const auto relpath = path + "/path1"_l; - ASSERT_TRUE(base::CreateDirectory(relpath)); + ASSERT_TRUE(base::CreateDirectory(Path(relpath.string_copy()))); const auto input_path = relpath + "/test.cc"_l; const auto header_path = relpath + "/header.h"_l; @@ -3177,16 +3177,16 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { const auto header_contents2 = "#define B"_l; const auto relpath2 = path + "/path2"_l; - ASSERT_TRUE(base::CreateDirectory(relpath2)); + ASSERT_TRUE(base::CreateDirectory(Path(relpath2.string_copy()))); const auto input_path2 = relpath2 + "/test.cc"_l; const auto header_path2 = relpath2 + "/header.h"_l; - ASSERT_TRUE(base::File::Write(input_path, source_code)); - ASSERT_TRUE(base::File::Write(header_path, header_contents)); + ASSERT_TRUE(base::File::Write(input_path.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header_path.string_copy(), header_contents)); - ASSERT_TRUE(base::File::Write(input_path2, source_code)); - ASSERT_TRUE(base::File::Write(header_path2, header_contents2)); + ASSERT_TRUE(base::File::Write(input_path2.string_copy(), source_code)); + ASSERT_TRUE(base::File::Write(header_path2.string_copy(), header_contents2)); // Prepare configuration. const String compiler_version = "fake_compiler_version"; @@ -3242,7 +3242,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { "-x"_l, language, "-o"_l, "-"_l, input_path}), process->args_); process->stdout_ = preprocessed_source; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps_path.c_str(), deps_contents)); } else if (run_count == 2) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, @@ -3250,14 +3250,14 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { language, "-o"_l, output_path, input_path}), process->args_) << process->PrintArgs(); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path.c_str(), object_code)); } else if (run_count == 3) { EXPECT_EQ((Immutable::Rope{"-E"_l, "-dependency-file"_l, deps_path2, "-x"_l, language, "-o"_l, "-"_l, input_path2}), process->args_); process->stdout_ = preprocessed_source2; - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + deps_path2, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / deps_path2.c_str(), deps_contents2)); } else if (run_count == 4) { EXPECT_EQ((Immutable::Rope{action, "-load"_l, plugin_path, @@ -3265,7 +3265,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { language, "-o"_l, output_path2, input_path2}), process->args_) << process->PrintArgs(); - EXPECT_TRUE(base::File::Write(process->cwd_path_ + "/"_l + output_path2, + EXPECT_TRUE(base::File::Write(process->cwd_path_ / output_path2.c_str(), object_code2)); } }; @@ -3332,25 +3332,25 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { emitter.reset(); Immutable cache_output; - const auto output2_path = String(temp_dir) + "/" + String(output_path); + const auto output2_path = temp_dir.GetPath() / output_path.c_str(); EXPECT_TRUE(base::File::Exists(output2_path)); EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); EXPECT_EQ(object_code, cache_output); Immutable cache_deps; - const auto deps2_path = String(temp_dir) + "/" + String(deps_path); + const auto deps2_path = temp_dir.GetPath() / deps_path.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path)); EXPECT_TRUE(base::File::Read(deps2_path, &cache_deps)); EXPECT_EQ(deps_contents, cache_deps); Immutable cache_output2; - const auto output2_path2 = String(temp_dir) + "/" + String(output_path2); + const auto output2_path2 = temp_dir.GetPath() / output_path2.c_str(); EXPECT_TRUE(base::File::Exists(output2_path2)); EXPECT_TRUE(base::File::Read(output2_path2, &cache_output2)); EXPECT_EQ(object_code2, cache_output2); Immutable cache_deps2; - const auto deps2_path2 = String(temp_dir) + "/" + String(deps_path2); + const auto deps2_path2 = temp_dir.GetPath() / deps_path2.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path2)); EXPECT_TRUE(base::File::Read(deps2_path2, &cache_deps2)); EXPECT_EQ(deps_contents2, cache_deps2); diff --git a/src/net/connection_linux_test.cc b/src/net/connection_linux_test.cc index 70ab69ea..4b87cbf5 100644 --- a/src/net/connection_linux_test.cc +++ b/src/net/connection_linux_test.cc @@ -63,7 +63,7 @@ class TestServer : public EventLoop { if (tmp_dir_.GetPath().empty()) { return false; } - socket_path_ = tmp_dir_.GetPath() + "/socket"; + socket_path_ = tmp_dir_.GetPath() / "socket"; sockaddr_un address; address.sun_family = AF_UNIX; diff --git a/src/net/network_service_impl.cc b/src/net/network_service_impl.cc index 5be63883..1658d5a9 100644 --- a/src/net/network_service_impl.cc +++ b/src/net/network_service_impl.cc @@ -49,7 +49,7 @@ bool NetworkServiceImpl::Listen(const String& path, ListenCallback callback, if (!fd.Bind(peer, error)) { return false; } - base::SetPermissions(path, 0777); + base::SetPermissions(path, Perms::all); Passive passive(std::move(fd)); if (!passive.IsValid()) { diff --git a/tools/clang/main.cc b/tools/clang/main.cc index 7af7a0eb..0d3e997d 100644 --- a/tools/clang/main.cc +++ b/tools/clang/main.cc @@ -67,7 +67,7 @@ int main(int argc, char* argv[]) { // objects! // FIXME: move |configuration| et al. inside |DoMain()| after mocking |GetEnv| if (client::DoMain(argc, argv, socket_path, - Immutable::WrapString(config.path()), + config.path(), Immutable::WrapString(config.version()), config.connect_timeout(), config.read_timeout(), config.send_timeout(), config.read_minimum(), From f7911240f1a6b496c79ecc3c2f8a1f265c11aaa6 Mon Sep 17 00:00:00 2001 From: Matvey Larionov Date: Mon, 17 Jul 2017 16:17:09 +0300 Subject: [PATCH 2/2] Remove base::TemporaryDir::GetPath() --- dist-clang.files | 1 + src/base/BUILD.gn | 1 + src/base/file/file.cc | 150 +++++++++++++++++++++++++++++ src/base/file/file.h | 1 - src/base/file/file_posix.cc | 159 ++----------------------------- src/base/file/file_test.cc | 19 ++-- src/base/file_utils.cc | 17 ++++ src/base/file_utils_test.cc | 11 +-- src/base/temporary_dir.h | 3 +- src/cache/file_cache_test.cc | 72 +++++++------- src/client/configuration_test.cc | 5 +- src/daemon/absorber.cc | 2 +- src/daemon/absorber_test.cc | 6 +- src/daemon/emitter_test.cc | 92 +++++++++--------- src/net/connection_linux_test.cc | 4 +- 15 files changed, 283 insertions(+), 260 deletions(-) create mode 100644 src/base/file/file.cc diff --git a/dist-clang.files b/dist-clang.files index d4ebf845..77a90db8 100644 --- a/dist-clang.files +++ b/dist-clang.files @@ -2202,6 +2202,7 @@ src/base/file/data.cc src/base/file/data.h src/base/file/epoll_linux.cc src/base/file/epoll_linux.h +src/base/file/file.cc src/base/file/file.h src/base/file/file_posix.cc src/base/file/file_test.cc diff --git a/src/base/BUILD.gn b/src/base/BUILD.gn index 40a3ab90..92e72a49 100644 --- a/src/base/BUILD.gn +++ b/src/base/BUILD.gn @@ -30,6 +30,7 @@ shared_library("base") { "file/data.h", "file/epoll_linux.cc", "file/epoll_linux.h", + "file/file.cc", "file/file.h", "file/file_posix.cc", "file/handle_posix.cc", diff --git a/src/base/file/file.cc b/src/base/file/file.cc new file mode 100644 index 00000000..d78cbc3a --- /dev/null +++ b/src/base/file/file.cc @@ -0,0 +1,150 @@ +#include + +#include +#include +#include + +#include STL(cstdio) +#include STL_EXPERIMENTAL(filesystem) +#include STL(system_error) + +namespace dist_clang { + +namespace { + +bool GetStatus(const Path& path, + std::experimental::filesystem::file_status* status, + String* error) { + CHECK(status); + std::error_code ec; + *status = std::experimental::filesystem::status(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return false; + } + return true; +} + +} // anonymous namespace + +namespace base { + +// static +bool File::IsFile(const Path& path, String* error) { + std::experimental::filesystem::file_status status; + if (!GetStatus(path, &status, error)) { + return false; + } + return !std::experimental::filesystem::is_directory(status); +} + +bool File::Hash(Immutable* output, const List& skip_list, + String* error) { + DCHECK(IsValid()); + + Immutable tmp_output; + if (!Read(&tmp_output, error)) { + return false; + } + + for (const char* skip : skip_list) { + if (tmp_output.find(skip) != String::npos) { + if (error) { + error->assign("Skip-list hit: " + String(skip)); + } + return false; + } + } + + output->assign(base::Hexify(tmp_output.Hash())); + return true; +} + +// static +ui64 File::Size(const Path& path, String* error) { + std::error_code ec; + const auto file_size = std::experimental::filesystem::file_size(path, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return 0; + } + return static_cast(file_size); +} + +// static +bool File::Read(const Path& path, Immutable* output, String* error) { + File file(path); + + if (!file.IsValid()) { + file.GetCreationError(error); + return false; + } + + return file.Read(output, error); +} + +// static +bool File::Exists(const Path& path, String* error) { + std::experimental::filesystem::file_status status; + if (!GetStatus(path, &status, error)) { + return false; + } + return std::experimental::filesystem::exists(status) && + !std::experimental::filesystem::is_directory(status); +} + +// static +bool File::Hash(const Path& path, Immutable* output, + const List& skip_list, String* error) { + File file(path); + + if (!file.IsValid()) { + file.GetCreationError(error); + return false; + } + + return file.Hash(output, skip_list, error); +} + +// static +bool File::Copy(const Path& src_path, const Path& dst_path, String* error) { + File src(src_path); + + if (!src.IsValid()) { + src.GetCreationError(error); + return false; + } + + return src.CopyInto(dst_path, error); +} + +// static +bool File::Move(const Path& src, const Path& dst, String* error) { + std::error_code ec; + std::experimental::filesystem::rename(src, dst, ec); + if (ec) { + if (error) { + *error = ec.message(); + } + return false; + } + return true; +} + +// static +bool File::Delete(const Path& path, String* error) { + if (std::remove(path.c_str())) { + GetLastError(error); + return false; + } + + return true; +} + +} // namespace base + +} // namespace dist_clang diff --git a/src/base/file/file.h b/src/base/file/file.h index 7f5cb99c..f992548e 100644 --- a/src/base/file/file.h +++ b/src/base/file/file.h @@ -47,7 +47,6 @@ class File final : public Data { File(const Path& path, ui64 size); // Open truncated write-only file bool Close(String* error = nullptr); - String path_; String error_; String move_on_close_; }; diff --git a/src/base/file/file_posix.cc b/src/base/file/file_posix.cc index 6f13b958..c029fd3e 100644 --- a/src/base/file/file_posix.cc +++ b/src/base/file/file_posix.cc @@ -3,10 +3,6 @@ #include #include #include -#include - -#include STL_EXPERIMENTAL(filesystem) -#include STL(system_error) #include #include @@ -20,33 +16,10 @@ namespace dist_clang { -namespace { - -bool GetStatus(const Path& path, - std::experimental::filesystem::file_status* status, - String* error) { - CHECK(status); - std::error_code ec; - *status = std::experimental::filesystem::status(path, ec); - if (ec) { - if (error) { - *error = ec.message(); - } - return false; - } - return true; -} - -Path AddTempEnding(const Path& path) { - return base::AppendExtension(path, ".tmp"); -} - -} // anonymous namespace - namespace base { File::File(const Path& path) - : Data(open(path.c_str(), O_RDONLY | O_CLOEXEC)), path_(path) { + : Data(open(path.c_str(), O_RDONLY | O_CLOEXEC)) { if (!IsValid()) { GetLastError(&error_); return; @@ -73,15 +46,6 @@ File::File(const Path& path) #endif } -// static -bool File::IsFile(const Path& path, String* error) { - std::experimental::filesystem::file_status status; - if (!GetStatus(path, &status, error)) { - return false; - } - return std::experimental::filesystem::is_regular_file(status); -} - // static bool File::IsExecutable(const Path& path, String* error) { if (!IsFile(path, error)) { @@ -95,20 +59,16 @@ bool File::IsExecutable(const Path& path, String* error) { return true; } -// static -bool File::Exists(const Path& path, String* error) { - std::experimental::filesystem::file_status status; - if (!GetStatus(path, &status, error)) { - return false; - } - return std::experimental::filesystem::exists(status) && - std::experimental::filesystem::is_regular_file(status); -} - ui64 File::Size(String* error) const { DCHECK(IsValid()); - return File::Size(path_, error); + struct stat buffer; + if (fstat(native(), &buffer)) { + GetLastError(error); + return 0; + } + + return buffer.st_size; } bool File::Read(Immutable* output, String* error) { @@ -145,28 +105,6 @@ bool File::Read(Immutable* output, String* error) { return true; } -bool File::Hash(Immutable* output, const List& skip_list, - String* error) { - DCHECK(IsValid()); - - Immutable tmp_output; - if (!Read(&tmp_output, error)) { - return false; - } - - for (const char* skip : skip_list) { - if (tmp_output.find(skip) != String::npos) { - if (error) { - error->assign("Skip-list hit: " + String(skip)); - } - return false; - } - } - - output->assign(base::Hexify(tmp_output.Hash())); - return true; -} - bool File::CopyInto(const Path& dst_path, String* error) { DCHECK(IsValid()); @@ -214,56 +152,6 @@ bool File::CopyInto(const Path& dst_path, String* error) { return result; } -// static -ui64 File::Size(const Path& path, String* error) { - std::error_code ec; - const auto file_size = std::experimental::filesystem::file_size(path, ec); - if (ec) { - if (error) { - *error = ec.message(); - } - return 0; - } - return static_cast(file_size); -} - -// static -bool File::Read(const Path& path, Immutable* output, String* error) { - File file(path); - - if (!file.IsValid()) { - file.GetCreationError(error); - return false; - } - - return file.Read(output, error); -} - -// static -bool File::Hash(const Path& path, Immutable* output, - const List& skip_list, String* error) { - File file(path); - - if (!file.IsValid()) { - file.GetCreationError(error); - return false; - } - - return file.Hash(output, skip_list, error); -} - -// static -bool File::Delete(const Path& path, String* error) { - std::error_code ec; - if (!std::experimental::filesystem::remove(path, ec)) { - if (error) { - *error = ec.message(); - } - return false; - } - return true; -} - // static bool File::Write(const Path& path, Immutable input, String* error) { File dst(path, input.size()); @@ -291,31 +179,6 @@ bool File::Write(const Path& path, Immutable input, String* error) { return total_bytes == input.size(); } -// static -bool File::Copy(const Path& src_path, const Path& dst_path, String* error) { - File src(src_path); - - if (!src.IsValid()) { - src.GetCreationError(error); - return false; - } - - return src.CopyInto(dst_path, error); -} - -// static -bool File::Move(const Path& src, const Path& dst, String* error) { - std::error_code ec; - std::experimental::filesystem::rename(src, dst, ec); - if (ec) { - if (error) { - *error = ec.message(); - } - return false; - } - return true; -} - File::File(const Path& path, ui64 size) : Data([=] { // We need write-access even on object files after introduction of the @@ -323,10 +186,9 @@ File::File(const Path& path, ui64 size) // https://sourceware.org/bugzilla/show_bug.cgi?id=971 const auto mode = mode_t(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); const auto flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC; - const Path tmp_path = AddTempEnding(path); + const String tmp_path = path.string() + ".tmp"; return open(tmp_path.c_str(), flags, mode); }()), - path_(AddTempEnding(path)), move_on_close_(path) { if (!IsValid()) { GetLastError(&error_); @@ -370,13 +232,12 @@ bool File::Close(String* error) { return true; } - const Path tmp_path = AddTempEnding(move_on_close_); + const String tmp_path = move_on_close_ + ".tmp"; if (!Move(tmp_path, move_on_close_, error)) { Delete(tmp_path); return false; } - path_ = move_on_close_; return true; } diff --git a/src/base/file/file_test.cc b/src/base/file/file_test.cc index a5f823b6..2f1a7191 100644 --- a/src/base/file/file_test.cc +++ b/src/base/file/file_test.cc @@ -15,8 +15,7 @@ namespace base { TEST(FileTest, Read) { const auto expected_content = "All your base are belong to us"_l; const base::TemporaryDir temp_dir; - const Path temp_dir_path = temp_dir.GetPath(); - const Path file_path = temp_dir_path / "file"; + const Path file_path = Path(temp_dir) / "file"; { std::ofstream file(file_path, std::ios_base::out | std::ios_base::trunc); @@ -43,7 +42,7 @@ TEST(FileTest, Read) { TEST(FileTest, Write) { const auto expected_content = "All your base are belong to us"_l; const base::TemporaryDir temp_dir; - const Path file_path = temp_dir.GetPath() / "file"; + const Path file_path = Path(temp_dir) / "file"; String error; EXPECT_TRUE(File::Write(file_path, expected_content, &error)) << error; @@ -54,14 +53,14 @@ TEST(FileTest, Write) { file.read(content, expected_content.size()); EXPECT_EQ(expected_content, String(content, expected_content.size())); + // Can't write to directory. EXPECT_FALSE(File::Write(temp_dir, expected_content)); } TEST(FileTest, Size) { const base::TemporaryDir temp_dir; - const Path temp_dir_path = temp_dir.GetPath(); - const Path file_path = temp_dir_path / "file"; + const Path file_path = Path(temp_dir) / "file"; const String content = "1234567890"; { @@ -83,8 +82,7 @@ TEST(FileTest, Hash) { const auto content = "All your base are belong to us"_l; const auto expected_hash = "c9e92e37df1e856cbd0abffe104225b8"_l; const base::TemporaryDir temp_dir; - const Path temp_dir_path = temp_dir.GetPath(); - const String file_path = temp_dir_path / "file"; + const String file_path = Path(temp_dir) / "file"; { std::ofstream file(file_path, std::ios_base::out | std::ios_base::trunc); @@ -117,10 +115,9 @@ TEST(FileTest, Copy) { const auto expected_content1 = "All your base are belong to us"_l; const auto expected_content2 = "Nothing lasts forever"_l; const TemporaryDir temp_dir; - const Path temp_dir_path = temp_dir.GetPath(); - const Path file1 = temp_dir_path / "1"; - const Path file2 = temp_dir_path / "2"; - const Path file3 = temp_dir_path / "3"; + const Path file1 = Path(temp_dir) / "1"; + const Path file2 = Path(temp_dir) / "2"; + const Path file3 = Path(temp_dir) / "3"; ASSERT_TRUE(File::Write(file1, expected_content1)); ASSERT_TRUE(File::Copy(file1, file2)); diff --git a/src/base/file_utils.cc b/src/base/file_utils.cc index 02973019..7ecf5605 100644 --- a/src/base/file_utils.cc +++ b/src/base/file_utils.cc @@ -92,6 +92,7 @@ bool ChangeOwner(const Path& path, ui32 uid, String* error) { } bool CreateDirectory(const Path& path, String* error) { +#if defined(OS_WIN) std::error_code ec; std::experimental::filesystem::create_directories(path, ec); if (ec) { @@ -100,6 +101,22 @@ bool CreateDirectory(const Path& path, String* error) { } return false; } +#else + const String& path_str = path.string(); + for (size_t i = 1; i < path_str.size(); ++i) { + if (path_str[i] == '/' && + mkdir(path_str.substr(0, i).c_str(), 0755) == -1 && + errno != EEXIST) { + GetLastError(error); + return false; + } + } + + if (mkdir(path_str.c_str(), 0755) == -1 && errno != EEXIST) { + GetLastError(error); + return false; + } +#endif return true; } diff --git a/src/base/file_utils_test.cc b/src/base/file_utils_test.cc index 38f62efa..b936548e 100644 --- a/src/base/file_utils_test.cc +++ b/src/base/file_utils_test.cc @@ -17,10 +17,9 @@ namespace base { TEST(FileUtilsTest, CalculateDirectorySize) { const base::TemporaryDir temp_dir; - const Path temp_dir_path(temp_dir.GetPath()); - const Path dir1 = temp_dir_path / "1"; - const Path dir2 = temp_dir_path / "2"; - const Path file1 = temp_dir_path / "file1"; + const Path dir1 = Path(temp_dir) / "1"; + const Path dir2 = Path(temp_dir) / "2"; + const Path file1 = Path(temp_dir) / "file1"; const Path file2 = dir1 / "file2"; const Path file3 = dir2 / "file3"; const String content1 = "a"; @@ -66,9 +65,9 @@ TEST(FileUtilsTest, TempFile) { TEST(FileUtilsTest, CreateDirectory) { String error; const base::TemporaryDir temp_dir; - const Path& temp = temp_dir.GetPath() / "1" / "2" / "3"; + const Path& temp = Path(temp_dir) / "1" / "2" / "3"; - ASSERT_TRUE(CreateDirectory(temp.string(), &error)) << error; + ASSERT_TRUE(CreateDirectory(temp, &error)) << error; std::error_code ec; EXPECT_TRUE(std::experimental::filesystem::exists(temp, ec)); diff --git a/src/base/temporary_dir.h b/src/base/temporary_dir.h index f36b90bd..cfe8252c 100644 --- a/src/base/temporary_dir.h +++ b/src/base/temporary_dir.h @@ -10,12 +10,11 @@ class TemporaryDir { TemporaryDir(); ~TemporaryDir(); - inline const Path& GetPath() const { return path_; } inline const String& GetError() const { return error_; } + inline const String str() const { return path_; } inline operator bool() const { return path_.empty(); } inline operator Path() const { return path_; } - inline operator String() const { return path_; } private: Path path_; diff --git a/src/cache/file_cache_test.cc b/src/cache/file_cache_test.cc index e52d6145..442d6b40 100644 --- a/src/cache/file_cache_test.cc +++ b/src/cache/file_cache_test.cc @@ -43,7 +43,7 @@ TEST(FileCacheTest, ExtraFilesGiveDifferentHash) { TEST(FileCacheTest, LockNonExistentFile) { const base::TemporaryDir tmp_dir; - const Path absent_path = tmp_dir.GetPath() / "absent_file"; + const Path absent_path = Path(tmp_dir) / "absent_file"; FileCache cache(tmp_dir); { @@ -58,7 +58,7 @@ TEST(FileCacheTest, LockNonExistentFile) { TEST(FileCacheTest, DoubleLocks) { const base::TemporaryDir tmp_dir; - const Path file_path = tmp_dir.GetPath() / "file"; + const Path file_path = Path(tmp_dir) / "file"; FileCache cache(tmp_dir); ASSERT_TRUE(base::File::Write(file_path, "1"_l)); @@ -159,12 +159,12 @@ TEST(FileCacheTest, RemoveEntry) { TEST(FileCacheTest, RestoreSingleEntry) { const base::TemporaryDir tmp_dir; - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(tmp_dir.GetPath()); + FileCache cache(tmp_dir); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -194,12 +194,12 @@ TEST(FileCacheTest, RestoreSingleEntry) { TEST(FileCacheTest, RestoreSingleEntryWithExtraFile) { const base::TemporaryDir tmp_dir; - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(tmp_dir.GetPath()); + FileCache cache(tmp_dir); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -236,12 +236,12 @@ TEST(FileCacheTest, RestoreSingleEntryWithExtraFile) { TEST(FileCacheTest, RestoreEntryWithMissingFile) { const base::TemporaryDir tmp_dir; - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(tmp_dir.GetPath()); + FileCache cache(tmp_dir); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -304,7 +304,7 @@ TEST(FileCacheTest, DISABLED_BadInitialCacheSize) { TEST(FileCacheTest, ExceedCacheSize) { const base::TemporaryDir tmp_dir; - const Path cache_path = tmp_dir.GetPath() / "cache"; + const Path cache_path = Path(tmp_dir) / "cache"; const Literal obj_content[] = {"22"_l, "333"_l, "4444"_l}; const HandledSource code[] = {HandledSource("int main() { return 0; }"_l), HandledSource("int main() { return 1; }"_l), @@ -350,15 +350,15 @@ TEST(FileCacheTest, ExceedCacheSize) { TEST(FileCacheTest, RestoreDirectEntry) { const base::TemporaryDir tmp_dir; - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; - const Path header1_path = tmp_dir.GetPath() / "test1.h"; - const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; + const Path header1_path = Path(tmp_dir) / "test1.h"; + const Path header2_path = Path(tmp_dir) / "test2.h"; const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(tmp_dir.GetPath()); + FileCache cache(tmp_dir); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry1, entry2; @@ -382,11 +382,11 @@ TEST(FileCacheTest, RestoreDirectEntry) { // Store the direct entry. const UnhandledSource orig_code("int main() {}"_l); const List headers = {header1_path, header2_rel_path}; - cache.Store(orig_code, {}, cl, version, headers, {}, tmp_dir.GetPath(), hash); + cache.Store(orig_code, {}, cl, version, headers, {}, Path(tmp_dir), hash); // Restore the entry. ASSERT_TRUE(cache.Find( - orig_code, {}, cl, version, tmp_dir.GetPath(), &entry2)); + orig_code, {}, cl, version, Path(tmp_dir), &entry2)); EXPECT_EQ(expected_object_code, entry2.object); EXPECT_EQ(expected_deps, entry2.deps); EXPECT_EQ(expected_stderr, entry2.stderr); @@ -394,11 +394,11 @@ TEST(FileCacheTest, RestoreDirectEntry) { TEST(FileCacheTest, RestoreDirectEntryWithExtraFile) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; - const Path header1_path = tmp_dir.GetPath() / "test1.h"; - const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path path = Path(tmp_dir); + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; + const Path header1_path = Path(tmp_dir) / "test1.h"; + const Path header2_path = Path(tmp_dir) / "test2.h"; const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; @@ -448,15 +448,15 @@ TEST(FileCacheTest, RestoreDirectEntryWithExtraFile) { TEST(FileCacheTest, DirectEntry_ChangedHeaderContents) { const base::TemporaryDir tmp_dir; - const Path object_path = tmp_dir.GetPath() / "test.o"; - const Path deps_path = tmp_dir.GetPath() / "test.d"; - const Path header1_path = tmp_dir.GetPath() / "test1.h"; - const Path header2_path = tmp_dir.GetPath() / "test2.h"; + const Path object_path = Path(tmp_dir) / "test.o"; + const Path deps_path = Path(tmp_dir) / "test.d"; + const Path header1_path = Path(tmp_dir) / "test1.h"; + const Path header2_path = Path(tmp_dir) / "test2.h"; const Path header2_rel_path = "test2.h"; const auto expected_stderr = "some warning"_l; const auto expected_object_code = "some object code"_l; const auto expected_deps = "some deps"_l; - FileCache cache(tmp_dir.GetPath()); + FileCache cache(tmp_dir); ASSERT_TRUE(cache.Run(1)); FileCache::Entry entry; @@ -480,19 +480,19 @@ TEST(FileCacheTest, DirectEntry_ChangedHeaderContents) { // Store the direct entry. const UnhandledSource orig_code("int main() {}"_l); const List headers = {header1_path, header2_rel_path}; - cache.Store(orig_code, {}, cl, version, headers, {}, tmp_dir.GetPath(), hash); + cache.Store(orig_code, {}, cl, version, headers, {}, Path(tmp_dir), hash); // Change header contents. ASSERT_TRUE(base::File::Write(header2_path, "#define C"_l)); // Restore the entry. EXPECT_FALSE(cache.Find( - orig_code, {}, cl, version, tmp_dir.GetPath(), &entry)); + orig_code, {}, cl, version, Path(tmp_dir), &entry)); } TEST(FileCacheTest, DirectEntry_ChangedPreprocessedHeaderContents) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); + const Path path = Path(tmp_dir); const Path object_path = path / "test.o"; const Path deps_path = path / "test.d"; const Path header1_path = path / "test1.h"; @@ -540,7 +540,7 @@ TEST(FileCacheTest, DirectEntry_ChangedPreprocessedHeaderContents) { TEST(FileCacheTest, DirectEntry_RewriteManifest) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); + const Path path = Path(tmp_dir); const Path object_path = path / "test.o"; const Path deps_path = path / "test.d"; const Path header1_path = path / "test1.h"; @@ -586,7 +586,7 @@ TEST(FileCacheTest, DirectEntry_RewriteManifest) { TEST(FileCacheTest, DirectEntry_ChangedOriginalCode) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); + const Path path = Path(tmp_dir); const Path object_path = path / "test.o"; const Path deps_path = path / "test.d"; const Path header1_path = path / "test1.h"; @@ -627,7 +627,7 @@ TEST(FileCacheTest, DirectEntry_ChangedOriginalCode) { TEST(FileCacheTest, DirectEntry_ChangedExtraFile) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); + const Path path = Path(tmp_dir); const Path object_path = path / "test.o"; const Path deps_path = path / "test.d"; const Path header1_path = path / "test1.h"; @@ -673,7 +673,7 @@ TEST(FileCacheTest, DirectEntry_ChangedExtraFile) { TEST(FileCacheTest, RestoreAndMigrateSnappyEntry) { const base::TemporaryDir tmp_dir; - const Path path = tmp_dir.GetPath(); + const Path path(tmp_dir); const Path object_path = path / "test.o"; const Path deps_path = path / "test.d"; const auto expected_stderr = "some warning"_l; diff --git a/src/client/configuration_test.cc b/src/client/configuration_test.cc index 23d75681..69c72c9c 100644 --- a/src/client/configuration_test.cc +++ b/src/client/configuration_test.cc @@ -14,9 +14,8 @@ namespace client { TEST(ClientConfigurationTest, ConfigFileInParentDir) { const base::TemporaryDir tmp_dir; - const Path temp_dir_path = tmp_dir.GetPath(); - const Path child_dir = temp_dir_path / "1"; - const Path config_path = temp_dir_path / ".distclang"; + const Path child_dir = Path(tmp_dir) / "1"; + const Path config_path = Path(tmp_dir) / ".distclang"; ASSERT_TRUE(base::CreateDirectory(child_dir)); diff --git a/src/daemon/absorber.cc b/src/daemon/absorber.cc index 63f136f6..99ac5ed2 100644 --- a/src/daemon/absorber.cc +++ b/src/daemon/absorber.cc @@ -194,7 +194,7 @@ void Absorber::DoExecute(const base::WorkerPool& pool) { } base::TemporaryDir temp_dir; - if (!PrepareExtraFilesForCompiler(extra_files, temp_dir.GetPath(), + if (!PrepareExtraFilesForCompiler(extra_files, Path(temp_dir), incoming->mutable_flags(), &status)) { task->first->ReportStatus(status); continue; diff --git a/src/daemon/absorber_test.cc b/src/daemon/absorber_test.cc index e115bcd1..7a4c781b 100644 --- a/src/daemon/absorber_test.cc +++ b/src/daemon/absorber_test.cc @@ -250,7 +250,7 @@ TEST_F(AbsorberTest, StoreLocalCacheWithoutBlacklist) { conf.mutable_absorber()->mutable_local()->set_host(expected_host); conf.mutable_absorber()->mutable_local()->set_port(expected_port); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -354,7 +354,7 @@ TEST_F(AbsorberTest, StoreLocalCacheWithBlacklist) { conf.mutable_absorber()->mutable_local()->set_host(expected_host); conf.mutable_absorber()->mutable_local()->set_port(expected_port); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -463,7 +463,7 @@ TEST_F(AbsorberTest, StoreLocalCacheWithAndWithoutBlacklist) { conf.mutable_absorber()->mutable_local()->set_host(expected_host); conf.mutable_absorber()->mutable_local()->set_port(expected_port); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); diff --git a/src/daemon/emitter_test.cc b/src/daemon/emitter_test.cc index 242dc20e..7dea8e14 100644 --- a/src/daemon/emitter_test.cc +++ b/src/daemon/emitter_test.cc @@ -238,7 +238,7 @@ TEST_F(EmitterTest, ConfigurationUpdateFromCoordinator) { conf.mutable_emitter()->set_only_failed(true); conf.mutable_emitter()->set_poll_interval(1u); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -400,7 +400,7 @@ TEST_F(EmitterTest, ConfigurationUpdateFromCoordinator) { auto message = std::make_unique(); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); auto* flags = extension->mutable_flags(); flags->mutable_compiler()->set_version(compiler_version); @@ -430,7 +430,7 @@ TEST_F(EmitterTest, ConfigurationUpdateFromCoordinator) { auto message = std::make_unique(); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); auto* flags = extension->mutable_flags(); flags->mutable_compiler()->set_version(compiler_version); @@ -1536,10 +1536,10 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResult) { const auto plugin_name = "fake_plugin"_l; const auto plugin_path = "fake_plugin_path"_l; - const Path output_path2 = temp_dir.GetPath() / "test2.o"; + const Path output_path2 = Path(temp_dir) / "test2.o"; // |output_path2| checks that everything works fine with absolute paths. - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -1591,7 +1591,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path1); extension->mutable_flags()->set_output(output_path1); @@ -1618,7 +1618,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path2); extension->mutable_flags()->set_output(output_path2); @@ -1683,11 +1683,11 @@ TEST_F(EmitterTest, StoreSimpleCacheForRemoteResult) { const auto plugin_path = "fake_plugin_path"_l; const auto remote_compilation_time = std::chrono::milliseconds(10); - const Path output_path2 = temp_dir.GetPath() / "test2.o"; + const Path output_path2 = Path(temp_dir) / "test2.o"; // |output_path2| checks that everything works fine with absolute paths. conf.mutable_emitter()->set_only_failed(true); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -1772,7 +1772,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForRemoteResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path1); extension->mutable_flags()->set_output(output_path1); auto* compiler = extension->mutable_flags()->mutable_compiler(); @@ -1799,7 +1799,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForRemoteResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path2); extension->mutable_flags()->set_output(output_path2); @@ -1881,7 +1881,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteFail) { const auto local_compilation_time = std::chrono::milliseconds(10); conf.mutable_emitter()->set_only_failed(true); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -1955,7 +1955,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteFail) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path1); extension->mutable_flags()->set_output(output_path1); auto* compiler = extension->mutable_flags()->mutable_compiler(); @@ -2027,7 +2027,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteRejects) { const auto preprocess_time = std::chrono::milliseconds(10); conf.mutable_emitter()->set_only_failed(true); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -2101,7 +2101,7 @@ TEST_F(EmitterTest, FallbackToLocalCompilationAfterRemoteRejects) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path1); extension->mutable_flags()->set_output(output_path1); auto* compiler = extension->mutable_flags()->mutable_compiler(); @@ -2171,7 +2171,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResultWithAndWithoutBlacklist) { const auto plugin_path = "fake_plugin_path"_l; const auto sanitize_blacklist_path = "asan-blacklist.txt"_l; - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(false); conf.mutable_cache()->set_clean_period(1); @@ -2228,7 +2228,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResultWithAndWithoutBlacklist) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path1); @@ -2255,7 +2255,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResultWithAndWithoutBlacklist) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path2); @@ -2296,7 +2296,7 @@ TEST_F(EmitterTest, StoreSimpleCacheForLocalResultWithAndWithoutBlacklist) { TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { // Prepare environment. const base::TemporaryDir temp_dir; - const auto path = Immutable(String(temp_dir)); + const auto path = Immutable(temp_dir.str()); const auto input1_path = path + "/test1.cc"_l; const auto input2_path = path + "/test2.cc"_l; const auto header1_path = path + "/header1.h"_l; @@ -2314,7 +2314,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { const String plugin_name = "fake_plugin"; const auto plugin_path = "fake_plugin_path"_l; - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(true); conf.mutable_cache()->set_clean_period(1); @@ -2379,7 +2379,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input1_path); extension->mutable_flags()->set_output(output1_path); @@ -2407,7 +2407,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input2_path); extension->mutable_flags()->set_output(output2_path); @@ -2466,7 +2466,7 @@ TEST_F(EmitterTest, StoreDirectCacheForLocalResultWithAndWithoutIncludedHeaders) { // Prepare environment. const base::TemporaryDir temp_dir; - const auto path = Immutable(String(temp_dir)); + const auto path = Immutable(temp_dir.str()); const auto input_path = path + "/test.cc"_l; const auto header1_path = path + "/header1.h"_l; const auto header2_path = path + "/header2.h"_l; @@ -2499,7 +2499,7 @@ TEST_F(EmitterTest, const String plugin_name = "fake_plugin"; const auto plugin_path = "fake_plugin_path"_l; - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(true); conf.mutable_cache()->set_clean_period(1); @@ -2560,7 +2560,7 @@ TEST_F(EmitterTest, net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -2588,7 +2588,7 @@ TEST_F(EmitterTest, net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -2619,7 +2619,7 @@ TEST_F(EmitterTest, net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -2662,7 +2662,7 @@ TEST_F(EmitterTest, TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { // Prepare environment. const base::TemporaryDir temp_dir; - const auto path = Immutable(String(temp_dir)); + const auto path = Immutable(temp_dir.str()); const auto input1_path = path + "/test1.cc"_l; const auto input2_path = path + "/test2.cc"_l; const auto header1_path = path + "/header1.h"_l; @@ -2683,7 +2683,7 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { const ui16 port = 12345; conf.mutable_emitter()->set_only_failed(true); - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(true); conf.mutable_cache()->set_clean_period(1); @@ -2772,7 +2772,7 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input1_path); extension->mutable_flags()->set_output(output1_path); @@ -2800,7 +2800,7 @@ TEST_F(EmitterTest, StoreDirectCacheForRemoteResult) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input2_path); extension->mutable_flags()->set_output(output2_path); @@ -2968,7 +2968,7 @@ TEST_F(EmitterTest, ConfigurationUpdateCompiler) { TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { // Prepare environment. const base::TemporaryDir temp_dir1, temp_dir2; - const auto path = Immutable(String(temp_dir1)); + const auto path = Immutable(temp_dir1.str()); const auto input_path = path + "/test.cc"_l; const auto header_path = path + "/header.h"_l; const auto preprocessed_header_path = path + "header.h.pth"_l; @@ -2991,7 +2991,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { const String plugin_name = "fake_plugin"; const auto plugin_path = "fake_plugin_path"_l; - conf.mutable_cache()->set_path(temp_dir1); + conf.mutable_cache()->set_path(temp_dir1.str()); conf.mutable_cache()->set_direct(true); conf.mutable_cache()->set_clean_period(1); @@ -3060,7 +3060,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir1); + extension->set_current_dir(temp_dir1.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -3090,7 +3090,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { SharedPtr test_connection = std::static_pointer_cast(connection2); - const auto path = Immutable(String(temp_dir2)); + const auto path = Immutable(temp_dir2.str()); const auto input_path = path + "/test.cc"_l; const auto header_path = path + "/header.h"_l; const auto sanitize_blacklist_path = path + "/asan-blacklist.txt"_l; @@ -3102,7 +3102,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir2); + extension->set_current_dir(temp_dir2.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -3135,13 +3135,13 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { EXPECT_EQ(1u, metric.value()); Immutable cache_output; - const auto output2_path = temp_dir2.GetPath() / output_path.c_str(); + const auto output2_path = Path(temp_dir2) / output_path.c_str(); EXPECT_TRUE(base::File::Exists(output2_path)); EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); EXPECT_EQ(object_code, cache_output); Immutable cache_deps; - const auto deps2_path = temp_dir2.GetPath() / deps_path.c_str(); + const auto deps2_path = Path(temp_dir2) / deps_path.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path)); EXPECT_TRUE(base::File::Read(deps2_path, &cache_deps)); EXPECT_EQ(deps_contents, cache_deps); @@ -3166,7 +3166,7 @@ TEST_F(EmitterTest, HitDirectCacheFromTwoLocations) { TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { // Prepare environment. const base::TemporaryDir temp_dir; - const auto path = Immutable(String(temp_dir)); + const auto path = Immutable(temp_dir.str()); const auto relpath = path + "/path1"_l; ASSERT_TRUE(base::CreateDirectory(Path(relpath.string_copy()))); @@ -3194,7 +3194,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { const String plugin_name = "fake_plugin"; const auto plugin_path = "fake_plugin_path"_l; - conf.mutable_cache()->set_path(temp_dir); + conf.mutable_cache()->set_path(temp_dir.str()); conf.mutable_cache()->set_direct(true); conf.mutable_cache()->set_clean_period(1); @@ -3280,7 +3280,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path); extension->mutable_flags()->set_output(output_path); @@ -3308,7 +3308,7 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { net::Connection::ScopedMessage message(new net::Connection::Message); auto* extension = message->MutableExtension(base::proto::Local::extension); - extension->set_current_dir(temp_dir); + extension->set_current_dir(temp_dir.str()); extension->mutable_flags()->set_input(input_path2); extension->mutable_flags()->set_output(output_path2); @@ -3332,25 +3332,25 @@ TEST_F(EmitterTest, DontHitDirectCacheFromTwoRelativeSources) { emitter.reset(); Immutable cache_output; - const auto output2_path = temp_dir.GetPath() / output_path.c_str(); + const auto output2_path = Path(temp_dir) / output_path.c_str(); EXPECT_TRUE(base::File::Exists(output2_path)); EXPECT_TRUE(base::File::Read(output2_path, &cache_output)); EXPECT_EQ(object_code, cache_output); Immutable cache_deps; - const auto deps2_path = temp_dir.GetPath() / deps_path.c_str(); + const auto deps2_path = Path(temp_dir) / deps_path.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path)); EXPECT_TRUE(base::File::Read(deps2_path, &cache_deps)); EXPECT_EQ(deps_contents, cache_deps); Immutable cache_output2; - const auto output2_path2 = temp_dir.GetPath() / output_path2.c_str(); + const auto output2_path2 = Path(temp_dir) / output_path2.c_str(); EXPECT_TRUE(base::File::Exists(output2_path2)); EXPECT_TRUE(base::File::Read(output2_path2, &cache_output2)); EXPECT_EQ(object_code2, cache_output2); Immutable cache_deps2; - const auto deps2_path2 = temp_dir.GetPath() / deps_path2.c_str(); + const auto deps2_path2 = Path(temp_dir) / deps_path2.c_str(); EXPECT_TRUE(base::File::Exists(deps2_path2)); EXPECT_TRUE(base::File::Read(deps2_path2, &cache_deps2)); EXPECT_EQ(deps_contents2, cache_deps2); diff --git a/src/net/connection_linux_test.cc b/src/net/connection_linux_test.cc index 4b87cbf5..781e7f6c 100644 --- a/src/net/connection_linux_test.cc +++ b/src/net/connection_linux_test.cc @@ -60,10 +60,10 @@ int TestMessage::number_ = 1; class TestServer : public EventLoop { public: bool Init() { - if (tmp_dir_.GetPath().empty()) { + if (Path(tmp_dir_).empty()) { return false; } - socket_path_ = tmp_dir_.GetPath() / "socket"; + socket_path_ = Path(tmp_dir_) / "socket"; sockaddr_un address; address.sun_family = AF_UNIX;