From: Joel Rosdahl <joel@rosdahl.net>
Date: Sun, 9 Jul 2023 16:36:31 +0000 (+0200)
Subject: refactor: Move clone_hard_link_or_copy_file to local storage class
X-Git-Tag: v4.9~141
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ee97bd69a177b19501ed10ff67f38fa7f1b06ab3;p=thirdparty%2Fccache.git

refactor: Move clone_hard_link_or_copy_file to local storage class
---

diff --git a/src/Util.cpp b/src/Util.cpp
index 8eebcc0ca..8c253de23 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -51,28 +51,6 @@
 #  include <pwd.h>
 #endif
 
-#ifdef __linux__
-#  ifdef HAVE_SYS_IOCTL_H
-#    include <sys/ioctl.h>
-#  endif
-#  ifdef HAVE_LINUX_FS_H
-#    include <linux/fs.h>
-#    ifndef FICLONE
-#      define FICLONE _IOW(0x94, 9, int)
-#    endif
-#    define FILE_CLONING_SUPPORTED 1
-#  endif
-#endif
-
-#ifdef __APPLE__
-#  ifdef HAVE_SYS_CLONEFILE_H
-#    include <sys/clonefile.h>
-#    ifdef CLONE_NOOWNERCOPY
-#      define FILE_CLONING_SUPPORTED 1
-#    endif
-#  endif
-#endif
-
 using IncludeDelimiter = util::Tokenizer::IncludeDelimiter;
 
 namespace {
@@ -215,93 +193,6 @@ change_extension(std::string_view path, std::string_view new_ext)
   return std::string(without_ext).append(new_ext.data(), new_ext.length());
 }
 
