Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Commit

Permalink
Use platform independent path separators in dedup
Browse files Browse the repository at this point in the history
`npm dedupe` fails silently on windows due to incorrect
path splitting and joining without these changes.
  • Loading branch information
mcolyer authored and robertkowalski committed Nov 24, 2013
1 parent ff74687 commit 7677de4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 5 deletions.
12 changes: 7 additions & 5 deletions lib/dedupe.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,16 @@ function dedupe_ (dir, filter, unavoidable, dryrun, silent, cb) {
// a=/path/to/node_modules/foo/node_modules/bar
// b=/path/to/node_modules/elk/node_modules/bar
// ==/path/to/node_modules/bar
a = a.split(/\/node_modules\//)
b = b.split(/\/node_modules\//)
var nmReg = new RegExp("\\" + path.sep + "node_modules\\" + path.sep)
a = a.split(nmReg)
b = b.split(nmReg)
var name = a.pop()
b.pop()
// find the longest chain that both A and B share.
// then push the name back on it, and join by /node_modules/
var res = []
for (var i = 0, al = a.length, bl = b.length; i < al && i < bl && a[i] === b[i]; i++);
return a.slice(0, i).concat(name).join("/node_modules/")
return a.slice(0, i).concat(name).join(path.sep + "node_modules" + path.sep)
}) : undefined

return [item[0], { item: item
Expand Down Expand Up @@ -192,9 +193,10 @@ function installAndRetest (set, filter, dir, unavoidable, silent, cb) {
// where is /path/to/node_modules/foo/node_modules/bar
// for package "bar", but we need it to be just
// /path/to/node_modules/foo
where = where.split(/\/node_modules\//)
var nmReg = new RegExp("\\" + path.sep + "node_modules\\" + path.sep)
where = where.split(nmReg)
where.pop()
where = where.join("/node_modules/")
where = where.join(path.sep + "node_modules" + path.sep)
remove.push.apply(remove, others)

return npm.commands.install(where, what, cb)
Expand Down
29 changes: 29 additions & 0 deletions test/tap/dedupe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
var test = require("tap").test
, fs = require("fs")
, path = require("path")
, existsSync = fs.existsSync || path.existsSync
, npm = require("../../")
, rimraf = require("rimraf")

test("dedupe finds the common module and moves it up one level", function (t) {
t.plan(1)

setup(function () {
npm.install(".", function (err) {
if (err) return t.fail(err)
npm.dedupe(function(err) {
if (err) return t.fail(err)
t.ok(existsSync(path.join(__dirname, "dedupe", "node_modules", "minimist")))
})
})
})
})

function setup (cb) {
process.chdir(path.join(__dirname, "dedupe"))
npm.load(function () {
rimraf.sync(path.join(__dirname, "dedupe", "node_modules"))
fs.mkdirSync(path.join(__dirname, "dedupe", "node_modules"))
cb()
})
}
9 changes: 9 additions & 0 deletions test/tap/dedupe/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"author": "Dedupe tester",
"name": "dedupe",
"version": "0.0.0",
"dependencies": {
"optimist": "0.6.0",
"clean": "2.1.6"
}
}

0 comments on commit 7677de4

Please sign in to comment.