From: Joel Rosdahl <joel@rosdahl.net>
Date: Sun, 16 Jul 2023 08:22:06 +0000 (+0200)
Subject: refactor: Use std::filesystem::create_director{ies,y}
X-Git-Tag: v4.9~118
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1adaf8ef85ec5dea259cd1b3f836edff8b5874bd;p=thirdparty%2Fccache.git

refactor: Use std::filesystem::create_director{ies,y}
---

diff --git a/src/Config.cpp b/src/Config.cpp
index ffb722f5d..38fa4432d 100644
--- a/src/Config.cpp
+++ b/src/Config.cpp
@@ -33,6 +33,7 @@
 #include <util/environment.hpp>
 #include <util/expected.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
 #include <util/path.hpp>
 #include <util/string.hpp>
 
@@ -57,6 +58,8 @@ DLLIMPORT extern char** environ;
 // Make room for binary patching at install time.
 const char k_sysconfdir[4096 + 1] = SYSCONFDIR;
 
+namespace fs = util::filesystem;
+
 namespace {
 
 enum class ConfigItem {
@@ -1157,7 +1160,7 @@ Config::default_temporary_dir() const
     const char* const xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
     if (xdg_runtime_dir && Stat::stat(xdg_runtime_dir).is_directory()) {
       auto dir = FMT("{}/ccache-tmp", xdg_runtime_dir);
-      if (Util::create_dir(dir) && access(dir.c_str(), W_OK) == 0) {
+      if (fs::create_directories(dir) && access(dir.c_str(), W_OK) == 0) {
         return dir;
       }
     }
diff --git a/src/Util.cpp b/src/Util.cpp
index af397ea01..20b35cc57 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -186,34 +186,6 @@ common_dir_prefix_length(std::string_view dir, std::string_view path)
   return i;
 }
 
-bool
-create_dir(std::string_view dir)
-{
-  std::string dir_str(dir);
-  auto st = Stat::stat(dir_str);
-  if (st) {
-    if (st.is_directory()) {
-      return true;
-    } else {
-      errno = ENOTDIR;
-      return false;
-    }
-  } else {
-    if (!create_dir(Util::dir_name(dir))) {
-      return false;
-    }
-    int result = mkdir(dir_str.c_str(), 0777);
-    // Treat an already existing directory as OK since the file system could
-    // have changed in between calling stat and actually creating the
-    // directory. This can happen when there are multiple instances of ccache
-    // running and trying to create the same directory chain, which usually is
-    // the case when the cache root does not initially exist. As long as one of
-    // the processes creates the directories then our condition is satisfied
-    // and we avoid a race condition.
-    return result == 0 || errno == EEXIST;
-  }
-}
-
 std::string_view
 dir_name(std::string_view path)
 {
@@ -260,9 +232,9 @@ format_argv_for_logging(const char* const* argv)
 void
 ensure_dir_exists(std::string_view dir)
 {
-  if (!create_dir(dir)) {
+  if (auto result = fs::create_directories(dir); !result) {
     throw core::Fatal(
-      FMT("Failed to create directory {}: {}", dir, strerror(errno)));
+      FMT("Failed to create directory {}: {}", dir, result.error().message()));
   }
 }
 
diff --git a/src/Util.hpp b/src/Util.hpp
index 5c47bf249..1a79a1c92 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -51,11 +51,6 @@ std::string change_extension(std::string_view path, std::string_view new_ext);
 // `dir` (a directory) and `path` (any path).
 size_t common_dir_prefix_length(std::string_view dir, std::string_view path);
 
-// Create a directory if needed, including its parents if needed.
-//
-// Returns true if the directory exists or could be created, otherwise false.
-bool create_dir(std::string_view dir);
-
 // Get directory name of path.
 std::string_view dir_name(std::string_view path);
 
diff --git a/src/ccache.cpp b/src/ccache.cpp
index 8002f7ccc..0a6708351 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -190,10 +190,10 @@ prepare_debug_path(const std::string& debug_dir,
                   ? output_obj
                   : debug_dir + util::to_absolute_path_no_drive(output_obj);
 
-  // Ignore any error from create_dir since we can't handle an error in another
-  // way in this context. The caller takes care of logging when trying to open
-  // the path for writing.
-  Util::create_dir(Util::dir_name(prefix));
+  // Ignore any error from fs::create_directories since we can't handle an error
+  // in another way in this context. The caller takes care of logging when
+  // trying to open the path for writing.
+  fs::create_directories(Util::dir_name(prefix));
 
   char timestamp[100];
   const auto tm = Util::localtime(time_of_invocation);
diff --git a/src/storage/remote/FileStorage.cpp b/src/storage/remote/FileStorage.cpp
index 917cb2b82..0d3848eba 100644
--- a/src/storage/remote/FileStorage.cpp
+++ b/src/storage/remote/FileStorage.cpp
@@ -29,12 +29,15 @@
 #include <util/UmaskScope.hpp>
 #include <util/expected.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
 #include <util/string.hpp>
 
 #include <sys/stat.h> // for mode_t
 
 #include <string_view>
 
+namespace fs = util::filesystem;
+
 namespace storage::remote {
 
 namespace {
@@ -149,8 +152,8 @@ FileStorageBackend::put(const Hash::Digest& key,
     util::UmaskScope umask_scope(m_umask);
 
     const auto dir = Util::dir_name(path);
-    if (!Util::create_dir(dir)) {
-      LOG("Failed to create directory {}: {}", dir, strerror(errno));
+    if (auto result = fs::create_directories(dir); !result) {
+      LOG("Failed to create directory {}: {}", dir, result.error().message());
       return nonstd::make_unexpected(Failure::error);
     }
 
diff --git a/src/util/LockFile.cpp b/src/util/LockFile.cpp
index 9fb998f45..cfa3625d2 100644
--- a/src/util/LockFile.cpp
+++ b/src/util/LockFile.cpp
@@ -247,7 +247,7 @@ LockFile::do_acquire(const bool blocking)
     int saved_errno = errno;
     if (saved_errno == ENOENT) {
       // Directory doesn't exist?
-      if (Util::create_dir(Util::dir_name(m_lock_file))) {
+      if (fs::create_directories(Util::dir_name(m_lock_file))) {
         // OK. Retry.
         continue;
       }
@@ -380,7 +380,7 @@ LockFile::do_acquire(const bool blocking)
     DWORD error = GetLastError();
     if (error == ERROR_PATH_NOT_FOUND) {
       // Directory doesn't exist?
-      if (Util::create_dir(Util::dir_name(m_lock_file))) {
+      if (fs::create_directories(Util::dir_name(m_lock_file))) {
         // OK. Retry.
         continue;
       }
diff --git a/src/util/filesystem.hpp b/src/util/filesystem.hpp
index f780164b9..03f990e26 100644
--- a/src/util/filesystem.hpp
+++ b/src/util/filesystem.hpp
@@ -58,6 +58,8 @@ using path = std::filesystem::path;
   }
 
 DEFINE_FS_WRAPPER(canonical, (path{}))
+DEFINE_FS_WRAPPER(create_directories, (path{}))
+DEFINE_FS_WRAPPER(create_directory, (path{}))
 DEFINE_FS_WRAPPER(create_hard_link, (path{}, path{}))
 DEFINE_FS_WRAPPER(current_path, ())
 DEFINE_FS_WRAPPER(read_symlink, (path{}))
diff --git a/unittest/TestUtil.cpp b/unittest/TestUtil.cpp
index ea8abc952..0f07d0dc6 100644
--- a/unittest/TestUtil.cpp
+++ b/unittest/TestUtil.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2022 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -23,11 +23,14 @@
 #include <core/exceptions.hpp>
 #include <core/wincompat.hpp>
 #include <fmtmacros.hpp>
+#include <util/filesystem.hpp>
 
 #ifdef HAVE_UNISTD_H
 #  include <unistd.h>
 #endif
 
+namespace fs = util::filesystem;
+
 namespace TestUtil {
 
 size_t TestContext::m_subdir_counter = 0;
@@ -39,7 +42,7 @@ TestContext::TestContext() : m_test_dir(Util::get_actual_cwd())
   }
   ++m_subdir_counter;
   std::string subtest_dir = FMT("{}/test_{}", m_test_dir, m_subdir_counter);
-  Util::create_dir(subtest_dir);
+  fs::create_directories(subtest_dir);
   if (chdir(subtest_dir.c_str()) != 0) {
     abort();
   }
diff --git a/unittest/main.cpp b/unittest/main.cpp
index d67126f71..f2aab3ad4 100644
--- a/unittest/main.cpp
+++ b/unittest/main.cpp
@@ -21,6 +21,7 @@
 #include "TestUtil.hpp"
 
 #include <util/environment.hpp>
+#include <util/filesystem.hpp>
 
 #include "third_party/fmt/core.h"
 
@@ -28,6 +29,8 @@
 #define DOCTEST_CONFIG_IMPLEMENT
 #include "third_party/doctest.h"
 
+namespace fs = util::filesystem;
+
 int
 main(int argc, char** argv)
 {
@@ -39,7 +42,7 @@ main(int argc, char** argv)
   std::string dir_before = Util::get_actual_cwd();
   std::string testdir = FMT("testdir/{}", getpid());
   Util::wipe_path(testdir);
-  Util::create_dir(testdir);
+  fs::create_directories(testdir);
   TestUtil::check_chdir(testdir);
 
   doctest::Context context;
diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index 331482aa9..195e1caf9 100644
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -27,6 +27,7 @@
 #include <core/wincompat.hpp>
 #include <util/environment.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
 
 #include "third_party/doctest.h"
 
@@ -44,6 +45,8 @@
 
 using TestUtil::TestContext;
 
+namespace fs = util::filesystem;
+
 TEST_SUITE_BEGIN("Util");
 
 TEST_CASE("Util::base_name")
@@ -89,19 +92,6 @@ TEST_CASE("Util::common_dir_prefix_length")
   CHECK(Util::common_dir_prefix_length("/a/b", "/a/bc") == 2);
 }
 
-TEST_CASE("Util::create_dir")
-{
-  TestContext test_context;
-
-  CHECK(Util::create_dir("/"));
-
-  CHECK(Util::create_dir("create/dir"));
-  CHECK(Stat::stat("create/dir").is_directory());
-
-  util::write_file("create/dir/file", "");
-  CHECK(!Util::create_dir("create/dir/file"));
-}
-
 TEST_CASE("Util::dir_name")
 {
   CHECK(Util::dir_name("") == ".");
@@ -145,7 +135,7 @@ TEST_CASE("Util::ensure_dir_exists")
   util::write_file("create/dir/file", "");
   CHECK_THROWS_WITH(
     Util::ensure_dir_exists("create/dir/file"),
-    "Failed to create directory create/dir/file: Not a directory");
+    doctest::Contains("Failed to create directory create/dir/file:"));
 }
 
 TEST_CASE("Util::format_argv_for_logging")
@@ -265,7 +255,7 @@ TEST_CASE("Util::make_relative_path")
   const std::string apparent_cwd = FMT("{}/s", cwd);
 #endif
 
-  REQUIRE(Util::create_dir("d"));
+  REQUIRE(fs::create_directory("d"));
 #ifndef _WIN32
   REQUIRE(symlink("d", "s") == 0);
 #endif
@@ -388,7 +378,7 @@ TEST_CASE("Util::normalize_concrete_absolute_path")
   TestContext test_context;
 
   util::write_file("file", "");
-  REQUIRE(Util::create_dir("dir1/dir2"));
+  REQUIRE(fs::create_directories("dir1/dir2"));
   REQUIRE(symlink("dir1/dir2", "symlink") == 0);
   const auto cwd = Util::get_actual_cwd();
 
@@ -428,12 +418,12 @@ TEST_CASE("Util::traverse")
 {
   TestContext test_context;
 
-  REQUIRE(Util::create_dir("dir-with-subdir-and-file/subdir"));
+  REQUIRE(fs::create_directories("dir-with-subdir-and-file/subdir"));
   util::write_file("dir-with-subdir-and-file/subdir/f", "");
-  REQUIRE(Util::create_dir("dir-with-files"));
+  REQUIRE(fs::create_directory("dir-with-files"));
   util::write_file("dir-with-files/f1", "");
   util::write_file("dir-with-files/f2", "");
-  REQUIRE(Util::create_dir("empty-dir"));
+  REQUIRE(fs::create_directory("empty-dir"));
 
   std::vector<std::string> visited;
   auto visitor = [&visited](const std::string& path, bool is_dir) {
@@ -500,7 +490,7 @@ TEST_CASE("Util::wipe_path")
 
   SUBCASE("Wipe directory")
   {
-    REQUIRE(Util::create_dir("a/b"));
+    REQUIRE(fs::create_directories("a/b"));
     util::write_file("a/1", "");
     util::write_file("a/b/1", "");
     CHECK_NOTHROW(Util::wipe_path("a"));
diff --git a/unittest/test_storage_local_util.cpp b/unittest/test_storage_local_util.cpp
index 5c3cce2d5..b44468709 100644
--- a/unittest/test_storage_local_util.cpp
+++ b/unittest/test_storage_local_util.cpp
@@ -18,10 +18,10 @@
 
 #include "TestUtil.hpp"
 
-#include <Util.hpp>
 #include <fmtmacros.hpp>
 #include <storage/local/util.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
 
 #include <third_party/doctest.h>
 
@@ -31,6 +31,8 @@
 
 using TestUtil::TestContext;
 
+namespace fs = util::filesystem;
+
 static inline std::string
 os_path(std::string path)
 {
@@ -59,10 +61,10 @@ TEST_CASE("storage::local::get_cache_dir_files")
 {
   TestContext test_context;
 
-  Util::create_dir("e/m/p/t/y");
+  fs::create_directories("e/m/p/t/y");
 
-  Util::create_dir("0/1");
-  Util::create_dir("0/f/c");
+  fs::create_directories("0/1");
+  fs::create_directories("0/f/c");
   util::write_file("0/file_a", "");
   util::write_file("0/1/file_b", "1");
   util::write_file("0/1/file_c", "12");