From: Joel Rosdahl <joel@rosdahl.net>
Date: Tue, 2 Jan 2024 15:11:32 +0000 (+0100)
Subject: refactor: Convert usage of Util::dir_name to std::filesystem
X-Git-Tag: v4.10~136
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e3d2fc61ffa09078a90f8623c9337cfa440317d;p=thirdparty%2Fccache.git

refactor: Convert usage of Util::dir_name to std::filesystem
---

diff --git a/src/Util.cpp b/src/Util.cpp
index 5f2e791c3..cf27a1601 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -75,32 +75,6 @@ common_dir_prefix_length(std::string_view dir, std::string_view path)
   return i;
 }
 
-std::string_view
-dir_name(std::string_view path)
-{
-#ifdef _WIN32
-  const char delim[] = "/\\";
-#else
-  const char delim[] = "/";
-#endif
-  size_t n = path.find_last_of(delim);
-  if (n == std::string::npos) {
-    // "foo" -> "."
-    return ".";
-  } else if (n == 0) {
-    // "/" -> "/" (Windows: or "\\" -> "\\")
-    return path.substr(0, 1);
-#ifdef _WIN32
-  } else if (n == 2 && path[1] == ':') {
-    // Windows: "C:\\foo" -> "C:\\" or "C:/foo" -> "C:/"
-    return path.substr(0, 3);
-#endif
-  } else {
-    // "/dir/foo" -> "/dir" (Windows: or "C:\\dir\\foo" -> "C:\\dir")
-    return path.substr(0, n);
-  }
-}
-
 std::string
 get_relative_path(std::string_view dir, std::string_view path)
 {
@@ -201,13 +175,14 @@ make_relative_path(const std::string& base_dir,
 
   std::vector<std::string> relpath_candidates;
   const auto original_path = path;
-  DirEntry dir_entry(path);
-  while (!dir_entry.exists()) {
-    path = Util::dir_name(path);
-    dir_entry = DirEntry(path);
+  fs::path path_path = path;
+  while (!fs::exists(path_path)) {
+    path_path = path_path.parent_path();
   }
+  std::string path_str = path_path.string();
+  path = path_str;
   const auto path_suffix = std::string(original_path.substr(path.length()));
-  const std::string real_path = fs::canonical(path).value_or(path).string();
+  const fs::path real_path = fs::canonical(path).value_or(fs::path(path));
 
   const auto add_relpath_candidates = [&](auto p) {
     const std::string normalized_path =
@@ -221,7 +196,7 @@ make_relative_path(const std::string& base_dir,
   };
   add_relpath_candidates(path);
   if (real_path != path) {
-    add_relpath_candidates(real_path);
+    add_relpath_candidates(real_path.string());
   }
 
   // Find best (i.e. shortest existing) match:
@@ -231,7 +206,7 @@ make_relative_path(const std::string& base_dir,
               return path1.length() < path2.length();
             });
   for (const auto& relpath : relpath_candidates) {
-    if (DirEntry(relpath).same_inode_as(dir_entry)) {
+    if (fs::equivalent(relpath, path_path)) {
       return relpath + path_suffix;
     }
   }
diff --git a/src/Util.hpp b/src/Util.hpp
index f8a6c3e05..891cd7532 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -39,9 +39,6 @@ namespace Util {
 // `dir` (a directory) and `path` (any path).
 size_t common_dir_prefix_length(std::string_view dir, std::string_view path);
 
-// Get directory name of path.
-std::string_view dir_name(std::string_view path);
-
 // Compute a relative path from `dir` (an absolute path to a directory) to
 // `path` (an absolute path). Assumes that both `dir` and `path` are normalized.
 // The algorithm does *not* follow symlinks, so the result may not actually
diff --git a/src/argprocessing.cpp b/src/argprocessing.cpp
index c5545b55f..d1fbae526 100644
--- a/src/argprocessing.cpp
+++ b/src/argprocessing.cpp
@@ -1420,8 +1420,8 @@ process_args(Context& ctx)
     args_info.generating_dependencies = false;
   }
 
-  auto output_dir = Util::dir_name(args_info.output_obj);
-  if (!DirEntry(output_dir).is_directory()) {
+  fs::path output_dir = fs::path(args_info.output_obj).parent_path();
+  if (!output_dir.empty() && !fs::is_directory(output_dir)) {
     LOG("Directory does not exist: {}", output_dir);
     return Statistic::bad_output_file;
   }
diff --git a/src/ccache.cpp b/src/ccache.cpp
index 37c4122f1..362d761c6 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -1510,7 +1510,7 @@ hash_common_info(const Context& ctx,
   if (ctx.config.is_compiler_group_msvc() && ctx.config.hash_dir()) {
     const std::string output_obj_dir =
       fs::path(args_info.output_obj).is_absolute()
-        ? std::string(Util::dir_name(args_info.output_obj))
+        ? fs::path(args_info.output_obj).parent_path().string()
         : ctx.actual_cwd;
     LOG("Hashing object file directory {}", output_obj_dir);
     hash.hash_delimiter("source path");
diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index 99b0d40ae..daac0fbe7 100644
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2023 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -21,7 +21,6 @@
 #include <Config.hpp>
 #include <Context.hpp>
 #include <MiniTrace.hpp>
-#include <Util.hpp>
 #include <core/AtomicFile.hpp>
 #include <core/CacheEntry.hpp>
 #include <core/FileRecompressor.hpp>
@@ -648,7 +647,7 @@ LocalStorage::put_raw_files(
   const std::vector<core::Result::Serializer::RawFile> raw_files)
 {
   const auto cache_file = look_up_cache_file(key, core::CacheEntryType::result);
-  core::ensure_dir_exists(Util::dir_name(cache_file.path));
+  core::ensure_dir_exists(fs::path(cache_file.path).parent_path());
 
   int64_t files_change = 0;
   int64_t size_kibibyte_change = 0;
@@ -1078,7 +1077,7 @@ LocalStorage::move_to_wanted_cache_level(const StatisticsCounters& counters,
   const auto wanted_path = get_path_in_cache(
     wanted_level, util::format_digest(key) + suffix_from_type(type));
   if (cache_file_path != wanted_path) {
-    core::ensure_dir_exists(Util::dir_name(wanted_path));
+    core::ensure_dir_exists(fs::path(wanted_path).parent_path());
 
     // Note: Two ccache processes may move the file at the same time, so failure
     // to rename is OK.
@@ -1087,7 +1086,7 @@ LocalStorage::move_to_wanted_cache_level(const StatisticsCounters& counters,
     for (const auto& raw_file : m_added_raw_files) {
       fs::rename(raw_file,
                  FMT("{}/{}",
-                     Util::dir_name(wanted_path),
+                     fs::path(wanted_path).parent_path(),
                      fs::path(raw_file).filename()));
     }
   }
@@ -1507,7 +1506,7 @@ std::string
 LocalStorage::get_lock_path(const std::string& name) const
 {
   auto path = FMT("{}/lock/{}", m_config.cache_dir(), name);
-  core::ensure_dir_exists(Util::dir_name(path));
+  core::ensure_dir_exists(fs::path(path).parent_path());
   return path;
 }
 
diff --git a/src/storage/remote/FileStorage.cpp b/src/storage/remote/FileStorage.cpp
index 548d63e5d..5fdf1717b 100644
--- a/src/storage/remote/FileStorage.cpp
+++ b/src/storage/remote/FileStorage.cpp
@@ -153,7 +153,7 @@ FileStorageBackend::put(const Hash::Digest& key,
   {
     util::UmaskScope umask_scope(m_umask);
 
-    const auto dir = Util::dir_name(path);
+    const fs::path dir = fs::path(path).parent_path();
     if (auto result = fs::create_directories(dir); !result) {
       LOG("Failed to create directory {}: {}", dir, result.error().message());
       return tl::unexpected(Failure::error);
diff --git a/unittest/test_Depfile.cpp b/unittest/test_Depfile.cpp
index 8540610c9..a556f82cc 100644
--- a/unittest/test_Depfile.cpp
+++ b/unittest/test_Depfile.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2024 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -20,7 +20,7 @@
 #include "../src/Depfile.hpp"
 #include "TestUtil.hpp"
 
-#include <Util.hpp>
+#include <util/filesystem.hpp>
 #include <util/fmtmacros.hpp>
 
 #include "third_party/doctest.h"
@@ -28,6 +28,8 @@
 #include <string>
 #include <vector>
 
+namespace fs = util::filesystem;
+
 using TestUtil::TestContext;
 
 TEST_SUITE_BEGIN("Depfile");
@@ -47,12 +49,12 @@ TEST_CASE("Depfile::rewrite_source_paths")
 {
   Context ctx;
 
-  const auto cwd = ctx.actual_cwd;
+  const fs::path cwd = ctx.actual_cwd;
 
   const auto content =
     FMT("{0}/foo.o {0}/foo.o: bar.c {0}/bar.h \\\n\n {1}/fie.h {0}/fum.h\n",
         cwd,
-        Util::dir_name(cwd));
+        cwd.parent_path());
 
   SUBCASE("Base directory not in dep file content")
   {
@@ -63,19 +65,19 @@ TEST_CASE("Depfile::rewrite_source_paths")
 
   SUBCASE("Base directory in dep file content but not matching")
   {
-    ctx.config.set_base_dir(FMT("{}/other", Util::dir_name(cwd)));
+    ctx.config.set_base_dir((cwd.parent_path() / "other").string());
     CHECK(!Depfile::rewrite_source_paths(ctx, ""));
     CHECK(!Depfile::rewrite_source_paths(ctx, content));
   }
 
   SUBCASE("Absolute paths under base directory rewritten")
   {
-    ctx.config.set_base_dir(cwd);
+    ctx.config.set_base_dir(cwd.string());
     const auto actual = Depfile::rewrite_source_paths(ctx, content);
     const auto expected =
       FMT("{0}/foo.o {0}/foo.o: bar.c ./bar.h \\\n\n {1}/fie.h ./fum.h\n",
           cwd,
-          Util::dir_name(cwd));
+          cwd.parent_path());
     REQUIRE(actual);
     CHECK(*actual == expected);
   }
diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index dde50d97b..5290a37f9 100644
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -66,26 +66,6 @@ TEST_CASE("Util::common_dir_prefix_length")
   CHECK(Util::common_dir_prefix_length("/a/b", "/a/bc") == 2);
 }
 
-TEST_CASE("Util::dir_name")
-{
-  CHECK(Util::dir_name("") == ".");
-  CHECK(Util::dir_name(".") == ".");
-  CHECK(Util::dir_name("foo") == ".");
-  CHECK(Util::dir_name("/") == "/");
-  CHECK(Util::dir_name("/foo") == "/");
-  CHECK(Util::dir_name("/foo/bar/f.txt") == "/foo/bar");
-
-#ifdef _WIN32
-  CHECK(Util::dir_name("C:/x/y") == "C:/x");
-  CHECK(Util::dir_name("X:/x/y") == "X:/x");
-  CHECK(Util::dir_name("C:\\x\\y") == "C:\\x");
-  CHECK(Util::dir_name("C:/x") == "C:/");
-  CHECK(Util::dir_name("C:\\x") == "C:\\");
-  CHECK(Util::dir_name("C:/") == "C:/");
-  CHECK(Util::dir_name("C:\\") == "C:\\");
-#endif
-}
-
 TEST_CASE("Util::get_relative_path")
 {
 #ifdef _WIN32
@@ -178,7 +158,7 @@ TEST_CASE("Util::make_relative_path")
     CHECK(
       make_relative_path(
         actual_cwd.substr(0, 3), actual_cwd, apparent_cwd, actual_cwd + "\\\\x")
-      == ".\\x");
+      == ".\\\\x");
 #else
     CHECK(make_relative_path("/", actual_cwd, apparent_cwd, actual_cwd + "/x")
           == "./x");