From f1fc426cce9db230cb83866871f355afa0b92d3b Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 12 Feb 2018 13:02:42 +0200 Subject: module: support main w/o extension, pjson cache This adds support for ensuring that the top-level main into Node is supported loading when it has no extension for backwards-compat with NodeJS bin workflows. In addition package.json caching is implemented in the module lookup process. PR-URL: https://github.com/nodejs/node/pull/18728 Reviewed-By: Benjamin Gruenbaum --- src/env.h | 23 +++++++++- src/module_wrap.cc | 125 +++++++++++++++++++++++++++++++++-------------------- src/module_wrap.h | 7 ++- 3 files changed, 105 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/env.h b/src/env.h index 68b674f4fd..dedf821b24 100644 --- a/src/env.h +++ b/src/env.h @@ -54,7 +54,26 @@ class performance_state; namespace loader { class ModuleWrap; -} + +struct Exists { + enum Bool { Yes, No }; +}; + +struct IsValid { + enum Bool { Yes, No }; +}; + +struct HasMain { + enum Bool { Yes, No }; +}; + +struct PackageConfig { + const Exists::Bool exists; + const IsValid::Bool is_valid; + const HasMain::Bool has_main; + const std::string main; +}; +} // namespace loader // Pick an index that's hopefully out of the way when we're embedded inside // another application. Performance-wise or memory-wise it doesn't matter: @@ -609,6 +628,8 @@ class Environment { std::unordered_multimap module_map; + std::unordered_map package_json_cache; + inline double* heap_statistics_buffer() const; inline void set_heap_statistics_buffer(double* pointer); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4a9c847a6a..3d34da570a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -461,10 +461,9 @@ enum CheckFileOptions { CLOSE_AFTER_CHECK }; -Maybe CheckFile(const URL& search, +Maybe CheckFile(const std::string& path, CheckFileOptions opt = CLOSE_AFTER_CHECK) { uv_fs_t fs_req; - std::string path = search.ToFilePath(); if (path.empty()) { return Nothing(); } @@ -481,19 +480,74 @@ Maybe CheckFile(const URL& search, uv_fs_req_cleanup(&fs_req); if (is_directory) { - uv_fs_close(nullptr, &fs_req, fd, nullptr); + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); uv_fs_req_cleanup(&fs_req); return Nothing(); } if (opt == CLOSE_AFTER_CHECK) { - uv_fs_close(nullptr, &fs_req, fd, nullptr); + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); uv_fs_req_cleanup(&fs_req); } return Just(fd); } +const PackageConfig& GetPackageConfig(Environment* env, + const std::string path) { + auto existing = env->package_json_cache.find(path); + if (existing != env->package_json_cache.end()) { + return existing->second; + } + Maybe check = CheckFile(path, LEAVE_OPEN_AFTER_CHECK); + if (check.IsNothing()) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); + return entry.first->second; + } + + Isolate* isolate = env->isolate(); + v8::HandleScope handle_scope(isolate); + + std::string pkg_src = ReadFile(check.FromJust()); + uv_fs_t fs_req; + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr)); + uv_fs_req_cleanup(&fs_req); + + Local src; + if (!String::NewFromUtf8(isolate, + pkg_src.c_str(), + v8::NewStringType::kNormal, + pkg_src.length()).ToLocal(&src)) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); + return entry.first->second; + } + + Local pkg_json_v; + Local pkg_json; + + if (!JSON::Parse(env->context(), src).ToLocal(&pkg_json_v) || + !pkg_json_v->ToObject(env->context()).ToLocal(&pkg_json)) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "" }); + return entry.first->second; + } + + Local pkg_main; + HasMain::Bool has_main = HasMain::No; + std::string main_std; + if (pkg_json->Get(env->context(), env->main_string()).ToLocal(&pkg_main)) { + has_main = HasMain::Yes; + Utf8Value main_utf8(isolate, pkg_main); + main_std.assign(std::string(*main_utf8, main_utf8.length())); + } + + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::Yes, has_main, "" }); + return entry.first->second; +} + enum ResolveExtensionsOptions { TRY_EXACT_NAME, ONLY_VIA_EXTENSIONS @@ -502,7 +556,8 @@ enum ResolveExtensionsOptions { template Maybe ResolveExtensions(const URL& search) { if (options == TRY_EXACT_NAME) { - Maybe check = CheckFile(search); + std::string filePath = search.ToFilePath(); + Maybe check = CheckFile(filePath); if (!check.IsNothing()) { return Just(search); } @@ -510,7 +565,7 @@ Maybe ResolveExtensions(const URL& search) { for (const char* extension : EXTENSIONS) { URL guess(search.path() + extension, &search); - Maybe check = CheckFile(guess); + Maybe check = CheckFile(guess.ToFilePath()); if (!check.IsNothing()) { return Just(guess); } @@ -525,44 +580,18 @@ inline Maybe ResolveIndex(const URL& search) { Maybe ResolveMain(Environment* env, const URL& search) { URL pkg("package.json", &search); - Maybe check = CheckFile(pkg, LEAVE_OPEN_AFTER_CHECK); - if (check.IsNothing()) { - return Nothing(); - } - - Isolate* isolate = env->isolate(); - Local context = isolate->GetCurrentContext(); - std::string pkg_src = ReadFile(check.FromJust()); - uv_fs_t fs_req; - uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr); - uv_fs_req_cleanup(&fs_req); - - // It's not okay for the called of this method to not be able to tell - // whether an exception is pending or not. - TryCatch try_catch(isolate); - Local src; - if (!String::NewFromUtf8(isolate, - pkg_src.c_str(), - v8::NewStringType::kNormal, - pkg_src.length()).ToLocal(&src)) { + const PackageConfig& pjson = + GetPackageConfig(env, pkg.ToFilePath()); + // Note invalid package.json should throw in resolver + // currently we silently ignore which is incorrect + if (!pjson.exists || !pjson.is_valid || !pjson.has_main) { return Nothing(); } - - Local pkg_json; - if (!JSON::Parse(context, src).ToLocal(&pkg_json) || !pkg_json->IsObject()) - return Nothing(); - Local pkg_main; - if (!pkg_json.As()->Get(context, env->main_string()) - .ToLocal(&pkg_main) || !pkg_main->IsString()) { - return Nothing(); + if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { + return Resolve(env, "./" + pjson.main, search); } - Utf8Value main_utf8(isolate, pkg_main.As()); - std::string main_std(*main_utf8, main_utf8.length()); - if (!ShouldBeTreatedAsRelativeOrAbsolutePath(main_std)) { - main_std.insert(0, "./"); - } - return Resolve(env, main_std, search); + return Resolve(env, pjson.main, search); } Maybe ResolveModule(Environment* env, @@ -572,7 +601,8 @@ Maybe ResolveModule(Environment* env, URL dir(""); do { dir = parent; - Maybe check = Resolve(env, "./node_modules/" + specifier, dir, true); + Maybe check = + Resolve(env, "./node_modules/" + specifier, dir, IgnoreMain); if (!check.IsNothing()) { const size_t limit = specifier.find('/'); const size_t spec_len = @@ -594,8 +624,8 @@ Maybe ResolveModule(Environment* env, Maybe ResolveDirectory(Environment* env, const URL& search, - bool read_pkg_json) { - if (read_pkg_json) { + PackageMainCheck check_pjson_main) { + if (check_pjson_main) { Maybe main = ResolveMain(env, search); if (!main.IsNothing()) return main; @@ -605,15 +635,14 @@ Maybe ResolveDirectory(Environment* env, } // anonymous namespace - Maybe Resolve(Environment* env, const std::string& specifier, const URL& base, - bool read_pkg_json) { + PackageMainCheck check_pjson_main) { URL pure_url(specifier); if (!(pure_url.flags() & URL_FLAGS_FAILED)) { // just check existence, without altering - Maybe check = CheckFile(pure_url); + Maybe check = CheckFile(pure_url.ToFilePath()); if (check.IsNothing()) { return Nothing(); } @@ -630,7 +659,7 @@ Maybe Resolve(Environment* env, if (specifier.back() != '/') { resolved = URL(specifier + "/", base); } - return ResolveDirectory(env, resolved, read_pkg_json); + return ResolveDirectory(env, resolved, check_pjson_main); } else { return ResolveModule(env, specifier, base); } @@ -667,7 +696,7 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { return; } - Maybe result = node::loader::Resolve(env, specifier_std, url, true); + Maybe result = node::loader::Resolve(env, specifier_std, url); if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) { std::string msg = "Cannot find module " + specifier_std; env->ThrowError(msg.c_str()); diff --git a/src/module_wrap.h b/src/module_wrap.h index 5950c5a1be..ee3740b561 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -12,10 +12,15 @@ namespace node { namespace loader { +enum PackageMainCheck : bool { + CheckMain = true, + IgnoreMain = false +}; + v8::Maybe Resolve(Environment* env, const std::string& specifier, const url::URL& base, - bool read_pkg_json = false); + PackageMainCheck read_pkg_json = CheckMain); class ModuleWrap : public BaseObject { public: -- cgit v1.2.3