refactor: Move util::rename to util::filesystem

diff --git a/src/AtomicFile.cpp b/src/AtomicFile.cpp
index d68fbd62ae38e3f3d48d80caebd10d06f9135c7d..a3a877a8341e64bece6438bcfdac01fbec0b6008 100644 (file)
--- a/src/AtomicFile.cpp
+++ b/src/AtomicFile.cpp
@@ -24,6 +24,9 @@
 #include <core/exceptions.hpp>
 #include <fmtmacros.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
+
+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 092b558e2561777725d919c639463577c27ccfcf..883b093f9ae2467cd79ae61c4820dc4d70dcd4c1 100644 (file)
--- 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 0bac8faff990158087f050d2ec55c199361043b2..38e0b290f9b8e88ca22a827c26680c84f61e461d 100644 (file)
--- 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 0204993667d61985d2b69768a31cf76ad9434f77..888b5616df0ed94f1f8e2ad8006a818c3ef21fb6 100644 (file)
--- 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<void, std::error_code>
-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<util::TimePoint> mtime,

diff --git a/src/util/file.hpp b/src/util/file.hpp
index e1174c6562ae5cfdaaf30e64842ce7d45f74e69e..905e3c39a25d781602f5695d90c2b478bd0c885b 100644 (file)
--- a/src/util/file.hpp
+++ b/src/util/file.hpp
@@ -101,14 +101,6 @@ nonstd::expected<bool, std::error_code>
 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<void, std::error_code> 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 (file)
index 0000000..3b20ead
--- /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 <core/wincompat.hpp>
+
+namespace util::filesystem {
+
+nonstd::expected<void, std::error_code>
+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 01525968fe7ef68eb915c4c43485f651dd079749..c992d132d75a7cb1018ca20b23467c63c95762ac 100644 (file)
--- 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<void, std::error_code> rename(const path& old_p,
+                                               const path& new_p);
+
 } // namespace util::filesystem