From e338460aba07aa8d8d8935649b459abfbfe1ff29 Mon Sep 17 00:00:00 2001
From: Philip Harrison <philip@mailharrison.com>
Date: Tue, 16 Apr 2019 12:01:59 +0100
Subject: [PATCH] Use Installer when packaging git dependencies

This fixes a couple of config issues we've found when installing
packages in `dry-run` mode to only get the lockfile changes.

When setting `dry-run` or `ignore-scripts`, the setting isn't
passed through when installing git dependencies meaning the `prepare`
script is always run.

We've also found an issue with proxying existing npm config to the cli
when a large `cafile` has been set, as this is expanded into the `ca`
config key with the full string contents of the ca file. If the ca file
is huge this will blow up on linux with an E2BIG exception (Argument
list too long). Increasing the limit requires kernel recompilation so
isn't really an option.

The current implementation requires `prepublish` to be fully deprecated,
is this something you are planning? Otherwise I'm happy to find another
way of passing this config through the installer.
---
 lib/pack.js             | 89 +++++++----------------------------------
 test/tap/git-prepare.js |  2 +-
 2 files changed, 16 insertions(+), 75 deletions(-)

diff --git a/lib/pack.js b/lib/pack.js
index 78e5bfd174d7b..e41836fe4324d 100644
--- a/lib/pack.js
+++ b/lib/pack.js
@@ -9,12 +9,12 @@ const BB = require('bluebird')
 const byteSize = require('byte-size')
 const cacache = require('cacache')
 const columnify = require('columnify')
-const cp = require('child_process')
 const deprCheck = require('./utils/depr-check')
 const fpm = require('./fetch-package-metadata')
 const fs = require('graceful-fs')
 const install = require('./install')
 const lifecycle = BB.promisify(require('./utils/lifecycle'))
+const noProgressTillDone = require('./utils/no-progress-while-running').tillDone
 const log = require('npmlog')
 const move = require('move-concurrently')
 const npm = require('./npm')
@@ -265,89 +265,30 @@ function getContents (pkg, target, filename, silent) {
     })
 }
 
-const PASSTHROUGH_OPTS = [
-  'always-auth',
-  'auth-type',
-  'ca',
-  'cafile',
-  'cert',
-  'git',
-  'local-address',
-  'maxsockets',
-  'offline',
-  'prefer-offline',
-  'prefer-online',
-  'proxy',
-  'https-proxy',
-  'registry',
-  'send-metrics',
-  'sso-poll-frequency',
-  'sso-type',
-  'strict-ssl'
-]
-
 module.exports.packGitDep = packGitDep
 function packGitDep (manifest, dir) {
   const stream = new PassThrough()
   readJson(path.join(dir, 'package.json')).then((pkg) => {
     if (pkg.scripts && pkg.scripts.prepare) {
       log.verbose('prepareGitDep', `${manifest._spec}: installing devDeps and running prepare script.`)
-      const cliArgs = PASSTHROUGH_OPTS.reduce((acc, opt) => {
-        if (npm.config.get(opt, 'cli') != null) {
-          acc.push(`--${opt}=${npm.config.get(opt)}`)
-        }
-        return acc
-      }, [])
-      const child = cp.spawn(process.env.NODE || process.execPath, [
-        require.resolve('../bin/npm-cli.js'),
-        'install',
-        '--dev',
-        '--prod',
-        '--ignore-prepublish',
-        '--no-progress',
-        '--no-save'
-      ].concat(cliArgs), {
-        cwd: dir,
-        env: process.env
-      })
-      let errData = []
-      let errDataLen = 0
-      let outData = []
-      let outDataLen = 0
-      child.stdout.on('data', (data) => {
-        outData.push(data)
-        outDataLen += data.length
-        log.gauge.pulse('preparing git package')
-      })
-      child.stderr.on('data', (data) => {
-        errData.push(data)
-        errDataLen += data.length
-        log.gauge.pulse('preparing git package')
-      })
-      return BB.fromNode((cb) => {
-        child.on('error', cb)
-        child.on('exit', (code, signal) => {
-          if (code > 0) {
-            const err = new Error(`${signal}: npm exited with code ${code} while attempting to build ${manifest._requested}. Clone the repository manually and run 'npm install' in it for more information.`)
-            err.code = code
-            err.signal = signal
-            cb(err)
-          } else {
-            cb()
-          }
-        })
-      }).then(() => {
-        if (outDataLen > 0) log.silly('prepareGitDep', '1>', Buffer.concat(outData, outDataLen).toString())
-        if (errDataLen > 0) log.silly('prepareGitDep', '2>', Buffer.concat(errData, errDataLen).toString())
-      }, (err) => {
-        if (outDataLen > 0) log.error('prepareGitDep', '1>', Buffer.concat(outData, outDataLen).toString())
-        if (errDataLen > 0) log.error('prepareGitDep', '2>', Buffer.concat(errData, errDataLen).toString())
-        throw err
+
+      const dryrun = !!npm.config.get('dry-run')
+      const args = []
+      const opts = {
+        dev: true,
+        prod: true
+      }
+      const installer = new install.Installer(dir, dryrun, args, opts)
+      // No-op changes to manifest/lockfile (--no-save)
+      installer.saveToDependencies = cb => cb()
+
+      return BB.fromNode(cb => {
+        installer.run(noProgressTillDone(cb))
       })
     }
   }).then(() => {
     return readJson(path.join(dir, 'package.json'))
-  }).then((pkg) => {
+  }).then(() => {
     return cacache.tmp.withTmp(npm.tmp, {
       tmpPrefix: 'pacote-packing'
     }, (tmp) => {
diff --git a/test/tap/git-prepare.js b/test/tap/git-prepare.js
index 1a61056b4beda..050560e0af164 100644
--- a/test/tap/git-prepare.js
+++ b/test/tap/git-prepare.js
@@ -47,7 +47,7 @@ const fixture = new Tacks(Dir({
         version: '1.0.3',
         main: 'dobuild.js',
         scripts: {
-          'prepublish': 'exit 123',
+          'prepublishOnly': 'exit 123',
           'prepare': 'writer build-artifact'
         },
         devDependencies: {