refactor: fs::path-ify util::traverse_directory

diff --git a/src/Context.cpp b/src/Context.cpp
index 9eab8a86978e4c110f51439f3124ac7daaafcd07..fbbfb2fde5e6e7ede367b2dd78f4763617d5fbff 100644 (file)
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -25,6 +25,7 @@
 
 #include <Win32Util.hpp>
 #include <util/TimePoint.hpp>
+#include <util/file.hpp>
 #include <util/path.hpp>
 #include <util/process.hpp>
 #include <util/string.hpp>

diff --git a/src/core/common.cpp b/src/core/common.cpp
index b7560976e1829089c8af897fc40a8cf842695c1d..72acaab61616bc2ed54d034f06eb02de2f5758cc 100644 (file)
--- a/src/core/common.cpp
+++ b/src/core/common.cpp
@@ -24,6 +24,7 @@
 #include <fmtmacros.hpp>
 #include <util/Tokenizer.hpp>
 #include <util/expected.hpp>
+#include <util/file.hpp>
 #include <util/filesystem.hpp>
 #include <util/path.hpp>
 

diff --git a/src/core/mainoptions.cpp b/src/core/mainoptions.cpp
index 02341b47f96e1f04de67e356a74e3d3815d697e0..f12c4f6ec58169d4faa14f3af7787ed1bb3bf9c5 100644 (file)
--- a/src/core/mainoptions.cpp
+++ b/src/core/mainoptions.cpp
@@ -302,23 +302,22 @@ trim_dir(const std::string& dir,
   std::vector<DirEntry> files;
   uint64_t initial_size = 0;
 
-  util::throw_on_error<core::Error>(util::traverse_directory(
-    dir, [&](const std::string& path, const bool is_dir) {
-      if (is_dir || util::TemporaryFile::is_tmp_file(path)) {
+  util::throw_on_error<core::Error>(
+    util::traverse_directory(dir, [&](const auto& de) {
+      if (de.is_directory() || util::TemporaryFile::is_tmp_file(de.path())) {
         return;
       }
-      DirEntry entry(path);
-      if (!entry) {
+      if (!de) {
         // Probably some race, ignore.
         return;
       }
-      initial_size += entry.size_on_disk();
-      const auto name = Util::base_name(path);
+      initial_size += de.size_on_disk();
+      const auto name = de.path().filename();
       if (name == "ccache.conf" || name == "stats") {
         throw Fatal(
-          FMT("this looks like a local cache directory (found {})", path));
+          FMT("this looks like a local cache directory (found {})", de.path()));
       }
-      files.emplace_back(std::move(entry));
+      files.emplace_back(de);
     }));
 
   std::sort(files.begin(), files.end(), [&](const auto& f1, const auto& f2) {

diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index fea093cdb83c20b7587ebd8b0232ff707b5f45b5..8d54f81e1b95309fba473af02dfa2f6ec24de223 100644 (file)
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -1421,20 +1421,16 @@ LocalStorage::clean_internal_tempdir()
 
   LOG("Cleaning up {}", m_config.temporary_dir());
   core::ensure_dir_exists(m_config.temporary_dir());
-  util::traverse_directory(
-    m_config.temporary_dir(),
-    [now](const std::string& path, bool is_dir) {
-      if (is_dir) {
-        return;
-      }
-      DirEntry dir_entry(path, DirEntry::LogOnError::yes);
-      if (dir_entry && dir_entry.mtime() + k_tempdir_cleanup_interval < now) {
-        util::remove(path);
-      }
-    })
-    .or_else([&](const auto& error) {
-      LOG("Failed to clean up {}: {}", m_config.temporary_dir(), error);
-    });
+  util::traverse_directory(m_config.temporary_dir(), [now](const auto& de) {
+    if (de.is_directory()) {
+      return;
+    }
+    if (de && de.mtime() + k_tempdir_cleanup_interval < now) {
+      util::remove(de.path());
+    }
+  }).or_else([&](const auto& error) {
+    LOG("Failed to clean up {}: {}", m_config.temporary_dir(), error);
+  });
 
   util::write_file(cleaned_stamp, "");
 }

diff --git a/src/storage/local/util.cpp b/src/storage/local/util.cpp
index 871f8361e822fe6c4cf9dcfeb84b69a433aa1169..9aacb94d6ab2873659e9c2ab38057649e982e865 100644 (file)
--- a/src/storage/local/util.cpp
+++ b/src/storage/local/util.cpp
@@ -73,15 +73,15 @@ get_cache_dir_files(const std::string& dir)
     return files;
   }
   util::throw_on_error<core::Error>(
-    util::traverse_directory(dir, [&](const std::string& path, bool is_dir) {
-      auto name = Util::base_name(path);
+    util::traverse_directory(dir, [&](const auto& de) {
+      auto name = de.path().filename().string();
       if (name == "CACHEDIR.TAG" || name == "stats"
-          || util::starts_with(std::string(name), ".nfs")) {
+          || util::starts_with(name, ".nfs")) {
         return;
       }
 
-      if (!is_dir) {
-        files.emplace_back(path);
+      if (!de.is_directory()) {
+        files.emplace_back(de);
       }
     }));
 

diff --git a/src/util/DirEntry.cpp b/src/util/DirEntry.cpp
index c6335dd150e7827551175b780653725961e36a5d..ca686050f763f9276aa42cc803423b371f00f35d 100644 (file)
--- a/src/util/DirEntry.cpp
+++ b/src/util/DirEntry.cpp
@@ -22,6 +22,7 @@
 #include <Logging.hpp>
 #include <Win32Util.hpp>
 #include <fmtmacros.hpp>
+#include <util/file.hpp>
 #include <util/wincompat.hpp>
 
 #ifdef _WIN32
@@ -210,6 +211,16 @@ auto stat_func = ::stat;
 
 namespace util {
 
+uint64_t
+DirEntry::size_on_disk() const
+{
+#ifdef _WIN32
+  return util::likely_size_on_disk(size());
+#else
+  return do_stat().st_blocks * 512;
+#endif
+}
+
 const DirEntry::stat_t&
 DirEntry::do_stat() const
 {

diff --git a/src/util/DirEntry.hpp b/src/util/DirEntry.hpp
index 9b4b81c0b7129a498d3006e195dc6a564b5e184b..8d5b5314f412f3d3e3b8e4e9edc79fe03ea8f337 100644 (file)
--- a/src/util/DirEntry.hpp
+++ b/src/util/DirEntry.hpp
@@ -19,7 +19,6 @@
 #pragma once
 
 #include <util/TimePoint.hpp>
-#include <util/file.hpp>
 #include <util/wincompat.hpp>
 
 #include <sys/stat.h>
@@ -226,16 +225,6 @@ DirEntry::size() const
   return do_stat().st_size;
 }
 
-inline uint64_t
-DirEntry::size_on_disk() const
-{
-#ifdef _WIN32
-  return util::likely_size_on_disk(size());
-#else
-  return do_stat().st_blocks * 512;
-#endif
-}
-
 inline bool
 DirEntry::is_directory() const
 {

diff --git a/src/util/file.cpp b/src/util/file.cpp
index 8099a102277cd6dc90d26ac9ad5516ac1f87afc9..8882fa5df3934b2033b39215618fedd8477bd280 100644 (file)
--- a/src/util/file.cpp
+++ b/src/util/file.cpp
@@ -514,10 +514,10 @@ traverse_directory(const fs::path& directory,
     if (is_dir) {
       traverse_directory(path, visitor);
     } else {
-      visitor(path.string(), false);
+      visitor(path);
     }
   }
-  visitor(directory.string(), true);
+  visitor(directory);
 
   return {};
 }
@@ -542,12 +542,12 @@ traverse_directory(const fs::path& directory,
   try {
     for (const auto& entry : std::filesystem::directory_iterator(directory)) {
       if (entry.is_directory()) {
-        traverse_directory(entry.path().string(), visitor);
+        traverse_directory(entry.path(), visitor);
       } else {
-        visitor(entry.path().string(), entry.is_directory());
+        visitor(entry.path());
       }
     }
-    visitor(directory.string(), true);
+    visitor(directory);
   } catch (const std::filesystem::filesystem_error& e) {
     return tl::unexpected(e.what());
   }

diff --git a/src/util/file.hpp b/src/util/file.hpp
index 7fd25003b75e0c5a8d010a5e8367320ee34a333a..a7942d67edb6dfcb7b8b4d75f14eaa4701f6a619 100644 (file)
--- a/src/util/file.hpp
+++ b/src/util/file.hpp
@@ -19,6 +19,7 @@
 #pragma once
 
 #include <util/Bytes.hpp>
+#include <util/DirEntry.hpp>
 #include <util/TimePoint.hpp>
 #include <util/types.hpp>
 
@@ -43,8 +44,7 @@ enum class InPlace { yes, no };
 enum class LogFailure { yes, no };
 enum class ViaTmpFile { yes, no };
 
-using TraverseDirectoryVisitor =
-  std::function<void(const std::string& path, bool is_dir)>;
+using TraverseDirectoryVisitor = std::function<void(const DirEntry& dir_entry)>;
 
 // Copy a file from `src` to `dest`. If `via_tmp_file` is yes, `src` is copied
 // to a temporary file and then renamed to dest.

diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp
index 4af0b07586f1f5e1f95148d4447551415f4c8082..30936b6f368656fe55ba62c7ae5fce04f2821b2d 100644 (file)
--- a/unittest/test_util_file.cpp
+++ b/unittest/test_util_file.cpp
@@ -217,8 +217,8 @@ TEST_CASE("util::traverse_directory")
   REQUIRE(fs::create_directory("empty-dir"));
 
   std::vector<std::string> visited;
-  auto visitor = [&visited](const std::string& path, bool is_dir) {
-    visited.push_back(FMT("[{}] {}", is_dir ? 'd' : 'f', path));
+  auto visitor = [&visited](const auto& de) {
+    visited.push_back(FMT("[{}] {}", de.is_directory() ? 'd' : 'f', de.path()));
   };
 
   SUBCASE("traverse nonexistent path")