From 69c88cdd3a189ad8385cc4338e7defbb2c392209 Mon Sep 17 00:00:00 2001 From: Lars Willighagen Date: Thu, 13 Dec 2018 23:34:53 +0100 Subject: [PATCH 1/4] dedupe: get deps from shrinkwrap See https://npm.community/t/3807 --- lib/dedupe.js | 4 ++ test/tap/dedupe-optional.js | 82 +++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 test/tap/dedupe-optional.js diff --git a/lib/dedupe.js b/lib/dedupe.js index 325faeaabcd43..9ed17f0f6b988 100644 --- a/lib/dedupe.js +++ b/lib/dedupe.js @@ -59,6 +59,10 @@ Deduper.prototype.loadIdealTree = function (cb) { [this, this.cloneCurrentTreeToIdealTree], [this, this.finishTracker, 'cloneCurrentTree'], + [this.newTracker(this.progress.loadIdealTree, 'loadIdealTree:loadShrinkwrap')], + [this, this.loadShrinkwrap], + [this, this.finishTracker, 'loadIdealTree:loadShrinkwrap'], + [this.newTracker(this.progress.loadIdealTree, 'loadAllDepsIntoIdealTree', 10)], [ function (next) { loadExtraneous(self.idealTree, self.progress.loadAllDepsIntoIdealTree, next) diff --git a/test/tap/dedupe-optional.js b/test/tap/dedupe-optional.js new file mode 100644 index 0000000000000..9e07c05187616 --- /dev/null +++ b/test/tap/dedupe-optional.js @@ -0,0 +1,82 @@ +var fs = require('graceful-fs') +var path = require('path') + +var mkdirp = require('mkdirp') +var mr = require('npm-registry-mock') +var rimraf = require('rimraf') +var test = require('tap').test + +var common = require('../common-tap.js') +var server + +var pkg = path.join(__dirname, 'dedupe-optional') + +var EXEC_OPTS = { cwd: pkg } + +var json = { + author: 'Dedupe tester', + name: 'dedupe', + version: '0.0.0', + dependencies: { + clean: '2.1.6' + } +} + +var shrinkwrap = { + name: 'dedupe', + version: '0.0.0', + dependencies: { + clean: { + version: '2.1.6' + } + } +} + +test('setup', function (t) { + t.comment('test for https://npm.community/t/3807') + setup(function () { + t.end() + }) +}) + +test('dedupe keeps uninstalled packages in package-lock.json', function (t) { + common.npm(['dedupe'], EXEC_OPTS, function (err, code) { + t.ifError(err, 'successfully deduped against previous install') + t.notOk(code, 'npm dedupe exited with code') + + var shrinkwrap = JSON.parse(fs.readFileSync('package-lock.json', 'utf8')) + + t.ok(shrinkwrap.dependencies, 'npm dedupe kept packages') + t.end() + }) +}) + +test('cleanup', function (t) { + server.close() + cleanup() + + t.end() +}) + +function cleanup () { + rimraf.sync(pkg) +} + +function setup (cb) { + cleanup() + mkdirp.sync(pkg) + fs.writeFileSync( + path.join(pkg, 'package.json'), + JSON.stringify(json, null, 2) + ) + fs.writeFileSync( + path.join(pkg, 'package-lock.json'), + JSON.stringify(shrinkwrap, null, 2) + ) + process.chdir(pkg) + + mr({ port: common.port }, function (er, s) { + server = s + cb() + }) +} From 624b131bad699c480f0986c86aad55d20837abe2 Mon Sep 17 00:00:00 2001 From: Lars Willighagen Date: Tue, 8 Jan 2019 19:38:57 +0100 Subject: [PATCH 2/4] test: update dedupe-optional with maketest --- test/tap/dedupe-optional.js | 126 ++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 56 deletions(-) diff --git a/test/tap/dedupe-optional.js b/test/tap/dedupe-optional.js index 9e07c05187616..1678c950e86e6 100644 --- a/test/tap/dedupe-optional.js +++ b/test/tap/dedupe-optional.js @@ -1,82 +1,96 @@ -var fs = require('graceful-fs') -var path = require('path') +'use strict' +const fs = require('fs') +const path = require('path') +const test = require('tap').test +const mr = require('npm-registry-mock') +const Tacks = require('tacks') +const File = Tacks.File +const Symlink = Tacks.Symlink +const Dir = Tacks.Dir +const common = require('../common-tap.js') -var mkdirp = require('mkdirp') -var mr = require('npm-registry-mock') -var rimraf = require('rimraf') -var test = require('tap').test +const basedir = path.join(__dirname, path.basename(__filename, '.js')) +const testdir = path.join(basedir, 'testdir') +const cachedir = path.join(basedir, 'cache') +const globaldir = path.join(basedir, 'global') +const tmpdir = path.join(basedir, 'tmp') -var common = require('../common-tap.js') -var server - -var pkg = path.join(__dirname, 'dedupe-optional') - -var EXEC_OPTS = { cwd: pkg } - -var json = { - author: 'Dedupe tester', - name: 'dedupe', - version: '0.0.0', - dependencies: { - clean: '2.1.6' - } +const conf = { + cwd: testdir, + env: Object.assign({}, process.env, { + npm_config_cache: cachedir, + npm_config_tmp: tmpdir, + npm_config_prefix: globaldir, + npm_config_registry: common.registry, + npm_config_loglevel: 'warn' + }) } -var shrinkwrap = { - name: 'dedupe', - version: '0.0.0', - dependencies: { - clean: { - version: '2.1.6' - } - } -} +let server +const fixture = new Tacks(Dir({ + cache: Dir(), + global: Dir(), + tmp: Dir(), + testdir: Dir({ + 'package-lock.json': File({ + name: 'dedupe-optional', + version: '1.0.0', + lockfileVersion: 1, + requires: true, + dependencies: { + async: { + version: '0.9.2', + resolved: 'https://registry.npmjs.org/async/-/async-0.9.2.tgz', + integrity: 'sha1-rqdNXmHB+JlhO/ZL2mbUx48v0X0=', + optional: true + } + } + }), + 'package.json': File({ + name: 'dedupe-optional', + version: '1.0.0', + optionalDependencies: { + async: '*' + } + }) + }) +})) test('setup', function (t) { - t.comment('test for https://npm.community/t/3807') - setup(function () { - t.end() + setup() + mr({port: common.port, throwOnUnmatched: true}, function (err, s) { + if (err) throw err + server = s + t.done() }) }) test('dedupe keeps uninstalled packages in package-lock.json', function (t) { - common.npm(['dedupe'], EXEC_OPTS, function (err, code) { - t.ifError(err, 'successfully deduped against previous install') - t.notOk(code, 'npm dedupe exited with code') + t.comment('test for https://npm.community/t/3807') + common.npm(['dedupe'], conf, function (err, code) { + if (err) throw err + t.is(code, 0, 'command ran ok') - var shrinkwrap = JSON.parse(fs.readFileSync('package-lock.json', 'utf8')) + const shrinkwrap = JSON.parse( + fs.readFileSync( + path.join(testdir, 'package-lock.json'), 'utf8')) t.ok(shrinkwrap.dependencies, 'npm dedupe kept packages') - t.end() + t.done() }) }) test('cleanup', function (t) { server.close() cleanup() - - t.end() + t.done() }) function cleanup () { - rimraf.sync(pkg) + fixture.remove(basedir) } -function setup (cb) { +function setup () { cleanup() - mkdirp.sync(pkg) - fs.writeFileSync( - path.join(pkg, 'package.json'), - JSON.stringify(json, null, 2) - ) - fs.writeFileSync( - path.join(pkg, 'package-lock.json'), - JSON.stringify(shrinkwrap, null, 2) - ) - process.chdir(pkg) - - mr({ port: common.port }, function (er, s) { - server = s - cb() - }) + fixture.create(basedir) } From 2705383f66f4b1bb3236fe3e98be3e1535cf190e Mon Sep 17 00:00:00 2001 From: Lars Willighagen Date: Wed, 16 Jan 2019 23:08:06 +0100 Subject: [PATCH 3/4] doc: document new dedupe behavior --- doc/cli/npm-dedupe.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/cli/npm-dedupe.md b/doc/cli/npm-dedupe.md index d68832145f0a5..f193660da5630 100644 --- a/doc/cli/npm-dedupe.md +++ b/doc/cli/npm-dedupe.md @@ -43,10 +43,10 @@ be deleted. Arguments are ignored. Dedupe always acts on the entire tree. -Modules +### Modules -Note that this operation transforms the dependency tree, but will never -result in new modules being installed. +Note that this operation automatically fixes broken installs, which can sometimes +cause the installation of new packages, just like `npm-install(1)` does. ## SEE ALSO From d898fab3742a933148f36c844adae974e6fa4e5f Mon Sep 17 00:00:00 2001 From: Lars Willighagen Date: Sun, 20 Jan 2019 13:57:51 +0100 Subject: [PATCH 4/4] test: remove unused Symlink --- test/tap/dedupe-optional.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/tap/dedupe-optional.js b/test/tap/dedupe-optional.js index 1678c950e86e6..a4a7a4fe72f26 100644 --- a/test/tap/dedupe-optional.js +++ b/test/tap/dedupe-optional.js @@ -5,7 +5,6 @@ const test = require('tap').test const mr = require('npm-registry-mock') const Tacks = require('tacks') const File = Tacks.File -const Symlink = Tacks.Symlink const Dir = Tacks.Dir const common = require('../common-tap.js')