Skip to content

Commit

Permalink
src: revert filesystem::path changes
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Sep 19, 2024
1 parent 291d90a commit 37bd2dc
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 54 deletions.
28 changes: 11 additions & 17 deletions src/compile_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "node_internals.h"
#include "node_version.h"
#include "path.h"
#include "util.h"
#include "zlib.h"

namespace node {
Expand Down Expand Up @@ -219,14 +220,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(result->cache_key);
result->source_filename = filename_utf8.ToString();
result->cache = nullptr;
result->type = type;
Expand Down Expand Up @@ -364,43 +362,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) {
Expand All @@ -410,7 +405,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;
Expand Down
4 changes: 1 addition & 3 deletions src/compile_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <cinttypes>
#include <filesystem>
#include <memory>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -92,8 +91,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_;
};
Expand Down
6 changes: 2 additions & 4 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <atomic>
#include <cinttypes>
#include <cstdio>
#include <filesystem>
#include <iostream>
#include <limits>
#include <memory>
Expand Down Expand Up @@ -730,8 +729,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) {
Expand Down Expand Up @@ -2122,7 +2120,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);

Expand Down
5 changes: 2 additions & 3 deletions src/inspector_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "v8-inspector.h"

#include <cinttypes>
#include <filesystem>
#include <limits>
#include <sstream>
#include "simdutf.h"
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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()) {
Expand Down
18 changes: 12 additions & 6 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,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;
}
Expand Down Expand Up @@ -1741,9 +1747,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()) {
Expand Down Expand Up @@ -1821,9 +1827,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()) {
Expand Down
11 changes: 5 additions & 6 deletions src/node_modules.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "permission/permission.h"
#include "permission/permission_base.h"
#include "util-inl.h"
#include "util.h"
#include "v8-fast-api-calls.h"
#include "v8-function-callback.h"
#include "v8-primitive.h"
Expand Down Expand Up @@ -322,14 +323,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 =
Expand All @@ -349,14 +349,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 =
Expand Down
6 changes: 2 additions & 4 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <cstring>
#include <ctime>
#include <cwctype>
#include <filesystem>
#include <fstream>

constexpr int NODE_REPORT_VERSION = 3;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 13 additions & 5 deletions src/permission/fs_permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions src/permission/fs_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include "v8.h"

#include <filesystem>
#include <unordered_map>
#include "permission/permission_base.h"
#include "util.h"
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@

namespace node {

// Maybe remove kPathSeparator when cpp17 is ready
#ifdef _WIN32
constexpr char kPathSeparator = '\\';
/* MAX_PATH is in characters, not bytes. Make sure we have enough headroom. */
#define PATH_MAX_BYTES (MAX_PATH * 4)
#else
constexpr char kPathSeparator = '/';
#define PATH_MAX_BYTES (PATH_MAX)
#endif

Expand Down

0 comments on commit 37bd2dc

Please sign in to comment.