From: Joel Rosdahl <joel@rosdahl.net>
Date: Thu, 3 Aug 2023 06:44:21 +0000 (+0200)
Subject: refactor: fs::path-ify ignore_header_paths and related functions
X-Git-Tag: v4.9~79
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=587b0244451880c1b93af0e8ff10e8a46d1e167b;p=thirdparty%2Fccache.git

refactor: fs::path-ify ignore_header_paths and related functions
---

diff --git a/src/Context.hpp b/src/Context.hpp
index 7f3535fec..bfc92a423 100644
--- a/src/Context.hpp
+++ b/src/Context.hpp
@@ -86,7 +86,7 @@ public:
   util::Bytes cpp_stderr_data;
 
   // Headers (or directories with headers) to ignore in manifest mode.
-  std::vector<std::string> ignore_header_paths;
+  std::vector<std::filesystem::path> ignore_header_paths;
 
   // Storage (fronting local and remote storage backends).
   storage::Storage storage;
diff --git a/src/Util.cpp b/src/Util.cpp
index bbebbc621..cdd83c031 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -290,18 +290,6 @@ make_relative_path(const Context& ctx, std::string_view path)
     ctx.config.base_dir(), ctx.actual_cwd, ctx.apparent_cwd, path);
 }
 
-bool
-matches_dir_prefix_or_file(std::string_view dir_prefix_or_file,
-                           std::string_view path)
-{
-  return !dir_prefix_or_file.empty() && !path.empty()
-         && dir_prefix_or_file.length() <= path.length()
-         && util::starts_with(path, dir_prefix_or_file)
-         && (dir_prefix_or_file.length() == path.length()
-             || is_dir_separator(path[dir_prefix_or_file.length()])
-             || is_dir_separator(dir_prefix_or_file.back()));
-}
-
 static std::string
 do_normalize_abstract_absolute_path(std::string_view path)
 {
diff --git a/src/Util.hpp b/src/Util.hpp
index 6648cb7d6..6159390f0 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -97,11 +97,6 @@ std::string make_relative_path(const std::string& base_dir,
 // Like above but with base directory and apparent/actual CWD taken from `ctx`.
 std::string make_relative_path(const Context& ctx, std::string_view path);
 
-// Return whether `path` is equal to `dir_prefix_or_file` or if
-// `dir_prefix_or_file` is a directory prefix of `path`.
-bool matches_dir_prefix_or_file(std::string_view dir_prefix_or_file,
-                                std::string_view path);
-
 // Normalize absolute path `path`, not taking symlinks into account.
 //
 // Normalization here means syntactically removing redundant slashes and
diff --git a/src/ccache.cpp b/src/ccache.cpp
index 705878fd2..378a0867c 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -368,7 +368,7 @@ do_remember_include_file(Context& ctx,
   }
 
   for (const auto& ignore_header_path : ctx.ignore_header_paths) {
-    if (Util::matches_dir_prefix_or_file(ignore_header_path, path)) {
+    if (file_path_matches_dir_prefix_or_file(ignore_header_path, path)) {
       return true;
     }
   }
@@ -1540,11 +1540,11 @@ hash_common_info(const Context& ctx,
   }
 
   if (!ctx.config.extra_files_to_hash().empty()) {
-    for (const std::string& path :
+    for (const auto& path :
          util::split_path_list(ctx.config.extra_files_to_hash())) {
       LOG("Hashing extra file {}", path);
       hash.hash_delimiter("extrafile");
-      if (!hash_binary_file(ctx, hash, path)) {
+      if (!hash_binary_file(ctx, hash, path.string())) {
         return tl::unexpected(Statistic::error_hashing_extra_file);
       }
     }
@@ -2677,6 +2677,21 @@ is_ccache_executable(const fs::path& path)
   return util::starts_with(name, "ccache");
 }
 
+bool
+file_path_matches_dir_prefix_or_file(const fs::path& dir_prefix_or_file,
+                                     const fs::path& file_path)
+{
+  DEBUG_ASSERT(!dir_prefix_or_file.empty());
+  DEBUG_ASSERT(!file_path.filename().empty());
+
+  auto end = std::mismatch(dir_prefix_or_file.begin(),
+                           dir_prefix_or_file.end(),
+                           file_path.begin(),
+                           file_path.end())
+               .first;
+  return end == dir_prefix_or_file.end() || end->empty();
+}
+
 int
 ccache_main(int argc, const char* const* argv)
 {
diff --git a/src/ccache.hpp b/src/ccache.hpp
index 7fe12359a..c085c02db 100644
--- a/src/ccache.hpp
+++ b/src/ccache.hpp
@@ -52,5 +52,11 @@ ArgvParts split_argv(int argc, const char* const* argv);
 void find_compiler(Context& ctx,
                    const FindExecutableFunction& find_executable_function,
                    bool masquerading_as_compiler);
+
 CompilerType guess_compiler(std::string_view path);
+
 bool is_ccache_executable(const std::filesystem::path& path);
+
+bool file_path_matches_dir_prefix_or_file(
+  const std::filesystem::path& dir_prefix_or_file,
+  const std::filesystem::path& file_path);
diff --git a/src/execute.cpp b/src/execute.cpp
index 1c7164692..8e04eddce 100644
--- a/src/execute.cpp
+++ b/src/execute.cpp
@@ -33,6 +33,7 @@
 #include <fmtmacros.hpp>
 #include <util/DirEntry.hpp>
 #include <util/file.hpp>
+#include <util/filesystem.hpp>
 #include <util/path.hpp>
 #include <util/string.hpp>
 #include <util/wincompat.hpp>
@@ -51,6 +52,8 @@
 #  include "Finalizer.hpp"
 #endif
 
+namespace fs = util::filesystem;
+
 #ifdef _WIN32
 static int win32execute(const char* path,
                         const char* const* argv,
@@ -84,7 +87,7 @@ win32getshell(const std::string& path)
   const char* path_list = getenv("PATH");
   std::string sh;
   if (util::to_lowercase(Util::get_extension(path)) == ".sh" && path_list) {
-    sh = find_executable_in_path("sh.exe", path_list);
+    sh = find_executable_in_path("sh.exe", path_list).string();
   }
   if (sh.empty() && getenv("CCACHE_DETECT_SHEBANG")) {
     // Detect shebang.
@@ -93,7 +96,7 @@ win32getshell(const std::string& path)
       char buf[10] = {0};
       fgets(buf, sizeof(buf) - 1, fp.get());
       if (std::string(buf) == "#!/bin/sh" && path_list) {
-        sh = find_executable_in_path("sh.exe", path_list);
+        sh = find_executable_in_path("sh.exe", path_list).string();
       }
     }
   }
@@ -356,28 +359,28 @@ find_executable(const Context& ctx,
     return {};
   }
 
-  return find_executable_in_path(name, path_list, exclude_path);
+  return find_executable_in_path(name, path_list, exclude_path).string();
 }
 
-std::string
+fs::path
 find_executable_in_path(const std::string& name,
                         const std::string& path_list,
-                        std::optional<std::string> exclude_path)
+                        const std::optional<fs::path>& exclude_path)
 {
   if (path_list.empty()) {
     return {};
   }
 
-  const auto real_exclude_path =
-    exclude_path ? util::real_path(*exclude_path) : "";
+  auto real_exclude_path =
+    exclude_path ? fs::canonical(*exclude_path).value_or("") : "";
 
   // Search the path list looking for the first compiler of the right name that
   // isn't us.
-  for (const std::string& dir : util::split_path_list(path_list)) {
-    const std::vector<std::string> candidates = {
-      FMT("{}/{}", dir, name),
+  for (const auto& dir : util::split_path_list(path_list)) {
+    const std::vector<fs::path> candidates = {
+      dir / name,
 #ifdef _WIN32
-      FMT("{}/{}.exe", dir, name),
+      dir / FMT("{}.exe", name),
 #endif
     };
     for (const auto& candidate : candidates) {
@@ -398,9 +401,9 @@ find_executable_in_path(const std::string& name,
         access(candidate.c_str(), X_OK) == 0;
 #endif
       if (candidate_exists) {
-        const auto real_candidate = util::real_path(candidate);
-        if ((real_exclude_path.empty() || real_candidate != real_exclude_path)
-            && !is_ccache_executable(real_candidate)) {
+        auto real_candidate = fs::canonical(candidate);
+        if (real_candidate && *real_candidate != real_exclude_path
+            && !is_ccache_executable(*real_candidate)) {
           return candidate;
         }
       }
diff --git a/src/execute.hpp b/src/execute.hpp
index c44a465ab..5d0527dfb 100644
--- a/src/execute.hpp
+++ b/src/execute.hpp
@@ -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.
 //
@@ -20,6 +20,7 @@
 
 #include "Fd.hpp"
 
+#include <filesystem>
 #include <optional>
 #include <string>
 
@@ -35,10 +36,10 @@ std::string find_executable(const Context& ctx,
                             const std::string& name,
                             const std::string& exclude_path);
 
-std::string
-find_executable_in_path(const std::string& name,
-                        const std::string& path_list,
-                        std::optional<std::string> exclude_path = std::nullopt);
+std::filesystem::path find_executable_in_path(
+  const std::string& name,
+  const std::string& path_list,
+  const std::optional<std::filesystem::path>& exclude_path = std::nullopt);
 
 #ifdef _WIN32
 std::string win32getshell(const std::string& path);
diff --git a/src/hashutil.cpp b/src/hashutil.cpp
index 0cd4265c7..fc43e8764 100644
--- a/src/hashutil.cpp
+++ b/src/hashutil.cpp
@@ -382,7 +382,7 @@ hash_command_output(Hash& hash,
   STARTUPINFO si;
   memset(&si, 0x00, sizeof(si));
 
-  std::string path = find_executable_in_path(args[0], getenv("PATH"));
+  auto path = find_executable_in_path(args[0], getenv("PATH")).string();
   if (path.empty()) {
     path = args[0];
   }
diff --git a/src/util/path.cpp b/src/util/path.cpp
index 019d39656..c9891d6e7 100644
--- a/src/util/path.cpp
+++ b/src/util/path.cpp
@@ -26,10 +26,8 @@
 
 #ifdef _WIN32
 const char k_dev_null_path[] = "nul:";
-const char k_path_delimiter[] = ";";
 #else
 const char k_dev_null_path[] = "/dev/null";
-const char k_path_delimiter[] = ":";
 #endif
 
 namespace fs = util::filesystem;
@@ -130,12 +128,6 @@ real_path(std::string_view path)
   return real_path ? real_path->string() : std::string(path);
 }
 
-std::vector<std::string>
-split_path_list(std::string_view path_list)
-{
-  return split_into_strings(path_list, k_path_delimiter);
-}
-
 std::string
 to_absolute_path(std::string_view path)
 {
diff --git a/src/util/path.hpp b/src/util/path.hpp
index ccd92a2c0..02b790e47 100644
--- a/src/util/path.hpp
+++ b/src/util/path.hpp
@@ -57,10 +57,6 @@ bool path_starts_with(std::string_view path, std::string_view prefix);
 // doesn't exist) path is returned unmodified.
 std::string real_path(std::string_view path);
 
-// Split a list of paths (such as the content of $PATH on Unix platforms or
-// %PATH% on Windows platforms) into paths.
-std::vector<std::string> split_path_list(std::string_view path_list);
-
 // Make `path` an absolute path.
 std::string to_absolute_path(std::string_view path);
 
diff --git a/src/util/string.cpp b/src/util/string.cpp
index a154d0938..cf621e83d 100644
--- a/src/util/string.cpp
+++ b/src/util/string.cpp
@@ -429,6 +429,20 @@ split_once(const std::string_view string, const char split_char)
   }
 }
 
+std::vector<std::filesystem::path>
+split_path_list(std::string_view path_list)
+{
+#ifdef _WIN32
+  const char path_delimiter[] = ";";
+#else
+  const char path_delimiter[] = ":";
+#endif
+  auto strings = split_into_views(path_list, path_delimiter);
+  std::vector<std::filesystem::path> paths;
+  std::copy(strings.cbegin(), strings.cend(), std::back_inserter(paths));
+  return paths;
+}
+
 std::string
 strip_whitespace(const std::string_view string)
 {
diff --git a/src/util/string.hpp b/src/util/string.hpp
index c3c207a82..3b263651a 100644
--- a/src/util/string.hpp
+++ b/src/util/string.hpp
@@ -29,6 +29,7 @@
 
 #include <cstdint>
 #include <cstring>
+#include <filesystem>
 #include <optional>
 #include <string>
 #include <string_view>
@@ -171,6 +172,10 @@ split_once(std::string&& string, char split_char);
 std::pair<std::string_view, std::optional<std::string_view>>
 split_once(std::string_view string, char split_char);
 
+// Split a list of paths (such as the content of $PATH on Unix platforms or
+// %PATH% on Windows platforms) into paths.
+std::vector<std::filesystem::path> split_path_list(std::string_view path_list);
+
 // Return true if `prefix` is a prefix of `string`.
 bool starts_with(const char* string, std::string_view prefix);
 
diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index 52f182ad0..f0c355bb1 100644
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -252,35 +252,6 @@ TEST_CASE("Util::make_relative_path")
 #endif
 }
 
-TEST_CASE("Util::matches_dir_prefix_or_file")
-{
-  CHECK(!Util::matches_dir_prefix_or_file("", ""));
-  CHECK(!Util::matches_dir_prefix_or_file("/", ""));
-  CHECK(!Util::matches_dir_prefix_or_file("", "/"));
-
-  CHECK(Util::matches_dir_prefix_or_file("aa", "aa"));
-  CHECK(!Util::matches_dir_prefix_or_file("aaa", "aa"));
-  CHECK(!Util::matches_dir_prefix_or_file("aa", "aaa"));
-  CHECK(!Util::matches_dir_prefix_or_file("aa/", "aa"));
-
-  CHECK(Util::matches_dir_prefix_or_file("/aa/bb", "/aa/bb"));
-  CHECK(!Util::matches_dir_prefix_or_file("/aa/b", "/aa/bb"));
-  CHECK(!Util::matches_dir_prefix_or_file("/aa/bbb", "/aa/bb"));
-
-  CHECK(Util::matches_dir_prefix_or_file("/aa", "/aa/bb"));
-  CHECK(Util::matches_dir_prefix_or_file("/aa/", "/aa/bb"));
-  CHECK(!Util::matches_dir_prefix_or_file("/aa/bb", "/aa"));
-  CHECK(!Util::matches_dir_prefix_or_file("/aa/bb", "/aa/"));
-
-#ifdef _WIN32
-  CHECK(Util::matches_dir_prefix_or_file("\\aa", "\\aa\\bb"));
-  CHECK(Util::matches_dir_prefix_or_file("\\aa\\", "\\aa\\bb"));
-#else
-  CHECK(!Util::matches_dir_prefix_or_file("\\aa", "\\aa\\bb"));
-  CHECK(!Util::matches_dir_prefix_or_file("\\aa\\", "\\aa\\bb"));
-#endif
-}
-
 TEST_CASE("Util::normalize_abstract_absolute_path")
 {
   CHECK(Util::normalize_abstract_absolute_path("") == "");
diff --git a/unittest/test_ccache.cpp b/unittest/test_ccache.cpp
index 9e098bfbd..e0172a568 100644
--- a/unittest/test_ccache.cpp
+++ b/unittest/test_ccache.cpp
@@ -219,4 +219,28 @@ TEST_CASE("is_ccache_executable")
 #endif
 }
 
+TEST_CASE("file_path_matches_dir_prefix_or_file")
+{
+  CHECK(file_path_matches_dir_prefix_or_file("aa", "aa"));
+  CHECK(!file_path_matches_dir_prefix_or_file("aaa", "aa"));
+  CHECK(!file_path_matches_dir_prefix_or_file("aa", "aaa"));
+  CHECK(file_path_matches_dir_prefix_or_file("aa/", "aa"));
+
+  CHECK(file_path_matches_dir_prefix_or_file("/aa/bb", "/aa/bb"));
+  CHECK(!file_path_matches_dir_prefix_or_file("/aa/b", "/aa/bb"));
+  CHECK(!file_path_matches_dir_prefix_or_file("/aa/bbb", "/aa/bb"));
+
+  CHECK(file_path_matches_dir_prefix_or_file("/aa", "/aa/bb"));
+  CHECK(file_path_matches_dir_prefix_or_file("/aa/", "/aa/bb"));
+  CHECK(!file_path_matches_dir_prefix_or_file("/aa/bb", "/aa"));
+
+#ifdef _WIN32
+  CHECK(file_path_matches_dir_prefix_or_file("\\aa", "\\aa\\bb"));
+  CHECK(file_path_matches_dir_prefix_or_file("\\aa\\", "\\aa\\bb"));
+#else
+  CHECK(!file_path_matches_dir_prefix_or_file("\\aa", "\\aa\\bb"));
+  CHECK(!file_path_matches_dir_prefix_or_file("\\aa\\", "\\aa\\bb"));
+#endif
+}
+
 TEST_SUITE_END();
diff --git a/unittest/test_util_path.cpp b/unittest/test_util_path.cpp
index e7643019b..f781ff4e2 100644
--- a/unittest/test_util_path.cpp
+++ b/unittest/test_util_path.cpp
@@ -64,31 +64,6 @@ TEST_CASE("util::is_dev_null_path")
 #endif
 }
 
-TEST_CASE("util::split_path_list")
-{
-  CHECK(util::split_path_list("").empty());
-  {
-    const auto v = util::split_path_list("a");
-    REQUIRE(v.size() == 1);
-    CHECK(v[0] == "a");
-  }
-  {
-    const auto v = util::split_path_list("a/b");
-    REQUIRE(v.size() == 1);
-    CHECK(v[0] == "a/b");
-  }
-  {
-#ifdef _WIN32
-    const auto v = util::split_path_list("a/b;c");
-#else
-    const auto v = util::split_path_list("a/b:c");
-#endif
-    REQUIRE(v.size() == 2);
-    CHECK(v[0] == "a/b");
-    CHECK(v[1] == "c");
-  }
-}
-
 TEST_CASE("util::to_absolute_path")
 {
   CHECK(util::to_absolute_path("/foo/bar") == "/foo/bar");
diff --git a/unittest/test_util_string.cpp b/unittest/test_util_string.cpp
index a36a77302..8410c8228 100644
--- a/unittest/test_util_string.cpp
+++ b/unittest/test_util_string.cpp
@@ -537,6 +537,31 @@ TEST_CASE("util::split_once")
   }
 }
 
+TEST_CASE("util::split_path_list")
+{
+  CHECK(util::split_path_list("").empty());
+  {
+    const auto v = util::split_path_list("a");
+    REQUIRE(v.size() == 1);
+    CHECK(v[0] == "a");
+  }
+  {
+    const auto v = util::split_path_list("a/b");
+    REQUIRE(v.size() == 1);
+    CHECK(v[0] == "a/b");
+  }
+  {
+#ifdef _WIN32
+    const auto v = util::split_path_list("a/b;c");
+#else
+    const auto v = util::split_path_list("a/b:c");
+#endif
+    REQUIRE(v.size() == 2);
+    CHECK(v[0] == "a/b");
+    CHECK(v[1] == "c");
+  }
+}
+
 TEST_CASE("util::starts_with")
 {
   // starts_with(const char*, string_view)