From: Joel Rosdahl Date: Wed, 19 Jul 2023 06:43:38 +0000 (+0200) Subject: refactor: Move util::rename to util::filesystem X-Git-Tag: v4.9~99 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6bb3dc3aebc5619dba2090a93f04ff3b931d5c6f;p=thirdparty%2Fccache.git refactor: Move util::rename to util::filesystem --- diff --git a/src/AtomicFile.cpp b/src/AtomicFile.cpp index d68fbd62a..a3a877a83 100644 --- a/src/AtomicFile.cpp +++ b/src/AtomicFile.cpp @@ -24,6 +24,9 @@ #include #include #include +#include + +namespace fs = util::filesystem; AtomicFile::AtomicFile(const std::string& path, Mode mode) : m_path(path) { @@ -79,7 +82,7 @@ AtomicFile::commit() throw core::Error( FMT("failed to write data to {}: {}", m_path, strerror(errno))); } - const auto result = util::rename(m_tmp_path, m_path); + const auto result = fs::rename(m_tmp_path, m_path); if (!result) { throw core::Error(FMT("failed to rename {} to {}: {}", m_tmp_path, diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp index 092b558e2..883b093f9 100644 --- a/src/storage/local/LocalStorage.cpp +++ b/src/storage/local/LocalStorage.cpp @@ -270,7 +270,7 @@ clone_file(const std::string& src, const std::string& dest, bool via_tmp_file) src_fd.close(); if (via_tmp_file) { - const auto result = util::rename(tmp_file, dest); + const auto result = fs::rename(tmp_file, dest); if (!result) { throw core::Error(FMT("failed to rename {} to {}: {}", tmp_file, @@ -1048,9 +1048,9 @@ LocalStorage::move_to_wanted_cache_level(const StatisticsCounters& counters, // Note: Two ccache processes may move the file at the same time, so failure // to rename is OK. LOG("Moving {} to {}", cache_file_path, wanted_path); - util::rename(cache_file_path, wanted_path); + fs::rename(cache_file_path, wanted_path); for (const auto& raw_file : m_added_raw_files) { - util::rename( + fs::rename( raw_file, FMT("{}/{}", Util::dir_name(wanted_path), Util::base_name(raw_file))); } diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 0bac8faff..38e0b290f 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -9,6 +9,7 @@ set( UmaskScope.cpp environment.cpp file.cpp + filesystem.cpp path.cpp process.cpp string.cpp diff --git a/src/util/file.cpp b/src/util/file.cpp index 020499366..888b5616d 100644 --- a/src/util/file.cpp +++ b/src/util/file.cpp @@ -99,7 +99,7 @@ copy_file(const std::string& src, src_fd.close(); if (via_tmp_file == ViaTmpFile::yes) { - const auto result = util::rename(tmp_file, dest); + const auto result = fs::rename(tmp_file, dest); if (!result) { return nonstd::make_unexpected(FMT("Failed to rename {} to {}: {}", tmp_file, @@ -397,7 +397,7 @@ remove_nfs_safe(const std::string& path, LogFailure log_failure) std::string tmp_name = FMT("{}.ccache{}remove", path, TemporaryFile::tmp_file_infix); - auto rename_result = util::rename(path, tmp_name); + auto rename_result = fs::rename(path, tmp_name); if (!rename_result) { // It's OK if it was removed in a race. if (rename_result.error().value() != ENOENT @@ -422,29 +422,6 @@ remove_nfs_safe(const std::string& path, LogFailure log_failure) return remove_result; } -nonstd::expected -rename(const std::string& oldpath, const std::string& newpath) -{ -#ifndef _WIN32 - std::error_code ec; - std::filesystem::rename(oldpath, newpath, ec); - if (ec) { - return nonstd::make_unexpected(ec); - } -#else - // Windows' rename() won't overwrite an existing file, so need to use - // MoveFileEx instead. - if (!MoveFileExA( - oldpath.c_str(), newpath.c_str(), MOVEFILE_REPLACE_EXISTING)) { - DWORD error = GetLastError(); - // TODO: How should the Win32 error be mapped to std::error_code? - return nonstd::make_unexpected( - std::error_code(error, std::system_category())); - } -#endif - return {}; -} - void set_timestamps(const std::string& path, std::optional mtime, diff --git a/src/util/file.hpp b/src/util/file.hpp index e1174c656..905e3c39a 100644 --- a/src/util/file.hpp +++ b/src/util/file.hpp @@ -101,14 +101,6 @@ nonstd::expected remove_nfs_safe(const std::string& path, LogFailure log_failure = LogFailure::yes); -// Rename `oldpath` to `newpath` (deleting `newpath`). -// -// Note: Mingw-w64's std::filesystem::rename is buggy and doesn't properly -// overwrite an existing file, at least in version 9.1.0, hence this utility -// function. -nonstd::expected rename(const std::string& oldpath, - const std::string& newpath); - // Set the FD_CLOEXEC on file descriptor `fd`. This is a NOP on Windows. void set_cloexec_flag(int fd); diff --git a/src/util/filesystem.cpp b/src/util/filesystem.cpp new file mode 100644 index 000000000..3b20ead36 --- /dev/null +++ b/src/util/filesystem.cpp @@ -0,0 +1,49 @@ +// Copyright (C) 2023 Joel Rosdahl and other contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#include "filesystem.hpp" + +#include + +namespace util::filesystem { + +nonstd::expected +rename(const std::filesystem::path& old_p, const std::filesystem::path& new_p) +{ +#ifndef _WIN32 + std::error_code ec; + std::filesystem::rename(old_p, new_p, ec); + if (ec) { + return nonstd::make_unexpected(ec); + } +#else + // Windows' rename() won't overwrite an existing file, so need to use + // MoveFileEx instead. + if (!MoveFileExA(old_p.string().c_str(), + new_p.string().c_str(), + MOVEFILE_REPLACE_EXISTING)) { + DWORD error = GetLastError(); + // TODO: How should the Win32 error be mapped to std::error_code? + return nonstd::make_unexpected( + std::error_code(error, std::system_category())); + } +#endif + return {}; +} + +} // namespace util::filesystem diff --git a/src/util/filesystem.hpp b/src/util/filesystem.hpp index 01525968f..c992d132d 100644 --- a/src/util/filesystem.hpp +++ b/src/util/filesystem.hpp @@ -111,7 +111,6 @@ DEF_WRAP_1_P(is_directory, bool, const path&, p) DEF_WRAP_1_R(read_symlink, path, const path&, p) DEF_WRAP_1_R(remove, bool, const path&, p) DEF_WRAP_1_R(remove_all, std::uintmax_t, const path&, p) -// Note: Use util::rename instead of fs::rename. DEF_WRAP_0_R(temp_directory_path, path) // clang-format on @@ -122,4 +121,10 @@ DEF_WRAP_0_R(temp_directory_path, path) #undef DEF_WRAP_1_V #undef DEF_WRAP_2_V +// Note: Mingw-w64's std::filesystem::rename is buggy and doesn't properly +// overwrite an existing file, at least in version 9.1.0, hence this custom +// wrapper. +nonstd::expected rename(const path& old_p, + const path& new_p); + } // namespace util::filesystem