src: revert filesystem::path changes

anonrig · targos · commit 9ae80e1e4d9e · 2024-10-04T10:20:17.000+02:00
PR-URL: #55015 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
diff --git a/src/compile_cache.cc b/src/compile_cache.cc
@@ -6,6 +6,7 @@
 #include "node_internals.h"
 #include "node_version.h"
 #include "path.h"
+#include "util.h"
 #include "zlib.h"
 
 #ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
@@ -225,14 +226,11 @@ CompileCacheEntry* CompileCacheHandler::GetOrInsert(
       compiler_cache_store_.emplace(key, std::make_unique<CompileCacheEntry>());
   auto* result = emplaced.first->second.get();
 
-  std::u8string cache_filename_u8 =
-      (compile_cache_dir_ / Uint32ToHex(key)).u8string();
   result->code_hash = code_hash;
   result->code_size = code_utf8.length();
   result->cache_key = key;
   result->cache_filename =
-      std::string(cache_filename_u8.begin(), cache_filename_u8.end()) +
-      ".cache";
+      compile_cache_dir_ + kPathSeparator + Uint32ToHex(key);
   result->source_filename = filename_utf8.ToString();
   result->cache = nullptr;
   result->type = type;
@@ -452,43 +450,40 @@ CompileCacheEnableResult CompileCacheHandler::Enable(Environment* env,
                                                      const std::string& dir) {
   std::string cache_tag = GetCacheVersionTag();
   std::string absolute_cache_dir_base = PathResolve(env, {dir});
-  std::filesystem::path cache_dir_with_tag =
-      std::filesystem::path(absolute_cache_dir_base) / cache_tag;
-  std::u8string cache_dir_with_tag_u8 = cache_dir_with_tag.u8string();
-  std::string cache_dir_with_tag_str(cache_dir_with_tag_u8.begin(),
-                                     cache_dir_with_tag_u8.end());
+  std::string cache_dir_with_tag =
+      absolute_cache_dir_base + kPathSeparator + cache_tag;
   CompileCacheEnableResult result;
   Debug("[compile cache] resolved path %s + %s -> %s\n",
         dir,
         cache_tag,
-        cache_dir_with_tag_str);
+        cache_dir_with_tag);
 
   if (UNLIKELY(!env->permission()->is_granted(
           env,
           permission::PermissionScope::kFileSystemWrite,
-          cache_dir_with_tag_str))) {
+          cache_dir_with_tag))) {
     result.message = "Skipping compile cache because write permission for " +
-                     cache_dir_with_tag_str + " is not granted";
+                     cache_dir_with_tag + " is not granted";
     result.status = CompileCacheEnableStatus::FAILED;
     return result;
   }
 
   if (UNLIKELY(!env->permission()->is_granted(
           env,
           permission::PermissionScope::kFileSystemRead,
-          cache_dir_with_tag_str))) {
+          cache_dir_with_tag))) {
     result.message = "Skipping compile cache because read permission for " +
-                     cache_dir_with_tag_str + " is not granted";
+                     cache_dir_with_tag + " is not granted";
     result.status = CompileCacheEnableStatus::FAILED;
     return result;
   }
 
   fs::FSReqWrapSync req_wrap;
   int err = fs::MKDirpSync(
-      nullptr, &(req_wrap.req), cache_dir_with_tag_str, 0777, nullptr);
+      nullptr, &(req_wrap.req), cache_dir_with_tag, 0777, nullptr);
   if (is_debug_) {
     Debug("[compile cache] creating cache directory %s...%s\n",
-          cache_dir_with_tag_str,
+          cache_dir_with_tag,
           err < 0 ? uv_strerror(err) : "success");
   }
   if (err != 0 && err != UV_EEXIST) {
@@ -498,7 +493,6 @@ CompileCacheEnableResult CompileCacheHandler::Enable(Environment* env,
     return result;
   }
 
-  compile_cache_dir_str_ = absolute_cache_dir_base;
   result.cache_directory = absolute_cache_dir_base;
   compile_cache_dir_ = cache_dir_with_tag;
   result.status = CompileCacheEnableStatus::ENABLED;
diff --git a/src/compile_cache.h b/src/compile_cache.h
@@ -4,7 +4,6 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <cinttypes>
-#include <filesystem>
 #include <memory>
 #include <string>
 #include <string_view>
@@ -71,7 +70,7 @@ class CompileCacheHandler {
   void MaybeSave(CompileCacheEntry* entry,
                  v8::Local<v8::Module> mod,
                  bool rejected);
-  std::string_view cache_dir() { return compile_cache_dir_str_; }
+  std::string_view cache_dir() { return compile_cache_dir_; }
 
  private:
   void ReadCacheFile(CompileCacheEntry* entry);
@@ -94,8 +93,7 @@ class CompileCacheHandler {
   v8::Isolate* isolate_ = nullptr;
   bool is_debug_ = false;
 
-  std::string compile_cache_dir_str_;
-  std::filesystem::path compile_cache_dir_;
+  std::string compile_cache_dir_;
   std::unordered_map<uint32_t, std::unique_ptr<CompileCacheEntry>>
       compiler_cache_store_;
 };
diff --git a/src/env.cc b/src/env.cc
@@ -28,7 +28,6 @@
 #include <atomic>
 #include <cinttypes>
 #include <cstdio>
-#include <filesystem>
 #include <iostream>
 #include <limits>
 #include <memory>
@@ -741,8 +740,7 @@ std::string Environment::GetCwd(const std::string& exec_path) {
 
   // This can fail if the cwd is deleted. In that case, fall back to
   // exec_path.
-  return exec_path.substr(
-      0, exec_path.find_last_of(std::filesystem::path::preferred_separator));
+  return exec_path.substr(0, exec_path.find_last_of(kPathSeparator));
 }
 
 void Environment::add_refs(int64_t diff) {
@@ -2130,7 +2128,7 @@ size_t Environment::NearHeapLimitCallback(void* data,
     dir = Environment::GetCwd(env->exec_path_);
   }
   DiagnosticFilename name(env, "Heap", "heapsnapshot");
-  std::string filename = (std::filesystem::path(dir) / (*name)).string();
+  std::string filename = dir + kPathSeparator + (*name);
 
   Debug(env, DebugCategory::DIAGNOSTICS, "Start generating %s...\n", *name);
 
diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
@@ -11,7 +11,6 @@
 #include "v8-inspector.h"
 
 #include <cinttypes>
-#include <filesystem>
 #include <limits>
 #include <sstream>
 #include "simdutf.h"
@@ -249,7 +248,7 @@ void V8ProfilerConnection::WriteProfile(simdjson::ondemand::object* result) {
 
   std::string filename = GetFilename();
   DCHECK(!filename.empty());
-  std::string path = (std::filesystem::path(directory) / filename).string();
+  std::string path = directory + kPathSeparator + filename;
 
   WriteResult(env_, path.c_str(), profile);
 }
@@ -305,7 +304,7 @@ void V8CoverageConnection::WriteProfile(simdjson::ondemand::object* result) {
 
   std::string filename = GetFilename();
   DCHECK(!filename.empty());
-  std::string path = (std::filesystem::path(directory) / filename).string();
+  std::string path = directory + kPathSeparator + filename;
 
   // Only insert source map cache when there's source map data at all.
   if (!source_map_cache_v->IsUndefined()) {
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -82,6 +82,12 @@ using v8::Value;
 # define S_ISDIR(mode)  (((mode) & S_IFMT) == S_IFDIR)
 #endif
 
+#ifdef __POSIX__
+constexpr char kPathSeparator = '/';
+#else
+const char* const kPathSeparator = "\\/";
+#endif
+
 inline int64_t GetOffset(Local<Value> value) {
   return IsSafeJsInt(value) ? value.As<Integer>()->Value() : -1;
 }
@@ -1646,9 +1652,9 @@ int MKDirpSync(uv_loop_t* loop,
           return err;
         }
         case UV_ENOENT: {
-          auto filesystem_path = std::filesystem::path(next_path);
-          if (filesystem_path.has_parent_path()) {
-            std::string dirname = filesystem_path.parent_path().string();
+          std::string dirname =
+              next_path.substr(0, next_path.find_last_of(kPathSeparator));
+          if (dirname != next_path) {
             req_wrap->continuation_data()->PushPath(std::move(next_path));
             req_wrap->continuation_data()->PushPath(std::move(dirname));
           } else if (req_wrap->continuation_data()->paths().empty()) {
@@ -1726,9 +1732,9 @@ int MKDirpAsync(uv_loop_t* loop,
           break;
         }
         case UV_ENOENT: {
-          auto filesystem_path = std::filesystem::path(path);
-          if (filesystem_path.has_parent_path()) {
-            std::string dirname = filesystem_path.parent_path().string();
+          std::string dirname =
+              path.substr(0, path.find_last_of(kPathSeparator));
+          if (dirname != path) {
             req_wrap->continuation_data()->PushPath(path);
             req_wrap->continuation_data()->PushPath(std::move(dirname));
           } else if (req_wrap->continuation_data()->paths().empty()) {
diff --git a/src/node_modules.cc b/src/node_modules.cc
@@ -322,14 +322,13 @@ void BindingData::GetNearestParentPackageJSON(
   BufferValue path_value(realm->isolate(), args[0]);
   // Check if the path has a trailing slash. If so, add it after
   // ToNamespacedPath() as it will be deleted by ToNamespacedPath()
-  bool slashCheck = path_value.ToStringView().ends_with(
-      std::filesystem::path::preferred_separator);
+  bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
 
   ToNamespacedPath(realm->env(), &path_value);
 
   std::string path_value_str = path_value.ToString();
   if (slashCheck) {
-    path_value_str.push_back(std::filesystem::path::preferred_separator);
+    path_value_str.push_back(kPathSeparator);
   }
 
   auto package_json =
@@ -349,14 +348,13 @@ void BindingData::GetNearestParentPackageJSONType(
   BufferValue path_value(realm->isolate(), args[0]);
   // Check if the path has a trailing slash. If so, add it after
   // ToNamespacedPath() as it will be deleted by ToNamespacedPath()
-  bool slashCheck = path_value.ToStringView().ends_with(
-      std::filesystem::path::preferred_separator);
+  bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
 
   ToNamespacedPath(realm->env(), &path_value);
 
   std::string path_value_str = path_value.ToString();
   if (slashCheck) {
-    path_value_str.push_back(std::filesystem::path::preferred_separator);
+    path_value_str.push_back(kPathSeparator);
   }
 
   auto package_json =
diff --git a/src/node_report.cc b/src/node_report.cc
@@ -21,7 +21,6 @@
 #include <cstring>
 #include <ctime>
 #include <cwctype>
-#include <filesystem>
 #include <fstream>
 
 constexpr int NODE_REPORT_VERSION = 3;
@@ -887,9 +886,8 @@ std::string TriggerNodeReport(Isolate* isolate,
       report_directory = per_process::cli_options->report_directory;
     }
     // Regular file. Append filename to directory path if one was specified
-    if (!report_directory.empty()) {
-      std::string pathname =
-          (std::filesystem::path(report_directory) / filename).string();
+    if (report_directory.length() > 0) {
+      std::string pathname = report_directory + kPathSeparator + filename;
       outfile.open(pathname, std::ios::out | std::ios::binary);
     } else {
       outfile.open(filename, std::ios::out | std::ios::binary);
diff --git a/src/path.cc b/src/path.cc
@@ -7,11 +7,11 @@
 namespace node {
 
 #ifdef _WIN32
-constexpr bool IsPathSeparator(char c) noexcept {
+constexpr bool IsPathSeparator(const char c) noexcept {
   return c == '\\' || c == '/';
 }
 #else   // POSIX
-constexpr bool IsPathSeparator(char c) noexcept {
+constexpr bool IsPathSeparator(const char c) noexcept {
   return c == '/';
 }
 #endif  // _WIN32
diff --git a/src/path.h b/src/path.h
@@ -10,9 +10,8 @@
 
 namespace node {
 
-class Environment;
+constexpr bool IsPathSeparator(const char c) noexcept;
 
-constexpr bool IsPathSeparator(char c) noexcept;
 std::string NormalizeString(const std::string_view path,
                             bool allowAboveRoot,
                             const std::string_view separator);
diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc
@@ -17,12 +17,20 @@
 namespace {
 
 std::string WildcardIfDir(const std::string& res) noexcept {
-  auto path = std::filesystem::path(res);
-  auto file_status = std::filesystem::status(path);
-  if (file_status.type() == std::filesystem::file_type::directory) {
-    path /= "*";
+  uv_fs_t req;
+  int rc = uv_fs_stat(nullptr, &req, res.c_str(), nullptr);
+  if (rc == 0) {
+    const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
+    if ((s->st_mode & S_IFMT) == S_IFDIR) {
+      // add wildcard when directory
+      if (res.back() == node::kPathSeparator) {
+        return res + "*";
+      }
+      return res + node::kPathSeparator + "*";
+    }
   }
-  return path.string();
+  uv_fs_req_cleanup(&req);
+  return res;
 }
 
 void FreeRecursivelyNode(
diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h
@@ -5,7 +5,6 @@
 
 #include "v8.h"
 
-#include <filesystem>
 #include <unordered_map>
 #include "permission/permission_base.h"
 #include "util.h"
@@ -107,7 +106,7 @@ class FSPermission final : public PermissionBase {
           // path = /home/subdirectory
           // child = subdirectory/*
           if (idx >= path.length() &&
-              child->prefix[i] == std::filesystem::path::preferred_separator) {
+              child->prefix[i] == node::kPathSeparator) {
             continue;
           }
 
diff --git a/src/util.h b/src/util.h
@@ -38,6 +38,7 @@
 
 #include <array>
 #include <bit>
+#include <filesystem>
 #include <limits>
 #include <memory>
 #include <optional>
@@ -57,6 +58,8 @@
 
 namespace node {
 
+constexpr char kPathSeparator = std::filesystem::path::preferred_separator;
+
 #ifdef _WIN32
 /* MAX_PATH is in characters, not bytes. Make sure we have enough headroom. */
 #define PATH_MAX_BYTES (MAX_PATH * 4)

Original file line number	Diff line number	Diff line change
`@@ -7,11 +7,11 @@`
`7`	`7`	`namespace node {`
`8`	`8`
`9`	`9`	`#ifdef _WIN32`
`10`		`-constexpr bool IsPathSeparator(char c) noexcept {`
	`10`	`+constexpr bool IsPathSeparator(const char c) noexcept {`
`11`	`11`	`return c == '\\' \|\| c == '/';`
`12`	`12`	`}`
`13`	`13`	`#else // POSIX`
`14`		`-constexpr bool IsPathSeparator(char c) noexcept {`
	`14`	`+constexpr bool IsPathSeparator(const char c) noexcept {`
`15`	`15`	`return c == '/';`
`16`	`16`	`}`
`17`	`17`	`#endif // _WIN32`