-#ifdef FILE_CLONING_SUPPORTED
-void
-clone_file(const std::string& src, const std::string& dest, bool via_tmp_file)
-{
-#  if defined(__linux__)
-  Fd src_fd(open(src.c_str(), O_RDONLY));
-  if (!src_fd) {
-    throw core::Error(FMT("{}: {}", src, strerror(errno)));
-  }
-
-  Fd dest_fd;
-  std::string tmp_file;
-  if (via_tmp_file) {
-    TemporaryFile temp_file(dest);
-    dest_fd = std::move(temp_file.fd);
-    tmp_file = temp_file.path;
-  } else {
-    dest_fd =
-      Fd(open(dest.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666));
-    if (!dest_fd) {
-      throw core::Error(FMT("{}: {}", src, strerror(errno)));
-    }
-  }
-
-  if (ioctl(*dest_fd, FICLONE, *src_fd) != 0) {
-    throw core::Error(strerror(errno));
-  }
-
-  dest_fd.close();
-  src_fd.close();
-
-  if (via_tmp_file) {
-    Util::rename(tmp_file, dest);
-  }
-#  elif defined(__APPLE__)
-  (void)via_tmp_file;
-  if (clonefile(src.c_str(), dest.c_str(), CLONE_NOOWNERCOPY) != 0) {
-    throw core::Error(strerror(errno));
-  }
-#  else
-  (void)src;
-  (void)dest;
-  (void)via_tmp_file;
-  throw core::Error(strerror(EOPNOTSUPP));
-#  endif
-}
-#endif // FILE_CLONING_SUPPORTED
-
-void
-clone_hard_link_or_copy_file(const Config& config,
-                             const std::string& source,
-                             const std::string& dest,
-                             bool via_tmp_file)
-{
-  if (config.file_clone()) {
-#ifdef FILE_CLONING_SUPPORTED
-    LOG("Cloning {} to {}", source, dest);
-    try {
-      clone_file(source, dest, via_tmp_file);
-      return;
-    } catch (core::Error& e) {
-      LOG("Failed to clone: {}", e.what());
-    }
-#else
-    LOG("Not cloning {} to {} since it's unsupported", source, dest);
-#endif
-  }
-  if (config.hard_link()) {
-    LOG("Hard linking {} to {}", source, dest);
-    try {
-      Util::hard_link(source, dest);
-#ifndef _WIN32
-      if (chmod(dest.c_str(), 0444 & ~Util::get_umask()) != 0) {
-        LOG("Failed to chmod {}: {}", dest.c_str(), strerror(errno));
-      }
-#endif
-      return;
-    } catch (const core::Error& e) {
-      LOG("Failed to hard link {} to {}: {}", source, dest, e.what());
-      // Fall back to copying.
-    }
-  }
-
-  LOG("Copying {} to {}", source, dest);
-  copy_file(source, dest, via_tmp_file);
-}
-
 size_t
 common_dir_prefix_length(std::string_view dir, std::string_view path)
 {
diff --git a/src/Util.hpp b/src/Util.hpp
index 0be214c82..d157808d1 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -49,21 +49,6 @@ std::string_view base_name(std::string_view path);
 // should start with a dot, no extra dot is inserted.
 std::string change_extension(std::string_view path, std::string_view new_ext);
 
-// Clone a file from `src` to `dest`. If `via_tmp_file` is true, `src` is cloned
-// to a temporary file and then renamed to `dest`. Throws `core::Error` on
-// error.
-void clone_file(const std::string& src,
-                const std::string& dest,
-                bool via_tmp_file = false);
-
-// Clone, hard link or copy a file from `source` to `dest` depending on settings
-// in `ctx`. If cloning or hard linking cannot and should not be done the file
-// will be copied instead. Throws `core::Error` on error.
-void clone_hard_link_or_copy_file(const Config& config,
-                                  const std::string& source,
-                                  const std::string& dest,
-                                  bool via_tmp_file = false);
-
 // Compute the length of the longest directory path that is common to paths
 // `dir` (a directory) and `path` (any path).
 size_t common_dir_prefix_length(std::string_view dir, std::string_view path);
diff --git a/src/core/ResultRetriever.cpp b/src/core/ResultRetriever.cpp
index 8b2e5c625..eefb62264 100644
--- a/src/core/ResultRetriever.cpp
+++ b/src/core/ResultRetriever.cpp
@@ -115,8 +115,8 @@ ResultRetriever::on_raw_file(uint8_t file_number,
   const auto dest_path = get_dest_path(file_type);
   if (!dest_path.empty()) {
     try {
-      Util::clone_hard_link_or_copy_file(
-        m_ctx.config, raw_file_path, dest_path, false);
+      m_ctx.storage.local.clone_hard_link_or_copy_file(
+        raw_file_path, dest_path, false);
     } catch (core::Error& e) {
       throw WriteError(FMT("Failed to clone/link/copy {} to {}: {}",
                            raw_file_path,
diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index 0ffd74ae0..ebc13ed43 100644
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -47,6 +47,33 @@
 
 #include <third_party/fmt/core.h>
 
+#include <fcntl.h>
+#ifdef HAVE_UNISTD_H
+#  include <unistd.h>
+#endif
+
+#ifdef __linux__
+#  ifdef HAVE_SYS_IOCTL_H
+#    include <sys/ioctl.h>
+#  endif
+#  ifdef HAVE_LINUX_FS_H
+#    include <linux/fs.h>
+#    ifndef FICLONE
+#      define FICLONE _IOW(0x94, 9, int)
+#    endif
+#    define FILE_CLONING_SUPPORTED 1
+#  endif
+#endif
+
+#ifdef __APPLE__
+#  ifdef HAVE_SYS_CLONEFILE_H
+#    include <sys/clonefile.h>
+#    ifdef CLONE_NOOWNERCOPY
+#      define FILE_CLONING_SUPPORTED 1
+#    endif
+#  endif
+#endif
+
 #include <algorithm>
 #include <atomic>
 #include <cstdlib>
@@ -55,10 +82,6 @@
 #include <string>
 #include <utility>
 
-#ifdef HAVE_UNISTD_H
-#  include <unistd.h>
-#endif
-
 using core::Statistic;
 using core::StatisticsCounters;
 
@@ -196,6 +219,59 @@ delete_file(const std::string& path,
   }
 }
 
+#ifdef FILE_CLONING_SUPPORTED
+
+// Clone a file from `src` to `dest`. If `via_tmp_file` is true, `src` is cloned
+// to a temporary file and then renamed to `dest`. Throws `core::Error` on
+// error.
+static void
+clone_file(const std::string& src, const std::string& dest, bool via_tmp_file)
+{
+#  if defined(__linux__)
+  Fd src_fd(open(src.c_str(), O_RDONLY));
+  if (!src_fd) {
+    throw core::Error(FMT("{}: {}", src, strerror(errno)));
+  }
+
+  Fd dest_fd;
+  std::string tmp_file;
+  if (via_tmp_file) {
+    TemporaryFile temp_file(dest);
+    dest_fd = std::move(temp_file.fd);
+    tmp_file = temp_file.path;
+  } else {
+    dest_fd =
+      Fd(open(dest.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666));
+    if (!dest_fd) {
+      throw core::Error(FMT("{}: {}", src, strerror(errno)));
+    }
+  }
+
+  if (ioctl(*dest_fd, FICLONE, *src_fd) != 0) {
+    throw core::Error(strerror(errno));
+  }
+
+  dest_fd.close();
+  src_fd.close();
+
+  if (via_tmp_file) {
+    Util::rename(tmp_file, dest);
+  }
+#  elif defined(__APPLE__)
+  (void)via_tmp_file;
+  if (clonefile(src.c_str(), dest.c_str(), CLONE_NOOWNERCOPY) != 0) {
+    throw core::Error(strerror(errno));
+  }
+#  else
+  (void)src;
+  (void)dest;
+  (void)via_tmp_file;
+  throw core::Error(strerror(EOPNOTSUPP));
+#  endif
+}
+
+#endif // FILE_CLONING_SUPPORTED
+
 struct CleanDirResult
 {
   Level2Counters before;
@@ -526,8 +602,7 @@ LocalStorage::put_raw_files(
     const auto dest_path = get_raw_file_path(cache_file.path, file_number);
     const auto old_stat = Stat::stat(dest_path);
     try {
-      Util::clone_hard_link_or_copy_file(
-        m_config, source_path, dest_path, true);
+      clone_hard_link_or_copy_file(source_path, dest_path, true);
       m_added_raw_files.push_back(dest_path);
     } catch (core::Error& e) {
       LOG("Failed to store {} as raw file {}: {}",
@@ -544,6 +619,44 @@ LocalStorage::put_raw_files(
   }
 }
 
+void
+LocalStorage::clone_hard_link_or_copy_file(const std::string& source,
+                                           const std::string& dest,
+                                           bool via_tmp_file) const
+{
+  if (m_config.file_clone()) {
+#ifdef FILE_CLONING_SUPPORTED
+    LOG("Cloning {} to {}", source, dest);
+    try {
+      clone_file(source, dest, via_tmp_file);
+      return;
+    } catch (core::Error& e) {
+      LOG("Failed to clone: {}", e.what());
+    }
+#else
+    LOG("Not cloning {} to {} since it's unsupported", source, dest);
+#endif
+  }
+  if (m_config.hard_link()) {
+    LOG("Hard linking {} to {}", source, dest);
+    try {
+      Util::hard_link(source, dest);
+#ifndef _WIN32
+      if (chmod(dest.c_str(), 0444 & ~Util::get_umask()) != 0) {
+        LOG("Failed to chmod {}: {}", dest.c_str(), strerror(errno));
+      }
+#endif
+      return;
+    } catch (const core::Error& e) {
+      LOG("Failed to hard link {} to {}: {}", source, dest, e.what());
+      // Fall back to copying.
+    }
+  }
+
+  LOG("Copying {} to {}", source, dest);
+  Util::copy_file(source, dest, via_tmp_file);
+}
+
 void
 LocalStorage::increment_statistic(const Statistic statistic,
                                   const int64_t value)
diff --git a/src/storage/local/LocalStorage.hpp b/src/storage/local/LocalStorage.hpp
index 1a49aa6d1..8b529b916 100644
--- a/src/storage/local/LocalStorage.hpp
+++ b/src/storage/local/LocalStorage.hpp
@@ -84,6 +84,13 @@ public:
   put_raw_files(const Hash::Digest& key,
                 const std::vector<core::Result::Serializer::RawFile> raw_files);
 
+  // Clone, hard link or copy a file from `source` to `dest` depending on
+  // settings in `ctx`. If cloning or hard linking cannot and should not be done
+  // the file will be copied instead. Throws `core::Error` on error.
+  void clone_hard_link_or_copy_file(const std::string& source,
+                                    const std::string& dest,
+                                    bool via_tmp_file = false) const;
+
   // --- Statistics ---
 
   void increment_statistic(core::Statistic statistic, int64_t value = 1);