Skip to content

Commit

Permalink
install: Switch to filtering the tree at load time rather than adhoc …
Browse files Browse the repository at this point in the history
…loads

It turns out this is necessary or else we get things wrong when determining
what's a symlink and what isn't.

PR-URL: npm#8977
  • Loading branch information
iarna committed Jul 18, 2015
1 parent c0721f3 commit 6d69ec9
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 29 deletions.
18 changes: 8 additions & 10 deletions lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,18 +542,16 @@ Installer.prototype.readGlobalPackageData = function (cb) {
validate('F', arguments)
log.silly('install', 'readGlobalPackageData')
var self = this
this.currentTree = new readPackageTree.Node(null, this.where, this.where, null, {})
this.loadArgMetadata(iferr(cb, function () {
mkdirp(self.where, iferr(cb, function () {
asyncMap(self.args, function (pkg, done) {
readPackageTree(path.resolve(self.where, 'node_modules', pkg.name), function (_, subTree) {
if (subTree) {
subTree.parent = self.currentTree
self.currentTree.children.push(subTree)
}
done()
})
}, cb)
var pkgs = {}
self.args.forEach(function (pkg) {
pkgs[pkg.name] = true
})
readPackageTree(self.where, function (ctx, kid) { return ctx.parent || pkgs[kid] }, iferr(cb, function (currentTree) {
self.currentTree = currentTree
return cb()
}))
}))
}))
}
Expand Down
3 changes: 2 additions & 1 deletion lib/install/deps.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ function matchingDep (tree, name) {
}

