From 8b634e86ec7db98301ee2d84ad751c350af493db Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 2 May 2016 15:38:16 -0700 Subject: [PATCH] Revert "module: preserve symlinks when requiring" This reverts commit de1dc0ae2eb52842b5c5c974090123a64c3a594c. While this is a legitimate bug fix for a legitimate problem, the fix itself is causing too much breakage. We need to step back and find another way of addressing the problem. --- lib/module.js | 43 ++++++-------- test/fixtures/module-require-symlink/foo.js | 2 - .../node_modules/bar/index.js | 1 - .../node_modules/dep1/index.js | 2 - .../dep1/node_modules/bar/index.js | 1 - .../node_modules/dep2/index.js | 1 - .../module-require-symlink/symlinked.js | 13 ----- test/parallel/test-require-symlink.js | 57 ------------------- 8 files changed, 17 insertions(+), 103 deletions(-) delete mode 100644 test/fixtures/module-require-symlink/foo.js delete mode 100644 test/fixtures/module-require-symlink/node_modules/bar/index.js delete mode 100644 test/fixtures/module-require-symlink/node_modules/dep1/index.js delete mode 100644 test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js delete mode 100644 test/fixtures/module-require-symlink/node_modules/dep2/index.js delete mode 100644 test/fixtures/module-require-symlink/symlinked.js delete mode 100644 test/parallel/test-require-symlink.js diff --git a/lib/module.js b/lib/module.js index fb233d6c4541fe..f633d62c490c39 100644 --- a/lib/module.js +++ b/lib/module.js @@ -97,32 +97,27 @@ function readPackage(requestPath) { return pkg; } -function tryPackage(requestPath, exts, isMain) { +function tryPackage(requestPath, exts) { var pkg = readPackage(requestPath); if (!pkg) return false; var filename = path.resolve(requestPath, pkg); - return tryFile(filename, isMain) || - tryExtensions(filename, exts, isMain) || - tryExtensions(path.resolve(filename, 'index'), exts, isMain); + return tryFile(filename) || + tryExtensions(filename, exts) || + tryExtensions(path.resolve(filename, 'index'), exts); } // 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. -function tryFile(requestPath, isMain) { +function tryFile(requestPath) { const rc = stat(requestPath); - if (isMain) { - return rc === 0 && fs.realpathSync(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 -function tryExtensions(p, exts, isMain) { +function tryExtensions(p, exts) { for (var i = 0; i < exts.length; i++) { - const filename = tryFile(p + exts[i], isMain); + const filename = tryFile(p + exts[i]); if (filename) { return filename; @@ -132,7 +127,7 @@ function tryExtensions(p, exts, isMain) { } var warned = false; -Module._findPath = function(request, paths, isMain) { +Module._findPath = function(request, paths) { if (path.isAbsolute(request)) { paths = ['']; } else if (!paths || paths.length === 0) { @@ -159,36 +154,32 @@ Module._findPath = function(request, paths, isMain) { if (!trailingSlash) { const rc = stat(basePath); if (rc === 0) { // File. - if (!isMain) { - filename = path.resolve(basePath); - } else { - filename = fs.realpathSync(basePath); - } + filename = fs.realpathSync(basePath); } else if (rc === 1) { // Directory. if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts, isMain); + filename = tryPackage(basePath, exts); } if (!filename) { // try it with each of the extensions if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryExtensions(basePath, exts, isMain); + filename = tryExtensions(basePath, exts); } } if (!filename) { if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts, isMain); + filename = tryPackage(basePath, exts); } if (!filename) { // try it with each of the extensions at "index" if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain); + filename = tryExtensions(path.resolve(basePath, 'index'), exts); } if (filename) { @@ -383,7 +374,7 @@ Module._load = function(request, parent, isMain) { debug('Module._load REQUEST %s parent: %s', request, parent.id); } - var filename = Module._resolveFilename(request, parent, isMain); + var filename = Module._resolveFilename(request, parent); var cachedModule = Module._cache[filename]; if (cachedModule) { @@ -421,7 +412,7 @@ function tryModuleLoad(module, filename) { } } -Module._resolveFilename = function(request, parent, isMain) { +Module._resolveFilename = function(request, parent) { if (NativeModule.nonInternalExists(request)) { return request; } @@ -433,7 +424,7 @@ Module._resolveFilename = function(request, parent, isMain) { // look up the filename first, since that's the cache key. debug('looking for %j in %j', id, paths); - var filename = Module._findPath(request, paths, isMain); + var filename = Module._findPath(request, paths); if (!filename) { var err = new Error("Cannot find module '" + request + "'"); err.code = 'MODULE_NOT_FOUND'; diff --git a/test/fixtures/module-require-symlink/foo.js b/test/fixtures/module-require-symlink/foo.js deleted file mode 100644 index e7361df25ed218..00000000000000 --- a/test/fixtures/module-require-symlink/foo.js +++ /dev/null @@ -1,2 +0,0 @@ -exports.dep1 = require('dep1'); -exports.dep2 = exports.dep1.dep2; diff --git a/test/fixtures/module-require-symlink/node_modules/bar/index.js b/test/fixtures/module-require-symlink/node_modules/bar/index.js deleted file mode 100644 index f8d439ec7e126c..00000000000000 --- a/test/fixtures/module-require-symlink/node_modules/bar/index.js +++ /dev/null @@ -1 +0,0 @@ -exports.version = 'INCORRECT_VERSION'; diff --git a/test/fixtures/module-require-symlink/node_modules/dep1/index.js b/test/fixtures/module-require-symlink/node_modules/dep1/index.js deleted file mode 100644 index 78f255b8f4ad35..00000000000000 --- a/test/fixtures/module-require-symlink/node_modules/dep1/index.js +++ /dev/null @@ -1,2 +0,0 @@ -exports.bar = require('bar'); -exports.dep2 = require('dep2'); diff --git a/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js b/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js deleted file mode 100644 index 6d8b0321d21fd5..00000000000000 --- a/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js +++ /dev/null @@ -1 +0,0 @@ -exports.version = 'CORRECT_VERSION'; diff --git a/test/fixtures/module-require-symlink/node_modules/dep2/index.js b/test/fixtures/module-require-symlink/node_modules/dep2/index.js deleted file mode 100644 index 2b81d1512858cd..00000000000000 --- a/test/fixtures/module-require-symlink/node_modules/dep2/index.js +++ /dev/null @@ -1 +0,0 @@ -exports.bar = require('bar'); diff --git a/test/fixtures/module-require-symlink/symlinked.js b/test/fixtures/module-require-symlink/symlinked.js deleted file mode 100644 index fea1cedb0aa4f8..00000000000000 --- a/test/fixtures/module-require-symlink/symlinked.js +++ /dev/null @@ -1,13 +0,0 @@ -'use strict'; -const assert = require('assert'); -const common = require('../../common'); -const path = require('path'); - -const linkScriptTarget = path.join(common.fixturesDir, - '/module-require-symlink/symlinked.js'); - -var foo = require('./foo'); -assert.equal(foo.dep1.bar.version, 'CORRECT_VERSION'); -assert.equal(foo.dep2.bar.version, 'CORRECT_VERSION'); -assert.equal(__filename, linkScriptTarget); -assert(__filename in require.cache); diff --git a/test/parallel/test-require-symlink.js b/test/parallel/test-require-symlink.js deleted file mode 100644 index 6b716ffa1381ac..00000000000000 --- a/test/parallel/test-require-symlink.js +++ /dev/null @@ -1,57 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const path = require('path'); -const fs = require('fs'); -const exec = require('child_process').exec; -const spawn = require('child_process').spawn; - -const linkTarget = path.join(common.fixturesDir, - '/module-require-symlink/node_modules/dep2/'); - -const linkDir = path.join(common.fixturesDir, - '/module-require-symlink/node_modules/dep1/node_modules/dep2'); - -const linkScriptTarget = path.join(common.fixturesDir, - '/module-require-symlink/symlinked.js'); - -const linkScript = path.join(common.tmpDir, 'module-require-symlink.js'); - -if (common.isWindows) { - // On Windows, creating symlinks requires admin privileges. - // We'll only try to run symlink test if we have enough privileges. - exec('whoami /priv', function(err, o) { - if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) { - console.log('Skipped: insufficient privileges'); - return; - } else { - test(); - } - }); -} else { - test(); -} - -function test() { - process.on('exit', function() { - fs.unlinkSync(linkDir); - fs.unlinkSync(linkScript); - }); - - fs.symlinkSync(linkTarget, linkDir); - fs.symlinkSync(linkScriptTarget, linkScript); - - // load symlinked-module - var fooModule = - require(path.join(common.fixturesDir, '/module-require-symlink/foo.js')); - assert.equal(fooModule.dep1.bar.version, 'CORRECT_VERSION'); - assert.equal(fooModule.dep2.bar.version, 'CORRECT_VERSION'); - - // load symlinked-script as main - var node = process.execPath; - var child = spawn(node, [linkScript]); - child.on('close', function(code, signal) { - assert(!code); - assert(!signal); - }); -}