mirror of https://github.com/lukechilds/node.git
Browse Source
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>process-exit-stdio-flushing
James M Snell
9 years ago
13 changed files with 271 additions and 9 deletions
@ -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); |
|||
@ -0,0 +1,8 @@ |
|||
{ |
|||
'targets': [ |
|||
{ |
|||
'target_name': 'binding', |
|||
'sources': [ 'binding.cc' ] |
|||
} |
|||
] |
|||
} |
|||
@ -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'); |
|||
}; |
|||
@ -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)); |
|||
}); |
|||
@ -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); |
|||
@ -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')); |
|||
}); |
|||
Loading…
Reference in new issue