]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
refactor: Use fs::create_hard_link instead of custom implementation
authorJoel Rosdahl <joel@rosdahl.net>
Sun, 9 Jul 2023 19:53:54 +0000 (21:53 +0200)
committerJoel Rosdahl <joel@rosdahl.net>
Wed, 12 Jul 2023 09:07:57 +0000 (11:07 +0200)
src/Util.cpp
src/Util.hpp
src/storage/local/LocalStorage.cpp
unittest/test_Util.cpp

index ee0890fdc1502d29541132ae8dcc200ed199181c..61992478f0f1df3ff5fcc41e328c98bf606f32cd 100644 (file)
@@ -491,25 +491,6 @@ get_umask()
   return g_umask;
 }
 
-void
-hard_link(const std::string& oldpath, const std::string& newpath)
-{
-  // Assumption: newpath may already exist as a left-over file from a previous
-  // run, but it's only we who can create the file entry now so we don't try to
-  // handle a race between unlink() and link() below.
-  unlink(newpath.c_str());
-
-#ifndef _WIN32
-  if (link(oldpath.c_str(), newpath.c_str()) != 0) {
-    throw core::Error(strerror(errno));
-  }
-#else
-  if (!CreateHardLink(newpath.c_str(), oldpath.c_str(), nullptr)) {
-    throw core::Error(Win32Util::error_message(GetLastError()));
-  }
-#endif
-}
-
 std::optional<size_t>
 is_absolute_path_with_prefix(std::string_view path)
 {
index bafd185a849b3a8c458cb48fb2be36d1b50a59d8..7026cfccb4ad1b64c3bce543d79b5c523085410f 100644 (file)
@@ -107,9 +107,6 @@ std::string get_relative_path(std::string_view dir, std::string_view path);
 // Get process umask.
 mode_t get_umask();
 
-// Hard-link `oldpath` to `newpath`. Throws `core::Error` on error.
-void hard_link(const std::string& oldpath, const std::string& newpath);
-
 // Determine if `path` is an absolute path with prefix, returning the split
 // point.
 std::optional<size_t> is_absolute_path_with_prefix(std::string_view path);
index bfb20ae3a7d1177ead826c6546d4a01dd65a0bd3..1d073f7fb64d103e2d7120a092ad2b75f3387fb7 100644 (file)
 #include <algorithm>
 #include <atomic>
 #include <cstdlib>
+#include <filesystem>
 #include <memory>
 #include <numeric>
 #include <string>
 #include <utility>
 
+namespace fs = std::filesystem;
+
 using core::Statistic;
 using core::StatisticsCounters;
 
@@ -638,19 +641,24 @@ LocalStorage::clone_hard_link_or_copy_file(const std::string& source,
 #endif
   }
   if (m_config.hard_link()) {
+    // Assumption: dest may already exist as a left-over file from a previous
+    // run, but it's only we who can create the file entry now so we don't try
+    // to handle a race between remove() and create_hard_link() below.
+
+    std::error_code ec;
+    fs::remove(dest, ec); // Ignore any error.
     LOG("Hard linking {} to {}", source, dest);
-    try {
-      Util::hard_link(source, dest);
+    fs::create_hard_link(source, dest, ec);
+    if (!ec) {
 #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("Failed to hard link {} to {}: {}", source, dest, ec.message());
+    // Fall back to copying.
   }
 
   LOG("Copying {} to {}", source, dest);
index 5b583254113350f45f66ced9faea9dd0b1584716..321b69ed31613962965b4ea5d68f7ee10c3668c2 100644 (file)
@@ -231,31 +231,6 @@ TEST_CASE("Util::get_relative_path")
 #endif
 }
 
-TEST_CASE("Util::hard_link")
-{
-  TestContext test_context;
-
-  SUBCASE("Link file to nonexistent destination")
-  {
-    util::write_file("old", "content");
-    CHECK_NOTHROW(Util::hard_link("old", "new"));
-    CHECK(*util::read_file<std::string>("new") == "content");
-  }
-
-  SUBCASE("Link file to existing destination")
-  {
-    util::write_file("old", "content");
-    util::write_file("new", "other content");
-    CHECK_NOTHROW(Util::hard_link("old", "new"));
-    CHECK(*util::read_file<std::string>("new") == "content");
-  }
-
-  SUBCASE("Link nonexistent file")
-  {
-    CHECK_THROWS_AS(Util::hard_link("old", "new"), core::Error);
-  }
-}
-
 TEST_CASE("Util::is_absolute_path_with_prefix")
 {
   CHECK(*Util::is_absolute_path_with_prefix("-I/c/foo") == 2);