Skip to content

Commit

Permalink
module: add --preserve-symlinks-main
Browse files Browse the repository at this point in the history
Add `--preserve-symlinks-main` option which behaves like
`--preserve-symlinks` but for `require.main`.

PR-URL: #19911
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
David Goldstein authored and targos committed May 12, 2018
1 parent de34cfa commit b6ea5df
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 4 deletions.
23 changes: 23 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,29 @@ 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).

The `--preserve-symlinks` flag does not apply to the main module, which allows
`node --preserve-symlinks node_module/.bin/<foo>` to work. To apply the same
behavior for the main module, also use `--preserve-symlinks-main`.

### `--preserve-symlinks-main`
<!-- YAML
added: REPLACEME
-->

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 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.

See `--preserve-symlinks` for more information.

### `--prof-process`
<!-- YAML
added: v5.2.0
Expand Down
5 changes: 4 additions & 1 deletion doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
Emit pending deprecation warnings.
.
.It Fl -preserve-symlinks
Instructs the module loader to preserve symbolic links when resolving and caching modules.
Instructs the module loader to preserve symbolic links when resolving and caching modules other than the main module.
.
.It F1 -preserve-symlinks-main
Instructs the module loader to preserve symbolic links when resolving and caching the main module.
.
.It Fl -prof-process
Process V8 profiler output generated using the V8 option
Expand Down
17 changes: 16 additions & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -239,7 +240,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 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
// 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);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/modules/esm/default_resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
});
Expand Down
16 changes: 15 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,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
Expand Down Expand Up @@ -3519,6 +3524,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\n"
" the main module\n"
#endif
" --prof-process process v8 profiler output generated\n"
" using --prof\n"
Expand Down Expand Up @@ -3569,7 +3576,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"
Expand Down Expand Up @@ -3832,6 +3838,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";
Expand Down Expand Up @@ -4276,6 +4284,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);

Expand Down
2 changes: 2 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ static void Initialize(Local<Object> 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");
Expand Down
5 changes: 5 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,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
Expand Down
57 changes: 57 additions & 0 deletions test/es-module/test-esm-preserve-symlinks-main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict';

const common = require('../common');
const { spawn } = require('child_process');
const assert = require('assert');
const path = require('path');
const fs = require('fs');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
const tmpDir = tmpdir.path;

fs.mkdirSync(path.join(tmpDir, 'nested'));
fs.mkdirSync(path.join(tmpDir, 'nested2'));

const entry = path.join(tmpDir, 'nested', 'entry.js');
const entry_link_absolute_path = path.join(tmpDir, 'link.js');
const submodule = path.join(tmpDir, 'nested2', 'submodule.js');
const submodule_link_absolute_path = path.join(tmpDir, 'submodule_link.js');

fs.writeFileSync(entry, `
const assert = require('assert');
// this import only resolves with --preserve-symlinks-main set
require('./submodule_link.js');
`);
fs.writeFileSync(submodule, '');

try {
fs.symlinkSync(entry, entry_link_absolute_path);
fs.symlinkSync(submodule, submodule_link_absolute_path);
} catch (err) {
if (err.code !== 'EPERM') throw err;
common.skip('insufficient privileges for symlinks');
}

function doTest(flags, done) {
// invoke the main file via a symlink. In this case --preserve-symlinks-main
// dictates that it'll resolve relative imports in the main file relative to
// the symlink, and not relative to the symlink target; the file structure set
// up above requires this to not crash when loading ./submodule_link.js
spawn(process.execPath,
flags.concat([
'--preserve-symlinks',
'--preserve-symlinks-main', entry_link_absolute_path
]),
{ stdio: 'inherit' }).on('exit', (code) => {
assert.strictEqual(code, 0);
done();
});
}

// first test the commonjs module loader
doTest([], () => {
// now test the new loader
doTest(['--experimental-modules'], () => {});
});

0 comments on commit b6ea5df

Please sign in to comment.