From: Joel Rosdahl <joel@rosdahl.net>
Date: Thu, 27 Jul 2023 17:11:09 +0000 (+0200)
Subject: refactor: Remove Stat::OnError::throw_error
X-Git-Tag: v4.9~84
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3974856a9b6f945162f827f59f048e55f2e64cb4;p=thirdparty%2Fccache.git

refactor: Remove Stat::OnError::throw_error

This is in preparation for moving the Stat class to util where we don't
want to know about core::Error.
---

diff --git a/src/Stat.cpp b/src/Stat.cpp
index 15293e12b..1d3c4f235 100644
--- a/src/Stat.cpp
+++ b/src/Stat.cpp
@@ -205,7 +205,7 @@ win32_lstat(const char* path, Stat::stat_t* st)
 
 Stat::Stat(StatFunction stat_function,
            const std::string& path,
-           Stat::OnError on_error)
+           Stat::LogOnError log_on_error)
   : m_path(path)
 {
   int result = stat_function(path.c_str(), &m_stat);
@@ -213,10 +213,7 @@ Stat::Stat(StatFunction stat_function,
     m_errno = 0;
   } else {
     m_errno = errno;
-    if (on_error == OnError::throw_error) {
-      throw core::Error(FMT("failed to stat {}: {}", path, strerror(errno)));
-    }
-    if (on_error == OnError::log) {
+    if (log_on_error == LogOnError::yes) {
       LOG("Failed to stat {}: {}", path, strerror(errno));
     }
 
@@ -227,7 +224,7 @@ Stat::Stat(StatFunction stat_function,
 }
 
 Stat
-Stat::stat(const std::string& path, OnError on_error)
+Stat::stat(const std::string& path, LogOnError log_on_error)
 {
   return Stat(
 #ifdef _WIN32
@@ -236,11 +233,11 @@ Stat::stat(const std::string& path, OnError on_error)
     ::stat,
 #endif
     path,
-    on_error);
+    log_on_error);
 }
 
 Stat
-Stat::lstat(const std::string& path, OnError on_error)
+Stat::lstat(const std::string& path, LogOnError log_on_error)
 {
   return Stat(
 #ifdef _WIN32
@@ -249,5 +246,5 @@ Stat::lstat(const std::string& path, OnError on_error)
     ::lstat,
 #endif
     path,
-    on_error);
+    log_on_error);
 }
diff --git a/src/Stat.hpp b/src/Stat.hpp
index 5452872a9..c29224b84 100644
--- a/src/Stat.hpp
+++ b/src/Stat.hpp
@@ -62,16 +62,7 @@
 class Stat
 {
 public:
-  enum class OnError {
-    // Ignore any error (including missing file) from the underlying stat call.
-    // On error, error_number() will return the error number (AKA errno) and
-    // the query functions will return 0 or false.
-    ignore,
-    // Like above but log an error message as well.
-    log,
-    // Throw Error on errors (including missing file).
-    throw_error,
-  };
+  enum class LogOnError { no, yes };
 
 #if defined(_WIN32)
   struct stat_t
@@ -98,20 +89,13 @@ public:
   // error_number() will return -1 and other accessors will return false or 0.
   Stat();
 
-  // Run stat(2).
-  //
-  // Arguments:
-  // - path: Path to stat.
-  // - on_error: What to do on errors (including missing file).
-  static Stat stat(const std::string& path, OnError on_error = OnError::ignore);
-
-  // Run lstat(2) if available, otherwise stat(2).
-  //
-  // Arguments:
-  // - path: Path to (l)stat.
-  // - on_error: What to do on errors (including missing file).
+  // Run stat(2) on `path`.
+  static Stat stat(const std::string& path,
+                   LogOnError log_on_error = LogOnError::no);
+
+  // Run lstat(2) on `path` if available, otherwise stat(2).
   static Stat lstat(const std::string& path,
-                    OnError on_error = OnError::ignore);
+                    LogOnError log_on_error = LogOnError::no);
 
   // Return true if the file could be (l)stat-ed (i.e., the file exists),
   // otherwise false.
@@ -149,7 +133,9 @@ public:
 protected:
   using StatFunction = int (*)(const char*, stat_t*);
 
-  Stat(StatFunction stat_function, const std::string& path, OnError on_error);
+  Stat(StatFunction stat_function,
+       const std::string& path,
+       LogOnError log_on_error);
 
 private:
   std::string m_path;
diff --git a/src/ccache.cpp b/src/ccache.cpp
index c9e6ed431..db14fd0d7 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -352,7 +352,7 @@ do_remember_include_file(Context& ctx,
   }
 #endif
 
-  auto st = Stat::stat(path, Stat::OnError::log);
+  auto st = Stat::stat(path, Stat::LogOnError::yes);
   if (!st) {
     return false;
   }
@@ -402,7 +402,7 @@ do_remember_include_file(Context& ctx,
       // hash pch.sum instead of pch when it exists
       // to prevent hashing a very large .pch file every time
       std::string pch_sum_path = FMT("{}.sum", path);
-      if (Stat::stat(pch_sum_path, Stat::OnError::log)) {
+      if (Stat::stat(pch_sum_path, Stat::LogOnError::yes)) {
         path = std::move(pch_sum_path);
         using_pch_sum = true;
         LOG("Using pch.sum file {}", path);
@@ -860,7 +860,7 @@ update_manifest(Context& ctx,
 
   const bool added = ctx.manifest.add_result(
     result_key, ctx.included_files, [&](const std::string& path) {
-      auto stat = Stat::stat(path, Stat::OnError::log);
+      auto stat = Stat::stat(path, Stat::LogOnError::yes);
       bool cache_time =
         save_timestamp
         && ctx.time_of_compilation > std::max(stat.mtime(), stat.ctime());
@@ -1368,7 +1368,7 @@ hash_nvcc_host_compiler(const Context& ctx,
       } else {
         std::string path = find_executable(ctx, compiler, ctx.orig_args[0]);
         if (!path.empty()) {
-          auto st = Stat::stat(path, Stat::OnError::log);
+          auto st = Stat::stat(path, Stat::LogOnError::yes);
           TRY(hash_compiler(ctx, hash, st, ccbin, false));
         }
       }
@@ -1405,7 +1405,7 @@ hash_common_info(const Context& ctx,
   const std::string compiler_path = args[0];
 #endif
 
-  auto st = Stat::stat(compiler_path, Stat::OnError::log);
+  auto st = Stat::stat(compiler_path, Stat::LogOnError::yes);
   if (!st) {
     return tl::unexpected(Statistic::could_not_find_compiler);
   }
@@ -1744,7 +1744,7 @@ hash_argument(const Context& ctx,
     } else {
       path = args[i].substr(eq_pos + 1);
     }
-    auto st = Stat::stat(path, Stat::OnError::log);
+    auto st = Stat::stat(path, Stat::LogOnError::yes);
     if (st) {
       // If given an explicit specs file, then hash that file, but don't
       // include the path to it in the hash.
@@ -1755,7 +1755,7 @@ hash_argument(const Context& ctx,
   }
 
   if (util::starts_with(args[i], "-fplugin=")) {
-    auto st = Stat::stat(&args[i][9], Stat::OnError::log);
+    auto st = Stat::stat(&args[i][9], Stat::LogOnError::yes);
     if (st) {
       hash.hash_delimiter("plugin");
       TRY(hash_compiler(ctx, hash, st, &args[i][9], false));
@@ -1765,7 +1765,7 @@ hash_argument(const Context& ctx,
 
   if (args[i] == "-Xclang" && i + 3 < args.size() && args[i + 1] == "-load"
       && args[i + 2] == "-Xclang") {
-    auto st = Stat::stat(args[i + 3], Stat::OnError::log);
+    auto st = Stat::stat(args[i + 3], Stat::LogOnError::yes);
     if (st) {
       hash.hash_delimiter("plugin");
       TRY(hash_compiler(ctx, hash, st, args[i + 3], false));
diff --git a/src/core/FileRecompressor.cpp b/src/core/FileRecompressor.cpp
index 92aa6c7a3..11e1f4799 100644
--- a/src/core/FileRecompressor.cpp
+++ b/src/core/FileRecompressor.cpp
@@ -56,7 +56,7 @@ FileRecompressor::recompress(const Stat& stat,
     new_cache_file.write(
       core::CacheEntry::serialize(header, cache_entry.payload()));
     new_cache_file.commit();
-    new_stat = Stat::lstat(stat.path(), Stat::OnError::log);
+    new_stat = Stat::lstat(stat.path(), Stat::LogOnError::yes);
   }
 
   // Restore mtime/atime to keep cache LRU cleanup working as expected:
diff --git a/src/core/Result.cpp b/src/core/Result.cpp
index b51b912f7..01af0986d 100644
--- a/src/core/Result.cpp
+++ b/src/core/Result.cpp
@@ -284,10 +284,10 @@ Serializer::serialize(util::Bytes& output)
     const bool store_raw =
       is_file_entry && should_store_raw_file(m_config, entry.file_type);
     const uint64_t file_size =
-      is_file_entry ? Stat::stat(std::get<std::string>(entry.data),
-                                 Stat::OnError::throw_error)
-                        .size()
-                    : std::get<nonstd::span<const uint8_t>>(entry.data).size();
+      is_file_entry
+        ? Stat::stat(std::get<std::string>(entry.data), Stat::LogOnError::yes)
+            .size()
+        : std::get<nonstd::span<const uint8_t>>(entry.data).size();
 
     LOG("Storing {} entry #{} {} ({} bytes){}",
         store_raw ? "raw" : "embedded",
diff --git a/src/core/ResultExtractor.cpp b/src/core/ResultExtractor.cpp
index 1969876cc..fe1366ab4 100644
--- a/src/core/ResultExtractor.cpp
+++ b/src/core/ResultExtractor.cpp
@@ -73,7 +73,11 @@ ResultExtractor::on_raw_file(uint8_t file_number,
     throw Error("Raw entry for non-local result");
   }
   const auto raw_file_path = (*m_get_raw_file_path)(file_number);
-  const auto st = Stat::stat(raw_file_path, Stat::OnError::throw_error);
+  const auto st = Stat::stat(raw_file_path, Stat::LogOnError::yes);
+  if (!st) {
+    throw Error(
+      FMT("Failed to stat {}: {}", raw_file_path, strerror(st.error_number())));
+  }
   if (st.size() != file_size) {
     throw Error(FMT("Bad file size of {} (actual {} bytes, expected {} bytes)",
                     raw_file_path,
diff --git a/src/core/ResultRetriever.cpp b/src/core/ResultRetriever.cpp
index 5e5b3a9b9..45cf533b2 100644
--- a/src/core/ResultRetriever.cpp
+++ b/src/core/ResultRetriever.cpp
@@ -104,7 +104,11 @@ ResultRetriever::on_raw_file(uint8_t file_number,
   }
   const auto raw_file_path =
     m_ctx.storage.local.get_raw_file_path(*m_result_key, file_number);
-  const auto st = Stat::stat(raw_file_path, Stat::OnError::throw_error);
+  const auto st = Stat::stat(raw_file_path, Stat::LogOnError::yes);
+  if (!st) {
+    throw Error(
+      FMT("Failed to stat {}: {}", raw_file_path, strerror(st.error_number())));
+  }
   if (st.size() != file_size) {
     throw core::Error(
       FMT("Bad file size of {} (actual {} bytes, expected {} bytes)",
diff --git a/src/core/mainoptions.cpp b/src/core/mainoptions.cpp
index 2cfc8e50f..0c28a7acd 100644
--- a/src/core/mainoptions.cpp
+++ b/src/core/mainoptions.cpp
@@ -719,7 +719,7 @@ process_main_options(int argc, const char* const* argv)
       }
       Statistics statistics(StatsLog(config.stats_log()).read());
       const auto timestamp =
-        Stat::stat(config.stats_log(), Stat::OnError::log).mtime();
+        Stat::stat(config.stats_log(), Stat::LogOnError::yes).mtime();
       PRINT_RAW(
         stdout,
         statistics.format_human_readable(config, timestamp, verbosity, true));
diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index 96963c35b..ad185ab17 100644
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -533,7 +533,7 @@ LocalStorage::put(const Hash::Digest& key,
 
   increment_statistic(Statistic::local_storage_write);
 
-  const auto new_stat = Stat::stat(cache_file.path, Stat::OnError::log);
+  const auto new_stat = Stat::stat(cache_file.path, Stat::LogOnError::yes);
   if (!new_stat) {
     return;
   }
@@ -1424,7 +1424,7 @@ LocalStorage::clean_internal_tempdir()
       if (is_dir) {
         return;
       }
-      const auto st = Stat::lstat(path, Stat::OnError::log);
+      const auto st = Stat::lstat(path, Stat::LogOnError::yes);
       if (st && st.mtime() + k_tempdir_cleanup_interval < now) {
         util::remove(path);
       }
diff --git a/unittest/test_Stat.cpp b/unittest/test_Stat.cpp
index fdba8c2ba..1e45d905f 100644
--- a/unittest/test_Stat.cpp
+++ b/unittest/test_Stat.cpp
@@ -174,10 +174,8 @@ TEST_CASE("Default constructor")
 TEST_CASE("Named constructors")
 {
   CHECK(!Stat::stat("does_not_exist"));
-  CHECK(!Stat::stat("does_not_exist", Stat::OnError::ignore));
-  CHECK(!Stat::stat("does_not_exist", Stat::OnError::log));
-  CHECK_THROWS_WITH(Stat::stat("does_not_exist", Stat::OnError::throw_error),
-                    "failed to stat does_not_exist: No such file or directory");
+  CHECK(!Stat::stat("does_not_exist", Stat::LogOnError::no));
+  CHECK(!Stat::stat("does_not_exist", Stat::LogOnError::yes));
 }
 
 TEST_CASE("Same i-node as")
@@ -339,7 +337,7 @@ TEST_CASE("Symlinks" * doctest::skip(!symlinks_supported()))
 
   SUBCASE("file lstat")
   {
-    auto stat = Stat::lstat("file", Stat::OnError::ignore);
+    auto stat = Stat::lstat("file", Stat::LogOnError::no);
     CHECK(stat);
     CHECK(stat.error_number() == 0);
     CHECK(!stat.is_directory());
@@ -356,7 +354,7 @@ TEST_CASE("Symlinks" * doctest::skip(!symlinks_supported()))
 
   SUBCASE("file stat")
   {
-    auto stat = Stat::stat("file", Stat::OnError::ignore);
+    auto stat = Stat::stat("file", Stat::LogOnError::no);
     CHECK(stat);
     CHECK(stat.error_number() == 0);
     CHECK(!stat.is_directory());
@@ -373,7 +371,7 @@ TEST_CASE("Symlinks" * doctest::skip(!symlinks_supported()))
 
   SUBCASE("symlink lstat")
   {
-    auto stat = Stat::lstat("symlink", Stat::OnError::ignore);
+    auto stat = Stat::lstat("symlink", Stat::LogOnError::no);
     CHECK(stat);
     CHECK(stat.error_number() == 0);
     CHECK(!stat.is_directory());
@@ -391,7 +389,7 @@ TEST_CASE("Symlinks" * doctest::skip(!symlinks_supported()))
 
   SUBCASE("symlink stat")
   {
-    auto stat = Stat::stat("symlink", Stat::OnError::ignore);
+    auto stat = Stat::stat("symlink", Stat::LogOnError::no);
     CHECK(stat);
     CHECK(stat.error_number() == 0);
     CHECK(!stat.is_directory());