src,module: add --preserve-symlinks command line flag
Add the `--preserve-symlinks` flag. This makes the changes added in #5950 conditional. By default the old behavior is used. With the flag set, symlinks are preserved, switching to the new behavior. This should be considered to be a temporary solution until we figure out how to solve the symlinked peer dependency problem in a more general way that does not break everything else. Additional test cases are included. PR-URL: https://github.com/nodejs/node/pull/6537 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
parent
f52b2f116b
commit
5d38d543cd
@ -97,6 +97,43 @@ Automatically zero-fills all newly allocated [Buffer][] and [SlowBuffer][]
|
||||
instances.
|
||||
|
||||
|
||||
### `--preserve-symlinks`
|
||||
|
||||
Instructs the module loader to preserve symbolic links when resolving and
|
||||
caching modules.
|
||||
|
||||
By default, when Node.js loads a module from a path that is symbolically linked
|
||||
to a different on-disk location, Node.js will dereference the link and use the
|
||||
actual on-disk "real path" of the module as both an identifier and as a root
|
||||
path to locate other dependency modules. In most cases, this default behavior
|
||||
is acceptable. However, when using symbolically linked peer dependencies, as
|
||||
illustrated in the example below, the default behavior causes an exception to
|
||||
be thrown if `moduleA` attempts to require `moduleB` as a peer dependency:
|
||||
|
||||
```text
|
||||
{appDir}
|
||||
├── app
|
||||
│ ├── index.js
|
||||
│ └── node_modules
|
||||
│ ├── moduleA -> {appDir}/moduleA
|
||||
│ └── moduleB
|
||||
│ ├── index.js
|
||||
│ └── package.json
|
||||
└── moduleA
|
||||
├── index.js
|
||||
└── package.json
|
||||
```
|
||||
|
||||
The `--preserve-symlinks` command line flag instructs Node.js to use the
|
||||
symlink path for modules as opposed to the real path, allowing symbolically
|
||||
linked peer dependencies to be found.
|
||||
|
||||
Note, however, that using `--preserve-symlinks` can have other side effects.
|
||||
Specifically, symbolically linked *native* modules can fail to load if those
|
||||
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).
|
||||
|
||||
### `--track-heap-objects`
|
||||
|
||||
Track heap object allocations for heap snapshots.
|
||||
@ -138,7 +175,6 @@ Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
|
||||
|
||||
Specify ICU data load path. (overrides `NODE_ICU_DATA`)
|
||||
|
||||
|
||||
## Environment Variables
|
||||
|
||||
### `NODE_DEBUG=module[,…]`
|
||||
|
@ -95,6 +95,11 @@ of the event loop.
|
||||
.BR \-\-zero\-fill\-buffers
|
||||
Automatically zero-fills all newly allocated Buffer and SlowBuffer instances.
|
||||
|
||||
.TP
|
||||
.BR \-\-preserve\-symlinks
|
||||
Instructs the module loader to preserve symbolic links when resolving and
|
||||
caching modules.
|
||||
|
||||
.TP
|
||||
.BR \-\-track\-heap-objects
|
||||
Track heap object allocations for heap snapshots.
|
||||
|
@ -10,6 +10,7 @@ const fs = require('fs');
|
||||
const path = require('path');
|
||||
const internalModuleReadFile = process.binding('fs').internalModuleReadFile;
|
||||
const internalModuleStat = process.binding('fs').internalModuleStat;
|
||||
const preserveSymlinks = !!process.binding('config').preserveSymlinks;
|
||||
|
||||
// If obj.hasOwnProperty has been overridden, then calling
|
||||
// obj.hasOwnProperty(prop) will break.
|
||||
@ -109,14 +110,15 @@ function tryPackage(requestPath, exts, isMain) {
|
||||
}
|
||||
|
||||
// check if the file exists and is not a directory
|
||||
// resolve to the absolute realpath if running main module,
|
||||
// otherwise resolve to absolute while keeping symlinks intact.
|
||||
// if using --preserve-symlinks and isMain is false,
|
||||
// keep symlinks intact, otherwise resolve to the
|
||||
// absolute realpath.
|
||||
function tryFile(requestPath, isMain) {
|
||||
const rc = stat(requestPath);
|
||||
if (isMain) {
|
||||
return rc === 0 && fs.realpathSync(requestPath);
|
||||
if (preserveSymlinks && !isMain) {
|
||||
return rc === 0 && path.resolve(requestPath);
|
||||
}
|
||||
return rc === 0 && path.resolve(requestPath);
|
||||
return rc === 0 && fs.realpathSync(requestPath);
|
||||
}
|
||||
|
||||
// given a path check a the file exists with any of the set extensions
|
||||
@ -159,7 +161,7 @@ Module._findPath = function(request, paths, isMain) {
|
||||
if (!trailingSlash) {
|
||||
const rc = stat(basePath);
|
||||
if (rc === 0) { // File.
|
||||
if (!isMain) {
|
||||
if (preserveSymlinks && !isMain) {
|
||||
filename = path.resolve(basePath);
|
||||
} else {
|
||||
filename = fs.realpathSync(basePath);
|
||||
|
@ -168,6 +168,11 @@ bool force_fips_crypto = false;
|
||||
bool no_process_warnings = false;
|
||||
bool trace_warnings = false;
|
||||
|
||||
// Set in node.cc by ParseArgs when --preserve-symlinks 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 = false;
|
||||
|
||||
// process-relative uptime base, initialized at start-up
|
||||
static double prog_start_time;
|
||||
static bool debugger_running;
|
||||
@ -3460,6 +3465,8 @@ static void PrintHelp() {
|
||||
" note: linked-in ICU data is\n"
|
||||
" present.\n"
|
||||
#endif
|
||||
" --preserve-symlinks preserve symbolic links when resolving\n"
|
||||
" and caching modules.\n"
|
||||
#endif
|
||||
"\n"
|
||||
"Environment variables:\n"
|
||||
@ -3589,6 +3596,8 @@ static void ParseArgs(int* argc,
|
||||
} else if (strncmp(arg, "--security-revert=", 18) == 0) {
|
||||
const char* cve = arg + 18;
|
||||
Revert(cve);
|
||||
} else if (strcmp(arg, "--preserve-symlinks") == 0) {
|
||||
config_preserve_symlinks = true;
|
||||
} else if (strcmp(arg, "--prof-process") == 0) {
|
||||
prof_process = true;
|
||||
short_circuit = true;
|
||||
|
@ -41,7 +41,10 @@ void InitConfig(Local<Object> target,
|
||||
if (flag_icu_data_dir)
|
||||
READONLY_BOOLEAN_PROPERTY("usingICUDataDir");
|
||||
#endif // NODE_HAVE_I18N_SUPPORT
|
||||
}
|
||||
|
||||
if (config_preserve_symlinks)
|
||||
READONLY_BOOLEAN_PROPERTY("preserveSymlinks");
|
||||
} // InitConfig
|
||||
|
||||
} // namespace node
|
||||
|
||||
|
@ -30,6 +30,11 @@ struct sockaddr;
|
||||
|
||||
namespace node {
|
||||
|
||||
// Set in node.cc by ParseArgs when --preserve-symlinks 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;
|
||||
|
||||
// Forward declaration
|
||||
class Environment;
|
||||
|
||||
|
13
test/addons/symlinked-module/binding.cc
Normal file
13
test/addons/symlinked-module/binding.cc
Normal file
@ -0,0 +1,13 @@
|
||||
#include <node.h>
|
||||
#include <v8.h>
|
||||
|
||||
void Method(const v8::FunctionCallbackInfo<v8::Value>& args) {
|
||||
v8::Isolate* isolate = args.GetIsolate();
|
||||
args.GetReturnValue().Set(v8::String::NewFromUtf8(isolate, "world"));
|
||||
}
|
||||
|
||||
void init(v8::Local<v8::Object> target) {
|
||||
NODE_SET_METHOD(target, "hello", Method);
|
||||
}
|
||||
|
||||
NODE_MODULE(binding, init);
|
8
test/addons/symlinked-module/binding.gyp
Normal file
8
test/addons/symlinked-module/binding.gyp
Normal file
@ -0,0 +1,8 @@
|
||||
{
|
||||
'targets': [
|
||||
{
|
||||
'target_name': 'binding',
|
||||
'sources': [ 'binding.cc' ]
|
||||
}
|
||||
]
|
||||
}
|
13
test/addons/symlinked-module/submodule.js
Normal file
13
test/addons/symlinked-module/submodule.js
Normal file
@ -0,0 +1,13 @@
|
||||
'use strict';
|
||||
require('../../common');
|
||||
const path = require('path');
|
||||
const assert = require('assert');
|
||||
|
||||
// This is a subtest of symlinked-module/test.js. This is not
|
||||
// intended to be run directly.
|
||||
|
||||
module.exports.test = function test(bindingDir) {
|
||||
const mod = require(path.join(bindingDir, 'binding.node'));
|
||||
assert.notStrictEqual(mod, null);
|
||||
assert.strictEqual(mod.hello(), 'world');
|
||||
};
|
34
test/addons/symlinked-module/test.js
Normal file
34
test/addons/symlinked-module/test.js
Normal file
@ -0,0 +1,34 @@
|
||||
'use strict';
|
||||
const common = require('../../common');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const assert = require('assert');
|
||||
|
||||
// This test verifies that symlinked native modules can be required multiple
|
||||
// times without error. The symlinked module and the non-symlinked module
|
||||
// should be the same instance. This expectation was not previously being
|
||||
// tested and ended up being broken by https://github.com/nodejs/node/pull/5950.
|
||||
|
||||
// This test should pass in Node.js v4 and v5. This test will pass in Node.js
|
||||
// with https://github.com/nodejs/node/pull/5950 reverted.
|
||||
|
||||
common.refreshTmpDir();
|
||||
|
||||
const addonPath = path.join(__dirname, 'build', 'Release');
|
||||
const addonLink = path.join(common.tmpDir, 'addon');
|
||||
|
||||
try {
|
||||
fs.symlinkSync(addonPath, addonLink);
|
||||
} catch (err) {
|
||||
if (err.code !== 'EPERM') throw err;
|
||||
common.skip('module identity test (no privs for symlinks)');
|
||||
return;
|
||||
}
|
||||
|
||||
const sub = require('./submodule');
|
||||
[addonPath, addonLink].forEach((i) => {
|
||||
const mod = require(path.join(i, 'binding.node'));
|
||||
assert.notStrictEqual(mod, null);
|
||||
assert.strictEqual(mod.hello(), 'world');
|
||||
assert.doesNotThrow(() => sub.test(i));
|
||||
});
|
68
test/parallel/test-module-circular-symlinks.js
Normal file
68
test/parallel/test-module-circular-symlinks.js
Normal file
@ -0,0 +1,68 @@
|
||||
'use strict';
|
||||
|
||||
// This tests to make sure that modules with symlinked circular dependencies
|
||||
// do not blow out the module cache and recurse forever. See issue
|
||||
// https://github.com/nodejs/node/pull/5950 for context. PR #5950 attempted
|
||||
// to solve a problem with symlinked peer dependencies by caching using the
|
||||
// symlink path. Unfortunately, that breaks the case tested in this module
|
||||
// because each symlinked module, despite pointing to the same code on disk,
|
||||
// is loaded and cached as a separate module instance, which blows up the
|
||||
// cache and leads to a recursion bug.
|
||||
|
||||
// This test should pass in Node.js v4 and v5. It should pass in Node.js v6
|
||||
// after https://github.com/nodejs/node/pull/5950 has been reverted.
|
||||
|
||||
const common = require('../common');
|
||||
const assert = require('assert');
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
|
||||
// {tmpDir}
|
||||
// ├── index.js
|
||||
// └── node_modules
|
||||
// ├── moduleA
|
||||
// │ ├── index.js
|
||||
// │ └── node_modules
|
||||
// │ └── moduleB -> {tmpDir}/node_modules/moduleB
|
||||
// └── moduleB
|
||||
// ├── index.js
|
||||
// └── node_modules
|
||||
// └── moduleA -> {tmpDir}/node_modules/moduleA
|
||||
|
||||
common.refreshTmpDir();
|
||||
const tmpDir = common.tmpDir;
|
||||
|
||||
const node_modules = path.join(tmpDir, 'node_modules');
|
||||
const moduleA = path.join(node_modules, 'moduleA');
|
||||
const moduleB = path.join(node_modules, 'moduleB');
|
||||
const moduleA_link = path.join(moduleB, 'node_modules', 'moduleA');
|
||||
const moduleB_link = path.join(moduleA, 'node_modules', 'moduleB');
|
||||
|
||||
fs.mkdirSync(node_modules);
|
||||
fs.mkdirSync(moduleA);
|
||||
fs.mkdirSync(moduleB);
|
||||
fs.mkdirSync(path.join(moduleA, 'node_modules'));
|
||||
fs.mkdirSync(path.join(moduleB, 'node_modules'));
|
||||
|
||||
try {
|
||||
fs.symlinkSync(moduleA, moduleA_link);
|
||||
fs.symlinkSync(moduleB, moduleB_link);
|
||||
} catch (err) {
|
||||
if (err.code !== 'EPERM') throw err;
|
||||
common.skip('insufficient privileges for symlinks');
|
||||
return;
|
||||
}
|
||||
|
||||
fs.writeFileSync(path.join(tmpDir, 'index.js'),
|
||||
'module.exports = require(\'moduleA\');', 'utf8');
|
||||
fs.writeFileSync(path.join(moduleA, 'index.js'),
|
||||
'module.exports = {b: require(\'moduleB\')};', 'utf8');
|
||||
fs.writeFileSync(path.join(moduleB, 'index.js'),
|
||||
'module.exports = {a: require(\'moduleA\')};', 'utf8');
|
||||
|
||||
// Ensure that the symlinks are not followed forever...
|
||||
const obj = require(path.join(tmpDir, 'index'));
|
||||
assert.ok(obj);
|
||||
assert.ok(obj.b);
|
||||
assert.ok(obj.b.a);
|
||||
assert.ok(!obj.b.a.b);
|
65
test/parallel/test-module-symlinked-peer-modules.js
Normal file
65
test/parallel/test-module-symlinked-peer-modules.js
Normal file
@ -0,0 +1,65 @@
|
||||
// Flags: --preserve-symlinks
|
||||
'use strict';
|
||||
// Refs: https://github.com/nodejs/node/pull/5950
|
||||
|
||||
// This test verifies that symlinked modules are able to find their peer
|
||||
// dependencies when using the --preserve-symlinks command line flag.
|
||||
|
||||
// This test passes in v6.2+ with --preserve-symlinks on and in v6.0/v6.1.
|
||||
// This test will fail in Node.js v4 and v5 and should not be backported.
|
||||
|
||||
const common = require('../common');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const assert = require('assert');
|
||||
|
||||
common.refreshTmpDir();
|
||||
|
||||
const tmpDir = common.tmpDir;
|
||||
|
||||
// Creates the following structure
|
||||
// {tmpDir}
|
||||
// ├── app
|
||||
// │ ├── index.js
|
||||
// │ └── node_modules
|
||||
// │ ├── moduleA -> {tmpDir}/moduleA
|
||||
// │ └── moduleB
|
||||
// │ ├── index.js
|
||||
// │ └── package.json
|
||||
// └── moduleA
|
||||
// ├── index.js
|
||||
// └── package.json
|
||||
|
||||
const moduleA = path.join(tmpDir, 'moduleA');
|
||||
const app = path.join(tmpDir, 'app');
|
||||
const moduleB = path.join(app, 'node_modules', 'moduleB');
|
||||
const moduleA_link = path.join(app, 'node_modules', 'moduleA');
|
||||
fs.mkdirSync(moduleA);
|
||||
fs.mkdirSync(app);
|
||||
fs.mkdirSync(path.join(app, 'node_modules'));
|
||||
fs.mkdirSync(moduleB);
|
||||
|
||||
// Attempt to make the symlink. If this fails due to lack of sufficient
|
||||
// permissions, the test will bail out and be skipped.
|
||||
try {
|
||||
fs.symlinkSync(moduleA, moduleA_link);
|
||||
} catch (err) {
|
||||
if (err.code !== 'EPERM') throw err;
|
||||
common.skip('insufficient privileges for symlinks');
|
||||
return;
|
||||
}
|
||||
|
||||
fs.writeFileSync(path.join(moduleA, 'package.json'),
|
||||
JSON.stringify({name: 'moduleA', main: 'index.js'}), 'utf8');
|
||||
fs.writeFileSync(path.join(moduleA, 'index.js'),
|
||||
'module.exports = require(\'moduleB\');', 'utf8');
|
||||
fs.writeFileSync(path.join(app, 'index.js'),
|
||||
'\'use strict\'; require(\'moduleA\');', 'utf8');
|
||||
fs.writeFileSync(path.join(moduleB, 'package.json'),
|
||||
JSON.stringify({name: 'moduleB', main: 'index.js'}), 'utf8');
|
||||
fs.writeFileSync(path.join(moduleB, 'index.js'),
|
||||
'module.exports = 1;', 'utf8');
|
||||
|
||||
assert.doesNotThrow(() => {
|
||||
require(path.join(app, 'index'));
|
||||
});
|
@ -1,3 +1,4 @@
|
||||
// Flags: --preserve-symlinks
|
||||
'use strict';
|
||||
const common = require('../common');
|
||||
const assert = require('assert');
|
||||
@ -49,7 +50,7 @@ function test() {
|
||||
|
||||
// load symlinked-script as main
|
||||
var node = process.execPath;
|
||||
var child = spawn(node, [linkScript]);
|
||||
var child = spawn(node, ['--preserve-symlinks', linkScript]);
|
||||
child.on('close', function(code, signal) {
|
||||
assert(!code);
|
||||
assert(!signal);
|
||||
|
Loading…
x
Reference in New Issue
Block a user