From 867642942bec69bb9ab71cff1914fb6a9fe67de8 Mon Sep 17 00:00:00 2001 From: Lars Willighagen Date: Wed, 14 Nov 2018 22:12:54 +0100 Subject: [PATCH] fix(packageRelativePath): fix 'where' for file deps Fix 'where' for file deps. It makes more sense for the 'where' to be the directory the file is in (and was possibly built in) than it being the file itself, having no use whatsoever. See https://npm.community/t/3364 PR-URL: https://github.com/npm/cli/pull/142 Credit: @larsgw Close: #142 Reviewed-by: @mikemimik --- lib/install/deps.js | 13 ++-- test/tap/install-dep-classification.js | 45 ++++++++---- test/tap/install-local-from-local.js | 94 ++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 16 deletions(-) create mode 100644 test/tap/install-local-from-local.js diff --git a/lib/install/deps.js b/lib/install/deps.js index dfe30b6c0f38c..3d8b333c64441 100644 --- a/lib/install/deps.js +++ b/lib/install/deps.js @@ -203,10 +203,15 @@ function removeObsoleteDep (child, log) { function packageRelativePath (tree) { if (!tree) return '' var requested = tree.package._requested || {} - var isLocal = requested.type === 'directory' || requested.type === 'file' - return isLocal ? requested.fetchSpec - : (tree.isLink || tree.isInLink) && !preserveSymlinks() ? tree.realpath - : tree.path + if (requested.type === 'directory') { + return requested.fetchSpec + } else if (requested.type === 'file') { + return path.dirname(requested.fetchSpec) + } else if ((tree.isLink || tree.isInLink) && !preserveSymlinks()) { + return tree.realpath + } else { + return tree.path + } } function matchingDep (tree, name) { diff --git a/test/tap/install-dep-classification.js b/test/tap/install-dep-classification.js index 2775c367b0002..257fc99fc123f 100644 --- a/test/tap/install-dep-classification.js +++ b/test/tap/install-dep-classification.js @@ -23,18 +23,39 @@ const env = common.newEnv().extend({ npm_config_loglevel: 'error' }) +/** + * NOTE: Tarball Fixtures + * They contain package.json files with dependencies like the following: + * a-1.0.0.tgz: package/package.json + * { + * "name":"a", + * "version":"1.0.0", + * "dependencies":{ + * "b":"./b-1.0.0.tgz" + * } + * } + * example-1.0.0.tgz: package/package.json + * { + * "name":"example", + * "version":"1.0.0", + * "dependencies":{ + * "a":"./a-1.0.0.tgz", + * "b":"./b-1.0.0.tgz" + * } + * } + */ const fixture = new Tacks(Dir({ cache: Dir(), global: Dir(), tmp: Dir(), testdir: Dir({ 'a-1.0.0.tgz': File(Buffer.from( - '1f8b0800000000000003edcfc10e82300c0660ce3ec5d2b38e4eb71d789b' + - '010d41e358187890f0ee56493c71319218937d977feb9aa50daebab886f2' + - 'b0a43cc7ce671b4344abb558ab3f2934223b198b4a598bdcc707a38f9c5b' + - '0fb2668c83eb79946fff597611effc131378772528c0c11e6ed4c7b6f37c' + - '53122572a5a640be265fb514a198a0e43729f3f2f06a9043738779defd7a' + - '89244992e4630fd69e456800080000', + '1f8b0800000000000003edcfc10a83300c06e09df71492f35653567bf06d' + + 'a2067163b558b7c3c4775f54f0e4654c18837e973f4da0249eca1bd59cfa' + + '25d535b4eeb03344b4c6245bfd8946995d328b5a5b3bd55264464beebdc8' + + '9647e8a99355befd67b92559f34f0ce0e8ce9003c1099edc85a675f2d20a' + + '154aa762cfae6257361c201fa090994a8bf33c577dfd82713cfefa86288a' + + 'a2e8736f68a0ff4400080000', 'hex' )), 'b-1.0.0.tgz': File(Buffer.from( @@ -55,12 +76,12 @@ const fixture = new Tacks(Dir({ }) }), 'example-1.0.0.tgz': File(Buffer.from( - '1f8b0800000000000003ed8fc10ac2300c8677f62946cedaa5d8f5e0db64' + - '5b1853d795758a38f6ee4607e261370722f4bbfce5cb4f493c9527aa39f3' + - '73aa63e85cb23288688d4997fc136d304df6b945adad45e9c923375a72ed' + - '4596b884817a59e5db7fe65bd277fe0923386a190ec0376afd99610b57ee' + - '43d339715aa14231157b7615bbb2e100871148664a65b47b15d450dfa554' + - 'ccb2f890d3b4f9f57d9148241259e60112d8208a00080000', + '1f8b0800000000000003ed8fc10a83300c863def2924e7ada6587bf06daa' + + '06719bb55837c6c4775fa6307670a70963d0ef92f02584fcce94275353e2' + + '962a8ebeb3d1c620a2562a5ef34f64aae328cd344aa935f21e379962875b' + + '3fb2c6c50fa6e757bebdb364895ff54f18c19a962007ba99d69d09f670a5' + + 'de379d6527050a645391235b912d1bf2908f607826127398e762a8efbc53' + + 'ccae7873d3b4fb75ba402010087ce2014747c9d500080000', 'hex' )), optional: Dir({ diff --git a/test/tap/install-local-from-local.js b/test/tap/install-local-from-local.js new file mode 100644 index 0000000000000..ec53c74a39206 --- /dev/null +++ b/test/tap/install-local-from-local.js @@ -0,0 +1,94 @@ +'use strict' +var path = require('path') +var fs = require('graceful-fs') +var test = require('tap').test +var common = require('../common-tap.js') +var Tacks = require('tacks') +var Dir = Tacks.Dir +var File = Tacks.File + +var testdir = path.join(__dirname, path.basename(__filename, '.js')) +var cwd = path.join(testdir, '3') + +/** + * NOTE: Tarball Fixtures + * They contain package.json files with dependencies like the following: + * 1-1.0.0.tgz: package/package.json + * { + * "name":"1", + * "version":"1.0.0" + * } + * 2-1.0.0.tgz: package/package.json + * { + * "name":"2", + * "version":"1.0.0", + * "dependencies":{ + * "1":"file:../1/1-1.0.0.tgz" + * } + * } + */ +var fixture = new Tacks(Dir({ + '1': Dir({ + '1-1.0.0.tgz': File(Buffer.from( + '1f8b08000000000000032b484cce4e4c4fd52f80d07a59c5f9790c540606' + + '06066626260ad8c4c1c0d85c81c1d8d4ccc0d0d0cccc00a80ec830353103' + + 'd2d4760836505a5c925804740aa5e640bca200a78708a856ca4bcc4d55b2' + + '523254d2512a4b2d2acecccf03f1f40cf40c946ab906da79a360148c8251' + + '300a6804007849dfdf00080000', + 'hex' + )) + }), + '2': Dir({ + '2-1.0.0.tgz': File(Buffer.from( + '1f8b0800000000000003ed8f3d0e83300c8599394594b90d36840cdc2602' + + '17d19f80087468c5ddeb14a9135b91aa4af996e73c3f47f660eb8b6d291b' + + '565567dfbb646700c0682db6fc00ea5c24456900d118e01c17a52e58f75e' + + '648bd94f76e455befd67bd457cf44f78a64248676f242b21737908cf3b8d' + + 'beeb5d70508182d56d6820d790ab3bf2dc0a83ec62489dba2b554a6598e1' + + 'f13da1a6f62139b0a44bfaeb0b23914824b2c50b8b5b623100080000', + 'hex' + )) + }), + '3': Dir({ + 'package.json': File({ + name: '3', + version: '1.0.0', + dependencies: { + '2': '../2/2-1.0.0.tgz' + } + }) + }) +})) + +function setup () { + fixture.create(testdir) +} + +function cleanup () { + fixture.remove(testdir) +} + +test('setup', function (t) { + cleanup() + setup() + t.end() +}) + +test('installing local package with local dependency', function (t) { + common.npm( + ['install'], + {cwd: cwd}, + function (er, code, stdout, stderr) { + t.is(code, 0, 'no error code') + t.is(stderr, '', 'no error output') + t.ok(fs.existsSync(path.join(cwd, 'node_modules', '2')), 'installed direct dep') + t.ok(fs.existsSync(path.join(cwd, 'node_modules', '1')), 'installed indirect dep') + t.end() + } + ) +}) + +test('cleanup', function (t) { + cleanup() + t.end() +})