From d19d7601ebe813ebb27a63abc1c994f05b381810 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Wed, 29 Oct 2025 01:23:33 +0100 Subject: [PATCH] Revert "Add function to copy a directory tree" This reverts commit 1afa2fad145841c98fe710d33bfac2e613d14496. --- elisp/private/tools/system.cc | 267 ---------------------------------- elisp/private/tools/system.h | 1 - tests/tools/system_test.cc | 113 -------------- 3 files changed, 381 deletions(-) diff --git a/elisp/private/tools/system.cc b/elisp/private/tools/system.cc index b26b7a82..207cffd7 100644 --- a/elisp/private/tools/system.cc +++ b/elisp/private/tools/system.cc @@ -837,273 +837,6 @@ absl::Status CopyFile(const FileName& from, const FileName& to) { return absl::OkStatus(); } -#if !defined _WIN32 && !defined __APPLE__ -# define RULES_ELISP_COPY_TREE_POSIX -#endif - -#ifdef _WIN32 -static absl::Status CopyTreeWindows(const FileName& from, const FileName& to) { - if (!::CreateDirectoryExW(from.pointer(), to.pointer(), nullptr)) { - return WindowsStatus("CreateDirectoryExW", from, to, nullptr); - } - const std::wstring pattern = from.string() + L"\\*"; - WIN32_FIND_DATAW data; - const HANDLE handle = ::FindFirstFileW(Pointer(pattern), &data); - if (handle == INVALID_HANDLE_VALUE) { - return ::GetLastError() == ERROR_FILE_NOT_FOUND - ? absl::OkStatus() - : WindowsStatus("FindFirstFileW", pattern); - } - const absl::Cleanup cleanup = [handle] { - if (!::FindClose(handle)) LOG(ERROR) << WindowsStatus("FindClose"); - }; - do { - const std::wstring_view name = data.cFileName; - if (name == L"." || name == L"..") continue; - const absl::StatusOr from_entry = from.Child(name); - if (!from_entry.ok()) return from_entry.status(); - const absl::StatusOr to_entry = to.Child(name); - if (!to_entry.ok()) return to_entry.status(); - if (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { - const absl::Status status = CopyTreeWindows(*from_entry, *to_entry); - if (!status.ok()) return status; - } else { - constexpr DWORD flags = COPY_FILE_FAIL_IF_EXISTS | COPY_FILE_COPY_SYMLINK; - if (!::CopyFileExW(from_entry->pointer(), to_entry->pointer(), nullptr, - nullptr, nullptr, flags)) { - return WindowsStatus("CopyFileExW", *from_entry, *to_entry, nullptr, - nullptr, nullptr, absl::Hex(flags)); - } - } - } while (::FindNextFileW(handle, &data)); - if (::GetLastError() != ERROR_NO_MORE_FILES) { - return WindowsStatus("FindNextFileW"); - } - return absl::OkStatus(); -} -#endif - -#ifdef RULES_ELISP_COPY_TREE_POSIX -static absl::Status CopyFilePosix(const int from_parent, const int to_parent, - const FileName& name, - const struct stat& from_stat) { - CHECK_NE(name.string(), "."); - CHECK_NE(name.string(), ".."); - CHECK_EQ(name.string().find('/'), name.string().npos) - << "invalid name " << name; - CHECK(S_ISREG(from_stat.st_mode)) << "file " << name << " is not regular"; - - constexpr int from_flags = O_RDONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW; - const int from_fd = openat(from_parent, name.pointer(), from_flags); - if (from_fd < 0) { - return ErrnoStatus("openat", from_parent, name, absl::Hex(from_flags)); - } - const absl::Cleanup close_from = [from_fd] { - if (close(from_fd) != 0) LOG(ERROR) << ErrnoStatus("close", from_fd); - }; - - const mode_t mode = from_stat.st_mode; - constexpr int to_flags = - O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW; - const int to_fd = openat(to_parent, name.pointer(), to_flags, mode); - if (to_fd < 0) { - return ErrnoStatus("openat", to_parent, name, to_flags, Oct(mode)); - } - const absl::Cleanup close_to = [to_fd] { - if (close(to_fd) != 0) LOG(ERROR) << ErrnoStatus("close", to_fd); - }; - - while (true) { - constexpr std::size_t buffer_size = 0x1000; - char buffer[buffer_size]; - const ssize_t result = read(from_fd, buffer, buffer_size); - if (result < 0) { - return ErrnoStatus("read", from_fd, kEllipsis, absl::Hex(buffer_size)); - } - if (result == 0) break; - std::string_view remaining( - buffer, CastNumber(result).value()); - while (!remaining.empty()) { - const ssize_t written = write(to_fd, remaining.data(), remaining.size()); - if (written < 0) { - return ErrnoStatus("write", to_fd, kEllipsis, remaining.size()); - } - if (written == 0) { - // Prevent infinite loop. - return absl::DataLossError(absl::StrFormat( - "Cannot write %d bytes to %v", remaining.size(), name)); - } - remaining.remove_prefix( - CastNumber(written).value()); - } - } - - if (fsync(to_fd) != 0) return ErrnoStatus("fsync", to_fd); - - const struct timespec times[2] = {{from_stat.st_atime, 0}, - {from_stat.st_mtime, 0}}; - if (futimens(to_fd, times) != 0) return ErrnoStatus("futimens", to_fd); - - return absl::OkStatus(); -} - -static absl::Status CopyTreePosix(const int from_parent, - const FileName& from_name, - std::optional from_stat, - const int to_parent, - const FileName& to_name) { - CHECK_NE(from_name.string(), "."); - CHECK_NE(from_name.string(), ".."); - CHECK(from_parent == AT_FDCWD || - from_name.string().find('/') == from_name.string().npos) - << "invalid source name " << from_name; - - CHECK_NE(to_name.string(), "."); - CHECK_NE(to_name.string(), ".."); - CHECK(to_parent == AT_FDCWD || - to_name.string().find('/') == to_name.string().npos) - << "invalid target name " << to_name; - - const int from_flags = - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC | O_NOCTTY; - const int from_fd = openat(from_parent, from_name.pointer(), from_flags); - if (from_fd < 0) { - return ErrnoStatus("openat", from_parent, from_name, absl::Hex(from_flags)); - } - bool owns_from_fd = true; - const absl::Cleanup close_from_fd = [from_fd, &owns_from_fd] { - if (owns_from_fd) { - if (close(from_fd) != 0) LOG(ERROR) << ErrnoStatus("close", from_fd); - } - }; - if (!from_stat.has_value()) { - struct stat buffer; - if (fstat(from_fd, &buffer) != 0) return ErrnoStatus("fstat", from_fd); - if (!S_ISDIR(buffer.st_mode)) { - return absl::FailedPreconditionError( - absl::StrFormat("Source %v is not a directory", from_name)); - } - from_stat = buffer; - } - CHECK(from_stat.has_value()); - - DIR* const absl_nullable dir = fdopendir(from_fd); - if (dir == nullptr) return ErrnoStatus("fdopendir", from_fd); - owns_from_fd = false; - const absl::Cleanup close_dir = [dir, &owns_from_fd] { - CHECK(!owns_from_fd); - if (closedir(dir) != 0) LOG(ERROR) << ErrnoStatus("closedir"); - }; - - const mode_t mode = from_stat->st_mode | S_IWUSR; - const int result = mkdirat(to_parent, to_name.pointer(), mode); - if (result != 0) return ErrnoStatus("mkdirat", to_parent, to_name, Oct(mode)); - - constexpr int to_flags = - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC | O_NOCTTY; - const int to_fd = openat(to_parent, to_name.pointer(), to_flags); - if (to_fd < 0) { - return ErrnoStatus("openat", to_parent, to_name, absl::Hex(to_flags)); - } - const absl::Cleanup close_to = [to_fd] { - if (close(to_fd) != 0) LOG(ERROR) << ErrnoStatus("close"); - }; - - while (true) { - errno = 0; - const struct dirent* const absl_nullable entry = readdir(dir); - if (entry == nullptr) { - if (errno != 0) return ErrnoStatus("readdir"); - break; - } - const std::string_view name = entry->d_name; - if (name == "." || name == "..") continue; - const absl::StatusOr file = FileName::FromString(name); - if (!file.ok()) return file.status(); - - struct stat buffer; - constexpr int stat_flags = AT_SYMLINK_NOFOLLOW; - if (fstatat(from_fd, entry->d_name, &buffer, stat_flags) != 0) { - return ErrnoStatus("fstatat", from_fd, name, kEllipsis, - absl::Hex(stat_flags)); - } - const mode_t mode = buffer.st_mode; - if (S_ISDIR(mode)) { - const absl::Status status = - CopyTreePosix(from_fd, *file, buffer, to_fd, *file); - if (!status.ok()) return status; - } else if (S_ISREG(mode)) { - const absl::Status status = CopyFilePosix(from_fd, to_fd, *file, buffer); - if (!status.ok()) return status; - } else { - return absl::FailedPreconditionError(absl::StrFormat( - "File %s with mode 0%03o cannot be copied", name, mode)); - } - } - - const struct timespec times[2] = {{from_stat->st_atime, 0}, - {from_stat->st_mtime, 0}}; - if (futimens(to_fd, times) != 0) return ErrnoStatus("futimens", to_fd); - - return absl::OkStatus(); -} -#endif - -absl::Status CopyTree(const FileName& from, const FileName& to) { -#if defined _WIN32 - const absl::StatusOr from_abs = from.MakeAbsolute(); - if (!from_abs.ok()) return from_abs.status(); - if (from_abs->string().length() < 4) { - return absl::InvalidArgumentError( - absl::StrFormat("Cannot copy drive %v", *from_abs)); - } - const absl::StatusOr to_abs = to.MakeAbsolute(); - if (!to_abs.ok()) return to_abs.status(); - if (to_abs->string().length() < 4) { - return absl::AlreadyExistsError( - absl::StrFormat("Cannot copy over drive %v", *from_abs)); - } - return CopyTreeWindows(*from_abs, *to_abs); -#elif defined __APPLE__ - std::string from_str = from.string(); - // The behavior of copyfile changes if the source directory name ends in a - // slash, see the man page copyfile(3). We need to remove trailing slashes to - // get the expected behavior. - while (from_str.back() == '/') from_str.pop_back(); - if (from_str.empty()) { - // If the source directory name is empty, it has consisted only of slashes. - return absl::InvalidArgumentError("Cannot copy from root directory"); - } - // The behavior of copyfile changes if the destination directory already - // exists, see the man page copyfile(3). - struct stat buffer; - if (lstat(to.pointer(), &buffer) == 0) { - return absl::AlreadyExistsError( - absl::StrFormat("Destination directory %v already exists", to)); - } - if (errno != ENOENT) return ErrnoStatus("lstat", to); - constexpr copyfile_flags_t flags = COPYFILE_ALL | COPYFILE_RECURSIVE | - COPYFILE_EXCL | COPYFILE_NOFOLLOW | - COPYFILE_CLONE | COPYFILE_DATA_SPARSE; - if (copyfile(Pointer(from_str), to.pointer(), nullptr, flags) != 0) { - return ErrnoStatus("copyfile", from_str, to, nullptr, absl::Hex(flags)); - } - return absl::OkStatus(); -#elif defined RULES_ELISP_COPY_TREE_POSIX - if (from.string().find_first_not_of('/') == from.string().npos) { - return absl::InvalidArgumentError( - absl::StrFormat("Cannot copy from root directory %v", from)); - } - if (to.string().find_first_not_of('/') == to.string().npos) { - return absl::AlreadyExistsError( - absl::StrFormat("Cannot overwrite root directory %v", to)); - } - return CopyTreePosix(AT_FDCWD, from, std::nullopt, AT_FDCWD, to); -#else -# error CopyTree not implemented on this system -#endif -} - namespace { class EnvironmentBlock final { diff --git a/elisp/private/tools/system.h b/elisp/private/tools/system.h index 80fa0cf0..4505a624 100644 --- a/elisp/private/tools/system.h +++ b/elisp/private/tools/system.h @@ -243,7 +243,6 @@ absl::Status RemoveTree(const FileName& directory); absl::StatusOr> ListDirectory(const FileName& dir); absl::Status Rename(const FileName& from, const FileName& to); absl::Status CopyFile(const FileName& from, const FileName& to); -absl::Status CopyTree(const FileName& from, const FileName& to); class Environment final { private: diff --git a/tests/tools/system_test.cc b/tests/tools/system_test.cc index 59a912c9..bfd56a27 100644 --- a/tests/tools/system_test.cc +++ b/tests/tools/system_test.cc @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -725,118 +724,6 @@ TEST(CopyFileTest, CopiesFile) { EXPECT_THAT(ReadFile(to), IsOkAndHolds("contents")); } -TEST(CopyTreeTest, RejectsSelfCopy) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - EXPECT_THAT(CopyTree(*temp, *temp), - StatusIs(AnyOf(absl::StatusCode::kAlreadyExists, - absl::StatusCode::kNotFound))); -} - -TEST(CopyTreeTest, RejectsCopyFromRootDirectory) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - const FileName from = - FileName::FromString(kWindows ? RULES_ELISP_NATIVE_LITERAL("C:\\") - : RULES_ELISP_NATIVE_LITERAL("/")) - .value(); - const FileName to = temp->Child(RULES_ELISP_NATIVE_LITERAL("to")).value(); - - EXPECT_THAT(CopyTree(from, to), StatusIs(absl::StatusCode::kInvalidArgument)); - - EXPECT_THAT(RemoveDirectory(to), StatusIs(absl::StatusCode::kNotFound)); -} - -TEST(CopyTreeTest, RejectsCopyToRootDirectory) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - const FileName to = - FileName::FromString(kWindows ? RULES_ELISP_NATIVE_LITERAL("C:\\") - : RULES_ELISP_NATIVE_LITERAL("/")) - .value(); - - EXPECT_THAT(CopyTree(*temp, to), StatusIs(absl::StatusCode::kAlreadyExists)); -} - -TEST(CopyTreeTest, RejectsCopyToExistingDirectory) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - const FileName from_dir = - temp->Child(RULES_ELISP_NATIVE_LITERAL("from")).value(); - const FileName from_file = - from_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - const FileName to_dir = temp->Child(RULES_ELISP_NATIVE_LITERAL("to")).value(); - const FileName to_file = - from_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - - const absl::Cleanup cleanup = [&from_dir, &to_dir] { - EXPECT_THAT(RemoveTree(from_dir), IsOk()); - EXPECT_THAT(RemoveTree(to_dir), IsOk()); - }; - - EXPECT_THAT(CreateDirectory(from_dir), IsOk()); - EXPECT_THAT(CreateDirectory(to_dir), IsOk()); - EXPECT_THAT(WriteFile(from_file, "contents"), IsOk()); - - EXPECT_THAT(CopyTree(from_dir, to_dir), - StatusIs(absl::StatusCode::kAlreadyExists)); -} - -TEST(CopyTreeTest, CopiesToNewDirectory) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - const FileName from_dir = - temp->Child(RULES_ELISP_NATIVE_LITERAL("from")).value(); - const FileName from_file = - from_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - const FileName to_dir = temp->Child(RULES_ELISP_NATIVE_LITERAL("to")).value(); - const FileName to_file = - to_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - - const absl::Cleanup cleanup = [&from_dir, &to_dir] { - EXPECT_THAT(RemoveTree(from_dir), IsOk()); - EXPECT_THAT(RemoveTree(to_dir), IsOk()); - }; - - EXPECT_THAT(CreateDirectory(from_dir), IsOk()); - EXPECT_THAT(WriteFile(from_file, "contents"), IsOk()); - - EXPECT_THAT(CopyTree(from_dir, to_dir), IsOk()); - - EXPECT_THAT(ReadFile(to_file), IsOkAndHolds("contents")); -} - -TEST(CopyTreeTest, IgnoresTrailingSlash) { - const absl::StatusOr temp = TempDir(); - ASSERT_THAT(temp, IsOk()); - - const FileName from_dir = - temp->Join(NativeString(RULES_ELISP_NATIVE_LITERAL("from")) + kSeparator) - .value(); - const FileName from_file = - from_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - const FileName to_dir = temp->Child(RULES_ELISP_NATIVE_LITERAL("to")).value(); - const FileName to_file = - to_dir.Child(RULES_ELISP_NATIVE_LITERAL("file")).value(); - - const absl::Cleanup cleanup = [&from_dir, &to_dir] { - EXPECT_THAT(RemoveTree(from_dir), IsOk()); - EXPECT_THAT(RemoveTree(to_dir), IsOk()); - }; - - EXPECT_THAT(CreateDirectory(from_dir), IsOk()); - EXPECT_THAT(WriteFile(from_file, "contents"), IsOk()); - - EXPECT_THAT(CopyTree(from_dir, to_dir), IsOk()); - - EXPECT_THAT(ReadFile(to_file), IsOkAndHolds("contents")); -} - TEST(EnvironmentTest, CurrentReturnsValidEnv) { EXPECT_THAT(Environment::Current(), IsOkAndHolds(Contains(Pair(