remove: Make file: spec/link removal match specification

When removing a link, dependencies that are installed inside of it should
not be removed if the link target is not inside the current project.

This fixes the "removing globally installed link deletes local deps"
problem, amongst others.

PR-URL: https://github.com/npm/npm/pull/17629
Credit: @iarna
Reviewed-By: @zkat

4 files changed, 10 insertions, 7 deletions

diff --git a/lib/install/deps.js b/lib/install/deps.js
index 35c35dbfd..46955a95e 100644
--- a/lib/install/deps.js
+++ b/lib/install/deps.js
@@ -671,6 +671,7 @@ var findRequirement = exports.findRequirement = function (tree, name, requested,
     return null
   }
   if (tree.isTop) return null
+  if (!preserveSymlinks() && /^[.][.][\\/]/.test(path.relative(tree.parent.realpath, tree.realpath))) return null
   return findRequirement(tree.parent, name, requested, requestor)
 }
 
diff --git a/lib/install/diff-trees.js b/lib/install/diff-trees.js
index 02bac31c1..ac4f421a5 100644
--- a/lib/install/diff-trees.js
+++ b/lib/install/diff-trees.js
@@ -5,6 +5,7 @@ var npa = require('npm-package-arg')
 var flattenTree = require('./flatten-tree.js')
 var isOnlyDev = require('./is-only-dev.js')
 var log = require('npmlog')
+var path = require('path')
 
 function nonRegistrySource (pkg) {
   validate('O', arguments)
@@ -122,6 +123,8 @@ var diffTrees = module.exports._diffTrees = function (oldTree, newTree) {
   Object.keys(flatOldTree).forEach(function (flatname) {
     if (flatNewTree[flatname]) return
     var pkg = flatOldTree[flatname]
+    if (pkg.isInLink && /^[.][.][/\\]/.test(path.relative(newTree.realpath, pkg.realpath))) return
+
     toRemove[flatname] = pkg
     var pkgunique = getUniqueId(pkg.package)
     if (!toRemoveByUniqueId[pkgunique]) toRemoveByUniqueId[pkgunique] = []
diff --git a/lib/install/inflate-shrinkwrap.js b/lib/install/inflate-shrinkwrap.js
index 4eb877a4a..48be93d09 100644
--- a/lib/install/inflate-shrinkwrap.js
+++ b/lib/install/inflate-shrinkwrap.js
@@ -135,6 +135,7 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
     path: childPath(tree.path, pkg),
     realpath: childPath(tree.realpath, pkg),
     location: tree.location + '/' + pkg.name,
+    isLink: requested.type === 'directory',
     isInLink: tree.isLink,
     swRequires: sw.requires
   })
diff --git a/lib/install/node.js b/lib/install/node.js
index a7c9e289a..aa1d0328b 100644
--- a/lib/install/node.js
+++ b/lib/install/node.js
@@ -27,6 +27,9 @@ var defaultTemplate = {
 function isLink (node) {
   return node && node.isLink
 }
+function isInLink (node) {
+  return node && (node.isInLink || node.isLink)
+}
 
 var create = exports.create = function (node, template, isNotTop) {
   if (!template) template = defaultTemplate
@@ -41,13 +44,8 @@ var create = exports.create = function (node, template, isNotTop) {
   if (!isNotTop) {
     // isLink is true for the symlink and everything inside it.
     // by contrast, isInLink is true for only the things inside a link
-    if (node.isLink == null && isLink(node.parent)) {
-      node.isLink = true
-      node.isInLink = true
-    } else if (node.isLink == null) {
-      node.isLink = false
-      node.isInLink = false
-    }
+    if (node.isLink == null) node.isLink = isLink(node.parent)
+    if (node.isInLink == null) node.isInLink = isInLink(node.parent)
     if (node.fromBundle == null) {
       node.fromBundle = false
     }