From 9ae80e1e4d9e7fdc33217e7316dd519b4adce05b Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sat, 21 Sep 2024 13:22:21 -0400 Subject: [PATCH] src: revert filesystem::path changes PR-URL: https://github.com/nodejs/node/pull/55015 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Joyee Cheung --- src/compile_cache.cc | 28 +++++++++++----------------- src/compile_cache.h | 6 ++---- src/env.cc | 6 ++---- src/inspector_profiler.cc | 5 ++--- src/node_file.cc | 18 ++++++++++++------ src/node_modules.cc | 10 ++++------ src/node_report.cc | 6 ++---- src/path.cc | 4 ++-- src/path.h | 3 +-- src/permission/fs_permission.cc | 18 +++++++++++++----- src/permission/fs_permission.h | 3 +-- src/util.h | 3 +++ 12 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/compile_cache.cc b/src/compile_cache.cc index d4ae86669712b8..1c5d64e434d11d 100644 --- 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()); 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,23 +450,20 @@ 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; } @@ -476,19 +471,19 @@ CompileCacheEnableResult CompileCacheHandler::Enable(Environment* env, 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 index fe8c0b93cb3ef3..a7bb58c4a0be95 100644 --- a/src/compile_cache.h +++ b/src/compile_cache.h @@ -4,7 +4,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include -#include #include #include #include @@ -71,7 +70,7 @@ class CompileCacheHandler { void MaybeSave(CompileCacheEntry* entry, v8::Local 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> compiler_cache_store_; }; diff --git a/src/env.cc b/src/env.cc index 453b87b05e9c79..588534518ec0b1 100644 --- a/src/env.cc +++ b/src/env.cc @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -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 index c7554e424be327..f09dd04ccd7f6e 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -11,7 +11,6 @@ #include "v8-inspector.h" #include -#include #include #include #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 index 54870db288480c..5750903d1054f8 100644 --- 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) { return IsSafeJsInt(value) ? value.As()->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 index 821eaa08741c11..157fa8d4442843 100644 --- 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 index 69b8646d40f3f0..cffb9dd1b655af 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -21,7 +21,6 @@ #include #include #include -#include #include 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 index fade21c8af9414..4068e1d892d6b7 100644 --- 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 index 79252dae4d2b87..2045e7b44a9bb1 100644 --- 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 index 8b79850d156feb..757b42b23a2d71 100644 --- 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(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 index 313a1344c6fed5..22b29b017e2061 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -5,7 +5,6 @@ #include "v8.h" -#include #include #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 index cbfe20c811901e..792108317ea594 100644 --- a/src/util.h +++ b/src/util.h @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -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)