module: Revert "preserve symlinks when requiring"

After much discussion, it seems that the change broke the way require detects unique identity of the binary modules. While resolving module's full path is not sufficient and maybe even not elegant (like anything that requires the use of realpath() except for certain security scenarios), we cannot break multiple inclusions of binary modules across symlinked, substed or otherwise hidden paths. There should be a test case invoving a binary module to check for issues related to the binary module identity. This reverts commit de1dc0a.
nodejs · saper · May 2, 2016 · May 2, 2016 · May 2, 2016 · 7ca9bc57f2d1a930c34b407847bca17a1c370b24
commit 7ca9bc57f2d1a930c34b407847bca17a1c370b24
diff --git 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
diff --git a/test/fixtures/module-require-symlink/node_modules/bar/index.js b/test/fixtures/module-require-symlink/node_modules/bar/index.js
diff --git a/test/fixtures/module-require-symlink/node_modules/dep1/index.js b/test/fixtures/module-require-symlink/node_modules/dep1/index.js
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
diff --git a/test/fixtures/module-require-symlink/node_modules/dep2/index.js b/test/fixtures/module-require-symlink/node_modules/dep2/index.js
diff --git a/test/fixtures/module-require-symlink/symlinked.js b/test/fixtures/module-require-symlink/symlinked.js
diff --git a/test/parallel/test-require-symlink.js b/test/parallel/test-require-symlink.js