Skip to content

Commit

Permalink
install: fix unclean unbuild of nested packages with bin
Browse files Browse the repository at this point in the history
Previously, installed packages that contained node_modules with a .bin
directory woudn't get cleaned up correctly. After unbuild of that particular
package, npm would complain with ENOENT errors because the package directory
would be empty (without package.json), as it only contains the leftover
`node_modules/.bin` directory.

PR-URL: npm#11181
Credit: @chrisirhc
Reviewed-By: @othiym23
Fixes: npm#10887
Fixes: npm#10938
  • Loading branch information
chrisirhc authored and iarna committed Jan 21, 2016
1 parent 00720db commit ea331c8
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/unbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ function rmBins (pkg, folder, parent, top, cb) {
} else {
gentlyRm(path.resolve(binRoot, b), true, folder, cb)
}
}, cb)
}, gentlyRmBinRoot)

function gentlyRmBinRoot (err) {
if (err || top) return cb(err)
return gentlyRm(binRoot, true, parent, cb)
}
}

function rmMans (pkg, folder, parent, top, cb) {
Expand Down
117 changes: 117 additions & 0 deletions test/tap/uninstall-link-clean.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
var fs = require('graceful-fs')
var path = require('path')
var existsSync = fs.existsSync || path.existsSync

var mkdirp = require('mkdirp')
var osenv = require('osenv')
var rimraf = require('rimraf')
var test = require('tap').test

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

var pkg = path.join(__dirname, 'uninstall-link-clean')
var dep = path.join(__dirname, 'dep')
var work = path.join(__dirname, 'uninstall-link-clean-TEST')
var modules = path.join(work, 'node_modules')

var EXEC_OPTS = { cwd: work }

var world = 'console.log("hello blrbld")\n'

var json = {
name: 'package',
version: '0.0.0',
bin: {
hello: './world.js'
},
dependencies: {
'dep': 'file:../dep'
}
}

var pjDep = {
name: 'dep',
version: '0.0.0',
bin: {
hello: './world.js'
}
}

test('setup', function (t) {
cleanup()
mkdirp.sync(pkg)
fs.writeFileSync(
path.join(pkg, 'package.json'),
JSON.stringify(json, null, 2)
)
fs.writeFileSync(path.join(pkg, 'world.js'), world)

mkdirp.sync(dep)
fs.writeFileSync(
path.join(dep, 'package.json'),
JSON.stringify(pjDep, null, 2)
)
fs.writeFileSync(path.join(dep, 'world.js'), world)

mkdirp.sync(modules)
process.chdir(work)

t.end()
})

test('installing package with links', function (t) {
common.npm(
[
'--loglevel', 'silent',
'install', pkg
],
EXEC_OPTS,
function (err, code) {
t.ifError(err, 'install ran to completion without error')
t.notOk(code, 'npm install exited with code 0')

t.ok(
existsSync(path.join(modules, 'package', 'package.json')),
'package installed'
)
t.ok(existsSync(path.join(modules, '.bin')), 'binary link directory exists')
t.ok(existsSync(path.join(modules, 'package', 'node_modules', '.bin')),
'nested binary link directory exists')

t.end()
}
)
})

test('uninstalling package with links', function (t) {
common.npm(
[
'--loglevel', 'silent',
'uninstall', 'package'
],
EXEC_OPTS,
function (err, code) {
t.ifError(err, 'uninstall ran to completion without error')
t.notOk(code, 'npm uninstall exited with code 0')

t.notOk(existsSync(path.join(modules, 'package')),
'package directory no longer exists')
t.notOk(existsSync(path.join(modules, 'package', 'node_modules', '.bin')),
'nested binary link directory no longer exists')

t.end()
}
)
})

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

function cleanup () {
process.chdir(osenv.tmpdir())
rimraf.sync(dep)
rimraf.sync(work)
rimraf.sync(pkg)
}

0 comments on commit ea331c8

Please sign in to comment.