From 8a799d966fffffb8d56ad1fa0a956607eb947398 Mon Sep 17 00:00:00 2001 From: David Goldstein Date: Tue, 10 Apr 2018 07:40:56 +0000 Subject: [PATCH 1/6] Add a --preserve-symlinks-main option which behaves like --preserve-symlinks but for require.main --- lib/internal/modules/cjs/loader.js | 17 ++++++++++++++++- lib/internal/modules/esm/default_resolve.js | 3 ++- src/node.cc | 16 +++++++++++++++- src/node_config.cc | 2 ++ src/node_internals.h | 5 +++++ 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 652378ad5782fb..783592e1dd6f26 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -42,6 +42,7 @@ const { stripShebang } = require('internal/modules/cjs/helpers'); const preserveSymlinks = !!process.binding('config').preserveSymlinks; +const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain; const experimentalModules = !!process.binding('config').experimentalModules; const { @@ -240,7 +241,21 @@ Module._findPath = function(request, paths, isMain) { var rc = stat(basePath); if (!trailingSlash) { if (rc === 0) { // File. - if (preserveSymlinks && !isMain) { + if (!isMain) { + if (preserveSymlinks) { + filename = path.resolve(basePath); + } else { + filename = toRealPath(basePath); + } + } else if (preserveSymlinksMain) { + // for the main module, we use the preserveSymlinksMain flag instead + // mainly for backwards compatibility, as the preserveSymlinks flag + // historically has not applied to the main module. Most likely this + // was intended to keep .bin/ binaries working, as following those + // symlinks is usually required for the imports in the corresponding + // files to resolve; that said, in some use cases following symlinks + // causes bigger problems which is why the preserveSymlinksMain option + // is needed. filename = path.resolve(basePath); } else { filename = toRealPath(basePath); diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 60516535e9ad03..bb015672f9f87e 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -7,6 +7,7 @@ const { NativeModule, internalBinding } = require('internal/bootstrap/loaders'); const { extname } = require('path'); const { realpathSync } = require('fs'); const preserveSymlinks = !!process.binding('config').preserveSymlinks; +const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain; const { ERR_MISSING_MODULE, ERR_MODULE_RESOLUTION_LEGACY, @@ -71,7 +72,7 @@ function resolve(specifier, parentURL) { const isMain = parentURL === undefined; - if (!preserveSymlinks || isMain) { + if (isMain ? !preserveSymlinksMain : !preserveSymlinks) { const real = realpathSync(getPathFromURL(url), { [internalFS.realpathCacheKey]: realpathCache }); diff --git a/src/node.cc b/src/node.cc index 669155a908e2eb..82663ef06afca3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -234,6 +234,11 @@ bool trace_warnings = false; // that is used by lib/module.js bool config_preserve_symlinks = false; +// Set in node.cc by ParseArgs when --preserve-symlinks-main is used. +// Used in node_config.cc to set a constant on process.binding('config') +// that is used by lib/module.js +bool config_preserve_symlinks_main = false; + // Set in node.cc by ParseArgs when --experimental-modules is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/module.js @@ -3490,6 +3495,8 @@ static void PrintHelp() { " --pending-deprecation emit pending deprecation warnings\n" #if defined(NODE_HAVE_I18N_SUPPORT) " --preserve-symlinks preserve symbolic links when resolving\n" + " --preserve-symlinks-main preserve symbolic links when resolving " + "the main module\n" #endif " --prof-process process v8 profiler output generated\n" " using --prof\n" @@ -3540,7 +3547,6 @@ static void PrintHelp() { " -r, --require module to preload (option can be " "repeated)\n" " -v, --version print Node.js version\n" - "\n" "Environment variables:\n" "NODE_DEBUG ','-separated list of core modules\n" @@ -3803,6 +3809,8 @@ static void ParseArgs(int* argc, Revert(cve); } else if (strcmp(arg, "--preserve-symlinks") == 0) { config_preserve_symlinks = true; + } else if (strcmp(arg, "--preserve-symlinks-main") == 0) { + config_preserve_symlinks_main = true; } else if (strcmp(arg, "--experimental-modules") == 0) { config_experimental_modules = true; new_v8_argv[new_v8_argc] = "--harmony-dynamic-import"; @@ -4247,6 +4255,12 @@ void Init(int* argc, SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1'; } + { + std::string text; + config_preserve_symlinks_main = + SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) && text[0] == '1'; + } + if (config_warning_file.empty()) SafeGetenv("NODE_REDIRECT_WARNINGS", &config_warning_file); diff --git a/src/node_config.cc b/src/node_config.cc index e2b2662abd0e80..603d55491a259b 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -72,6 +72,8 @@ static void Initialize(Local target, if (config_preserve_symlinks) READONLY_BOOLEAN_PROPERTY("preserveSymlinks"); + if (config_preserve_symlinks_main) + READONLY_BOOLEAN_PROPERTY("preserveSymlinksMain"); if (config_experimental_modules) { READONLY_BOOLEAN_PROPERTY("experimentalModules"); diff --git a/src/node_internals.h b/src/node_internals.h index e5ea575ebc5981..fcf48708163808 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -172,6 +172,11 @@ extern std::string openssl_config; // that is used by lib/module.js extern bool config_preserve_symlinks; +// Set in node.cc by ParseArgs when --preserve-symlinks-main is used. +// Used in node_config.cc to set a constant on process.binding('config') +// that is used by lib/module.js +extern bool config_preserve_symlinks_main; + // Set in node.cc by ParseArgs when --experimental-modules is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/module.js From ccd3ce8f81f7fccf75f1b74891a4d2ecffbc5fb3 Mon Sep 17 00:00:00 2001 From: David Goldstein Date: Wed, 11 Apr 2018 04:47:18 +0000 Subject: [PATCH 2/6] Update docs for --preserve-symlinks-main --- doc/api/cli.md | 24 ++++++++++++++++++++++++ doc/node.1 | 5 ++++- src/node.cc | 4 ++-- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 6d055d9d03a6dd..e13b75fc686d4b 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -217,6 +217,30 @@ are linked from more than one location in the dependency tree (Node.js would see those as two separate modules and would attempt to load the module multiple times, causing an exception to be thrown). +Note also that the `--preserve-symlinks` flag does not apply to the main +module - this special case helps `node --preserve-symlinks node_module/.bin/` +work. If you want the same behavior on the main module, also use +`--preserve-symlinks-main`. + +### `--preserve-symlinks-main` + + +Instructs the module loader to preserve symbolic links when resolving and +caching the main module (`require.main`). + +This flag exists so that the main module can be opted-in to the same behavior +that `--preserve-symlinks` gives to all other imports; they are separate flags, +however, for backwards compatibility with older node.js versions. + +Note that `--preserve-symlinks-main` does not imply `--preserve-symlinks`; it +is expected that `--preserve-symlinks-main` will be used in addition to +`--preserve-symlinks` when it is not desirable to follow symlinks before resolving +relative paths. + +See `--preserve-symlinks` for more information. + ### `--prof-process` Instructs the module loader to preserve symbolic links when resolving and @@ -232,12 +232,12 @@ caching the main module (`require.main`). This flag exists so that the main module can be opted-in to the same behavior that `--preserve-symlinks` gives to all other imports; they are separate flags, -however, for backwards compatibility with older node.js versions. +however, for backward compatibility with older Node.js versions. Note that `--preserve-symlinks-main` does not imply `--preserve-symlinks`; it is expected that `--preserve-symlinks-main` will be used in addition to -`--preserve-symlinks` when it is not desirable to follow symlinks before resolving -relative paths. +`--preserve-symlinks` when it is not desirable to follow symlinks before +resolving relative paths. See `--preserve-symlinks` for more information. diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 783592e1dd6f26..7d41b6ef67ebb4 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -248,8 +248,8 @@ Module._findPath = function(request, paths, isMain) { filename = toRealPath(basePath); } } else if (preserveSymlinksMain) { - // for the main module, we use the preserveSymlinksMain flag instead - // mainly for backwards compatibility, as the preserveSymlinks flag + // For the main module, we use the preserveSymlinksMain flag instead + // mainly for backward compatibility, as the preserveSymlinks flag // historically has not applied to the main module. Most likely this // was intended to keep .bin/ binaries working, as following those // symlinks is usually required for the imports in the corresponding From e434980992252725a95672b58a34c52610cdcd5e Mon Sep 17 00:00:00 2001 From: David Goldstein Date: Fri, 13 Apr 2018 22:32:34 +0000 Subject: [PATCH 4/6] rephrase doc --- doc/api/cli.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index b5fd4d745189af..d76c49615c9bca 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -217,10 +217,9 @@ are linked from more than one location in the dependency tree (Node.js would see those as two separate modules and would attempt to load the module multiple times, causing an exception to be thrown). -Note also that the `--preserve-symlinks` flag does not apply to the main -module - this special case helps -`node --preserve-symlinks node_module/.bin/` work. If you want the same -behavior on the main module, also use `--preserve-symlinks-main`. +The `--preserve-symlinks` flag does not apply to the main module, which allows +`node --preserve-symlinks node_module/.bin/` to work. To apply the same +behavior for the main module, also use `--preserve-symlinks-main`. ### `--preserve-symlinks-main`