function packageRelativePath (tree) {
if (!tree) return ''
var requested = tree.package._requested || {}
var isLocal = requested.type === 'directory' || requested.type === 'local'
return isLocal ? requested.spec : tree.path
Expand All @@ -131,7 +132,7 @@ function getShrinkwrap (tree, name) {

exports.getAllMetadata = function (args, tree, next) {
asyncMap(args, function (spec, done) {
if (spec.lastIndexOf('@') <= 0) {
if (tree && spec.lastIndexOf('@') <= 0) {
var sw = getShrinkwrap(tree, spec)
if (sw) {
// FIXME: This is duplicated in inflate-shrinkwrap and should be factoed
Expand Down
60 changes: 42 additions & 18 deletions test/tap/no-scan-full-global-dir.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,44 @@
'use strict'
var path = require('path')
var test = require('tap').test
var requireInject = require('require-inject')
var path = require('path')
var osenv = require('osenv')
var inherits = require('inherits')
var npm = require('../../lib/npm.js')

var packages = {
test: {package: {name: 'test'}, path: __dirname, children: ['abc', 'def', 'ghi', 'jkl']},
abc: {package: {name: 'abc'}, path: path.join(__dirname, 'node_modules', 'abc')},
def: {package: {name: 'def'}, path: path.join(__dirname, 'node_modules', 'def')},
ghi: {package: {name: 'ghi'}, path: path.join(__dirname, 'node_modules', 'ghi')},
jkl: {package: {name: 'jkl'}, path: path.join(__dirname, 'node_modules', 'jkl')}
}
var dir = {}
dir[__dirname] = { children: [ packages.abc, packages.def, packages.ghi, packages.jkl ] }
dir[packages.abc.path] = packages.abc
dir[packages.def.path] = packages.def
dir[packages.ghi.path] = packages.ghi
dir[packages.jkl.path] = packages.jkl
var dirs = {}
var files = {}
Object.keys(packages).forEach(function (name) {
dirs[path.join(packages[name].path, 'node_modules')] = packages[name].children || []
files[path.join(packages[name].path, 'package.json')] = packages[name].package
})

var rpt = function (root, cb) {
cb(null, dir[root])
process.chdir(osenv.tmpdir())

var mockReaddir = function (name, cb) {
if (dirs[name]) return cb(null, dirs[name])
var er = new Error('No such mock: ' + name)
er.code = 'ENOENT'
cb(er)
}
rpt.Node = function () {
this.children = []
var mockReadPackageJson = function (file, cb) {
if (files[file]) return cb(null, files[file])
var er = new Error('No such mock: ' + file)
er.code = 'ENOENT'
cb(er)
}
var mockFs = {
realpath: function (dir, cb) {
return cb(null, dir)
}
}

var npm = requireInject.installGlobally('../../lib/npm.js', {
'read-package-tree': rpt
})

test('setup', function (t) {
npm.load(function () {
Expand All @@ -42,7 +54,13 @@ function loadArgMetadata (cb) {

test('installer', function (t) {
t.plan(1)
var Installer = require('../../lib/install.js').Installer
var installer = requireInject('../../lib/install.js', {
'fs': mockFs,
'readdir-scoped-modules': mockReaddir,
'read-package-json': mockReadPackageJson
})

var Installer = installer.Installer
var TestInstaller = function () {
Installer.apply(this, arguments)
this.global = true
Expand All @@ -53,14 +71,20 @@ test('installer', function (t) {
var inst = new TestInstaller(__dirname, false, ['def', 'abc'])
inst.loadCurrentTree(function () {
var kids = inst.currentTree.children.map(function (child) { return child.package.name })
t.isDeeply(kids, ['def', 'abc'])
t.isDeeply(kids, ['abc', 'def'])
t.end()
})
})

test('uninstaller', function (t) {
t.plan(1)
var Uninstaller = require('../../lib/uninstall.js').Uninstaller
var uninstaller = requireInject('../../lib/uninstall.js', {
'fs': mockFs,
'readdir-scoped-modules': mockReaddir,
'read-package-json': mockReadPackageJson
})

var Uninstaller = uninstaller.Uninstaller
var TestUninstaller = function () {
Uninstaller.apply(this, arguments)
this.global = true
Expand Down
136 changes: 136 additions & 0 deletions test/tap/rm-linked.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
var mkdirp = require('mkdirp')
var osenv = require('osenv')
var path = require('path')
var rimraf = require('rimraf')
var test = require('tap').test
var writeFileSync = require('fs').writeFileSync

var common = require('../common-tap.js')

var link = path.join(__dirname, 'rmlinked')
var linkDep = path.join(link, 'node_modules', 'baz')
var linkInstall = path.join(__dirname, 'rmlinked-install')
var linkRoot = path.join(__dirname, 'rmlinked-root')

var config = 'prefix = ' + linkRoot
var configPath = path.join(link, '_npmrc')

var OPTS = {
env: {
'npm_config_userconfig': configPath
}
}

var linkedJSON = {
name: 'foo',
version: '1.0.0',
description: '',
main: 'index.js',
scripts: {
test: 'echo \"Error: no test specified\" && exit 1'
},
dependencies: {
'baz': '1.0.0'
},
author: '',
license: 'ISC'
}

var linkedDepJSON = {
name: 'baz',
version: '1.0.0',
description: '',
main: 'index.js',
scripts: {
test: 'echo \"Error: no test specified\" && exit 1'
},
author: '',
license: 'ISC'
}

var installJSON = {
name: 'bar',
version: '1.0.0',
description: '',
main: 'index.js',
scripts: {
test: 'echo \"Error: no test specified\" && exit 1'
},
dependencies: {
'foo': '1.0.0'
},
author: '',
license: 'ISC'
}

test('setup', function (t) {
setup()
common.npm(['ls', '-g', '--depth=0'], OPTS, function (err, c, out) {
t.ifError(err)
t.equal(c, 0, 'set up ok')
t.notOk(out.match(/UNMET DEPENDENCY foo@/), "foo isn't in global")
t.end()
})
})

test('creates global link', function (t) {
process.chdir(link)
common.npm(['link'], OPTS, function (err, c, out) {
t.ifError(err, 'link has no error')
common.npm(['ls', '-g'], OPTS, function (err, c, out, stderr) {
t.ifError(err)
t.equal(c, 0)
t.equal(stderr, '', 'got expected stderr')
t.has(out, /[email protected]/, 'creates global link ok')
t.end()
})
})
})

test('uninstall the global linked package', function (t) {
process.chdir(osenv.tmpdir())
common.npm(['uninstall', '-g', 'foo'], OPTS, function (err) {
t.ifError(err, 'uninstall has no error')
process.chdir(link)
common.npm(['ls'], OPTS, function (err, c, out) {
t.ifError(err)
t.equal(c, 0)
t.has(out, /[email protected]/, "uninstall didn't remove dep")
t.end()
})
})
})

test('cleanup', function (t) {
cleanup()
t.end()
})

function cleanup () {
process.chdir(osenv.tmpdir())
try { rimraf.sync(linkRoot) } catch (e) { }
try { rimraf.sync(linkDep) } catch (e) { }
try { rimraf.sync(link) } catch (e) { }
try { rimraf.sync(linkInstall) } catch (e) { }
}

function setup () {
cleanup()
mkdirp.sync(linkRoot)
mkdirp.sync(link)
writeFileSync(
path.join(link, 'package.json'),
JSON.stringify(linkedJSON, null, 2)
)
mkdirp.sync(linkDep)
writeFileSync(
path.join(linkDep, 'package.json'),
JSON.stringify(linkedDepJSON, null, 2)
)
mkdirp.sync(linkInstall)
writeFileSync(
path.join(linkInstall, 'package.json'),
JSON.stringify(installJSON, null, 2)
)
writeFileSync(configPath, config)
}

0 comments on commit 6d69ec9

Please sign in to comment.