feat(config): switch to modern, figgy-pudding configuration (#57)

* updates to pacote@9
* switches to figgy-pudding for config handling
* removes config/pacote-opts and config/lifecycle-opts
* all config is in opts itself. `opts.config` is no longer used
* adds support for auth config reading to npmConfig() utility

BREAKING CHANGE: this updates cipm to use pacote@9, which consumes npm-style config objects, not pacoteOpts()-style objects.

Author: zkat

bin/cli.js

@@ -17,11 +17,8 @@ function cliMain () {
   const log = require('npmlog')
   return fromNpm(process.argv)
     .then(c => {
-      log.level = c.get('loglevel')
-      return new Installer({
-        config: c,
-        log
-      })
+      log.level = c.loglevel
+      return new Installer(c.concat({log}))
     })
     .then(cipm => cipm.run())
     .then(

index.js

@@ -5,6 +5,7 @@ const BB = require('bluebird')
 const binLink = require('bin-links')
 const buildLogicalTree = require('npm-logical-tree')
 const extract = require('./lib/extract.js')
+const figgyPudding = require('figgy-pudding')
 const fs = require('graceful-fs')
 const getPrefix = require('find-npm-prefix')
 const lifecycle = require('npm-lifecycle')
@@ -20,10 +21,44 @@ const statAsync = BB.promisify(fs.stat)
 const symlinkAsync = BB.promisify(fs.symlink)
 const writeFileAsync = BB.promisify(fs.writeFile)
 
+const CipmOpts = figgyPudding({
+  also: {},
+  dev: 'development',
+  development: {},
+  force: {},
+  global: {},
+  ignoreScripts: 'ignore-scripts',
+  'ignore-scripts': {},
+  log: {},
+  loglevel: {},
+  only: {},
+  prefix: {},
+  prod: 'production',
+  production: {},
+  Promise: { default: () => BB },
+  umask: {}
+})
+
+const LifecycleOpts = figgyPudding({
+  config: {},
+  'script-shell': {},
+  scriptShell: 'script-shell',
+  'ignore-scripts': {},
+  ignoreScripts: 'ignore-scripts',
+  'ignore-prepublish': {},
+  ignorePrepublish: 'ignore-prepublish',
+  'scripts-prepend-node-path': {},
+  scriptsPrependNodePath: 'scripts-prepend-node-path',
+  'unsafe-perm': {},
+  unsafePerm: 'unsafe-perm',
+  prefix: {},
+  dir: 'prefix',
+  failOk: { default: false }
+}, { other () { return true } })
+
 class Installer {
   constructor (opts) {
-    this.opts = opts
-    this.config = opts.config
+    this.opts = CipmOpts(opts)
 
     // Stats
     this.startTime = Date.now()
@@ -80,17 +115,17 @@ class Installer {
 
   prepare () {
     this.log.info('prepare', 'initializing installer')
-    this.log.level = this.config.get('loglevel')
+    this.log.level = this.opts.loglevel
     this.log.verbose('prepare', 'starting workers')
     extract.startWorkers()
 
     return (
-      this.config.get('prefix') && this.config.get('global')
-        ? BB.resolve(this.config.get('prefix'))
+      this.opts.prefix && this.opts.global
+        ? BB.resolve(this.opts.prefix)
         // There's some Special™ logic around the `--prefix` config when it
         // comes from a config file or env vs when it comes from the CLI
         : process.argv.some(arg => arg.match(/^\s*--prefix\s*/i))
-          ? BB.resolve(this.config.get('prefix'))
+          ? BB.resolve(this.opts.prefix)
           : getPrefix(process.cwd())
     )
       .then(prefix => {
@@ -218,15 +253,15 @@ class Installer {
   checkDepEnv (dep) {
     const includeDev = (
       // Covers --dev and --development (from npm config itself)
-      this.config.get('dev') ||
+      this.opts.dev ||
       (
-        !/^prod(uction)?$/.test(this.config.get('only')) &&
-        !this.config.get('production')
+        !/^prod(uction)?$/.test(this.opts.only) &&
+        !this.opts.production
       ) ||
-      /^dev(elopment)?$/.test(this.config.get('only')) ||
-      /^dev(elopment)?$/.test(this.config.get('also'))
+      /^dev(elopment)?$/.test(this.opts.only) ||
+      /^dev(elopment)?$/.test(this.opts.also)
     )
-    const includeProd = !/^dev(elopment)?$/.test(this.config.get('only'))
+    const includeProd = !/^dev(elopment)?$/.test(this.opts.only)
     return (dep.dev && includeDev) || (!dep.dev && includeProd)
   }
 
@@ -274,14 +309,14 @@ class Installer {
           }
           return readPkgJson(path.join(depPath, 'package.json'))
             .then(pkg => binLink(pkg, depPath, false, {
-              force: this.config.get('force'),
-              ignoreScripts: this.config.get('ignore-scripts'),
+              force: this.opts.force,
+              ignoreScripts: this.opts['ignore-scripts'],
               log: Object.assign({}, this.log, { info: () => {} }),
               name: pkg.name,
               pkgId: pkg.name + '@' + pkg.version,
               prefix: this.prefix,
               prefixes: [this.prefix],
-              umask: this.config.get('umask')
+              umask: this.opts.umask
             }), e => {
               this.log.verbose('buildTree', `error linking ${spec}: ${e.message} ${e.stack}`)
             })
@@ -346,18 +381,21 @@ class Installer {
 
   runScript (stage, pkg, pkgPath) {
     const start = Date.now()
-    if (!this.config.get('ignore-scripts')) {
+    if (!this.opts['ignore-scripts']) {
       // TODO(mikesherov): remove pkg._id when npm-lifecycle no longer relies on it
       pkg._id = pkg.name + '@' + pkg.version
-      const opts = this.config.toLifecycle()
-      return BB.resolve(lifecycle(pkg, stage, pkgPath, opts))
-        .tap(() => { this.timings.scripts += Date.now() - start })
+      return BB.resolve(lifecycle(
+        pkg, stage, pkgPath, LifecycleOpts(this.opts).concat({
+          // TODO: can be removed once npm-lifecycle is updated to modern
+          //       config practices.
+          config: this.opts
+        }))
+      ).tap(() => { this.timings.scripts += Date.now() - start })
     }
     return BB.resolve()
   }
 }
 module.exports = Installer
-module.exports.CipmConfig = require('./lib/config/npm-config.js').CipmConfig
 
 function mark (tree, failed) {
   const liveDeps = new Set()

lib/config/lifecycle-opts.js

@@ -1,40 +1,22 @@
 'use strict'
 
 const BB = require('bluebird')
-const lifecycleOpts = require('./lifecycle-opts.js')
-const pacoteOpts = require('./pacote-opts.js')
-const protoduck = require('protoduck')
-const spawn = require('child_process').spawn
 
-class NpmConfig extends Map {}
+const fs = require('fs')
+const figgyPudding = require('figgy-pudding')
+const ini = require('ini')
+const path = require('path')
+const spawn = require('child_process').spawn
 
-const CipmConfig = protoduck.define({
-  get: [],
-  set: [],
-  toPacote: [],
-  toLifecycle: []
-}, {
-  name: 'CipmConfig'
-})
-module.exports.CipmConfig = CipmConfig
+const readFileAsync = BB.promisify(fs.readFile)
 
-CipmConfig.impl(NpmConfig, {
-  get: Map.prototype.get,
-  set: Map.prototype.set,
-  toPacote (opts) {
-    return pacoteOpts(this, opts)
-  },
-  toLifecycle () {
-    return lifecycleOpts(this)
-  }
+const NpmConfig = figgyPudding({
+  cache: { default: '' },
+  then: {},
+  userconfig: {}
 })
 
-module.exports.fromObject = fromObj
-function fromObj (obj) {
-  const map = new NpmConfig()
-  Object.keys(obj).forEach(k => map.set(k, obj[k]))
-  return map
-}
+module.exports = NpmConfig
 
 module.exports.fromNpm = getNpmConfig
 function getNpmConfig (argv) {
@@ -62,11 +44,41 @@ function getNpmConfig (argv) {
         reject(new Error('`npm` command not found. Please ensure you have npm@5.4.0 or later installed.'))
       } else {
         try {
-          resolve(fromObj(JSON.parse(stdout)))
+          resolve(JSON.parse(stdout))
         } catch (e) {
           reject(new Error('`npm config ls --json` failed to output json. Please ensure you have npm@5.4.0 or later installed.'))
         }
       }
     })
+  }).then(opts => {
+    return BB.all(
+      process.cwd().split(path.sep).reduce((acc, next) => {
+        acc.path = path.join(acc.path, next)
+        acc.promises.push(maybeReadIni(path.join(acc.path, '.npmrc')))
+        acc.promises.push(maybeReadIni(path.join(acc.path, 'npmrc')))
+        return acc
+      }, {
+        path: '',
+        promises: []
+      }).promises.concat(
+        opts.userconfig ? maybeReadIni(opts.userconfig) : {}
+      )
+    ).then(configs => NpmConfig(...configs, opts))
+  }).then(opts => {
+    if (opts.cache) {
+      return opts.concat({ cache: path.join(opts.cache, '_cacache') })
+    } else {
+      return opts
+    }
   })
 }
+
+function maybeReadIni (f) {
+  return readFileAsync(f, 'utf8').catch(err => {
+    if (err.code === 'ENOENT') {
+      return ''
+    } else {
+      throw err
+    }
+  }).then(ini.parse)
+}

lib/config/npm-config.js

@@ -32,15 +32,16 @@
   "dependencies": {
     "bin-links": "^1.1.2",
     "bluebird": "^3.5.1",
+    "figgy-pudding": "^3.5.1",
     "find-npm-prefix": "^1.0.2",
     "graceful-fs": "^4.1.11",
+    "ini": "^1.3.5",
     "lock-verify": "^2.0.2",
     "mkdirp": "^0.5.1",
     "npm-lifecycle": "^2.0.3",
     "npm-logical-tree": "^1.2.1",
     "npm-package-arg": "^6.1.0",
-    "pacote": "^8.1.6",
-    "protoduck": "^5.0.0",
+    "pacote": "^9.1.0",
     "read-package-json": "^2.0.13",
     "rimraf": "^2.6.2",
     "worker-farm": "^1.6.0"

lib/config/pacote-opts.js

@@ -2,7 +2,6 @@
 
 const BB = require('bluebird')
 
-const npmConfig = require('../../lib/config/npm-config.js')
 const npmlog = require('npmlog')
 const fixtureHelper = require('../lib/fixtureHelper.js')
 const fs = BB.promisifyAll(require('fs'))
@@ -37,15 +36,13 @@ const Installer = requireInject('../../index.js', {
 })
 
 function run (moreOpts) {
-  return new Installer({
+  return new Installer(Object.assign({
     log: npmlog,
-    config: npmConfig.fromObject(Object.assign({}, {
-      global: true,
-      prefix,
-      'unsafe-perm': true, // this is the default when running non-root
-      loglevel: process.env.LOGLEVEL || 'error'
-    }, moreOpts || {}))
-  }).run()
+    global: true,
+    prefix,
+    'unsafe-perm': true, // this is the default when running non-root
+    loglevel: process.env.LOGLEVEL || 'error'
+  }, moreOpts || {})).run()
 }
 
 test('throws error when no package.json is found', t => {

package-lock.json

@@ -56,19 +56,12 @@ test('config: errors if npm errors for any reason', t => {
 test('config: parses configs from npm', t => {
   cleanup()
 
-  const expectedConfig = { a: 1, b: 2 }
-
-  config().then(config => {
-    const objConf = {}
-    for (const k of config.keys()) {
-      objConf[k] = config.get(k)
-    }
-    t.deepEqual(objConf, expectedConfig, 'configs match')
-    t.end()
-  })
-
-  child.stdout.emit('data', JSON.stringify(expectedConfig))
+  const c = config()
+  child.stdout.emit('data', JSON.stringify({cache: 'foo'}))
   child.emit('close', 0)
+  return c.then(config => {
+    t.match(config.cache, /^foo[/\\]_cacache$/, 'configs match')
+  })
 })
 
 test('config: cleanup', t => {