feat: install is completely promisified

Due to a posibility that callbacks are called twice on error, the
install function is now internally completely promisified.

Author: imatlopez

lib/install.js

@@ -4,60 +4,42 @@ const fs = require('graceful-fs')
 const os = require('os')
 const tar = require('tar')
 const path = require('path')
+const util = require('util')
 const stream = require('stream')
 const crypto = require('crypto')
 const log = require('npmlog')
 const semver = require('semver')
 const fetch = require('make-fetch-happen')
 const processRelease = require('./process-release')
 const win = process.platform === 'win32'
-const streamPipeline = require('util').promisify(stream.pipeline)
+const streamPipeline = util.promisify(stream.pipeline)
 
 /**
  * @param {typeof import('graceful-fs')} fs
  */
 
-function install (fs, gyp, argv, callback) {
+async function install (fs, gyp, argv) {
   const release = processRelease(argv, gyp, process.version, process.release)
 
-  // ensure no double-callbacks happen
-  function cb (err) {
-    if (cb.done) {
-      return
-    }
-    cb.done = true
-    if (err) {
-      log.warn('install', 'got an error, rolling back install')
-      // roll-back the install if anything went wrong
-      gyp.commands.remove([release.versionDir], function () {
-        callback(err)
-      })
-    } else {
-      callback(null, release.version)
-    }
-  }
-
   // Determine which node dev files version we are installing
   log.verbose('install', 'input version string %j', release.version)
 
   if (!release.semver) {
     // could not parse the version string with semver
-    return callback(new Error('Invalid version number: ' + release.version))
+    throw new Error('Invalid version number: ' + release.version)
   }
 
   if (semver.lt(release.version, '0.8.0')) {
-    return callback(new Error('Minimum target version is `0.8.0` or greater. Got: ' + release.version))
+    throw new Error('Minimum target version is `0.8.0` or greater. Got: ' + release.version)
   }
 
   // 0.x.y-pre versions are not published yet and cannot be installed. Bail.
   if (release.semver.prerelease[0] === 'pre') {
     log.verbose('detected "pre" node version', release.version)
-    if (gyp.opts.nodedir) {
-      log.verbose('--nodedir flag was passed; skipping install', gyp.opts.nodedir)
-      callback()
-    } else {
-      callback(new Error('"pre" versions of node cannot be installed, use the --nodedir flag instead'))
+    if (!gyp.opts.nodedir) {
+      throw new Error('"pre" versions of node cannot be installed, use the --nodedir flag instead')
     }
+    log.verbose('--nodedir flag was passed; skipping install', gyp.opts.nodedir)
     return
   }
 
@@ -71,55 +53,48 @@ function install (fs, gyp, argv, callback) {
   // check if it is already installed, and only install when needed
   if (gyp.opts.ensure) {
     log.verbose('install', '--ensure was passed, so won\'t reinstall if already installed')
-    fs.stat(devDir, function (err) {
-      if (err) {
-        if (err.code === 'ENOENT') {
-          log.verbose('install', 'version not already installed, continuing with install', release.version)
-          go().then(cb, cb)
-        } else if (err.code === 'EACCES') {
-          eaccesFallback(err)
-        } else {
-          cb(err)
+    try {
+      await fs.promises.stat(devDir)
+    } catch (err) {
+      if (err.code === 'ENOENT') {
+        log.verbose('install', 'version not already installed, continuing with install', release.version)
+        try {
+          return await go()
+        } catch (err) {
+          return rollback(err)
         }
-        return
+      } else if (err.code === 'EACCES') {
+        return eaccesFallback(err)
       }
-      log.verbose('install', 'version is already installed, need to check "installVersion"')
-      const installVersionFile = path.resolve(devDir, 'installVersion')
-      fs.readFile(installVersionFile, 'ascii', (err, ver) => {
-        if (err && err.code !== 'ENOENT') {
-          return cb(err)
-        }
-        const installVersion = parseInt(ver, 10) || 0
-        log.verbose('got "installVersion"', installVersion)
-        log.verbose('needs "installVersion"', gyp.package.installVersion)
-        if (installVersion < gyp.package.installVersion) {
-          log.verbose('install', 'version is no good; reinstalling')
-          go().then(cb, cb)
-        } else {
-          log.verbose('install', 'version is good')
-          cb()
-        }
-      })
-    })
-  } else {
-    go().then(cb, cb)
-  }
-
-  class ShaSum extends stream.Transform {
-    constructor (callback) {
-      super()
-      this._callback = callback
-      this._digester = crypto.createHash('sha256')
+      throw err
     }
-
-    _transform (chunk, _, callback) {
-      this._digester.update(chunk)
-      callback(null, chunk)
+    log.verbose('install', 'version is already installed, need to check "installVersion"')
+    const installVersionFile = path.resolve(devDir, 'installVersion')
+    let installVersion = 0
+    try {
+      const ver = await fs.promises.readFile(installVersionFile, 'ascii')
+      installVersion = parseInt(ver, 10) || 0
+    } catch (err) {
+      if (err.code !== 'ENOENT') {
+        throw err
+      }
     }
-
-    _flush (callback) {
-      this._callback(null, this._digester.digest('hex'))
-      callback()
+    log.verbose('got "installVersion"', installVersion)
+    log.verbose('needs "installVersion"', gyp.package.installVersion)
+    if (installVersion < gyp.package.installVersion) {
+      log.verbose('install', 'version is no good; reinstalling')
+      try {
+        return await go()
+      } catch (err) {
+        return rollback(err)
+      }
+    }
+    log.verbose('install', 'version is good')
+  } else {
+    try {
+      return await go()
+    } catch (err) {
+      return rollback(err)
     }
   }
 
@@ -135,8 +110,7 @@ function install (fs, gyp, argv, callback) {
       }
     } catch (err) {
       if (err.code === 'EACCES') {
-        eaccesFallback(err)
-        return
+        return eaccesFallback(err)
       }
 
       throw err
@@ -173,7 +147,7 @@ function install (fs, gyp, argv, callback) {
       })
     } else {
       try {
-        const res = await download(gyp, process.env, release.tarballUrl)
+        const res = await download(gyp, release.tarballUrl)
 
         if (res.status !== 200) {
           throw new Error(`${res.status} response downloading ${release.tarballUrl}`)
@@ -234,7 +208,7 @@ function install (fs, gyp, argv, callback) {
       log.verbose('check download content checksum, need to download `SHASUMS256.txt`...')
       log.verbose('checksum url', release.shasumsUrl)
 
-      const res = await download(gyp, process.env, release.shasumsUrl)
+      const res = await download(gyp, release.shasumsUrl)
 
       if (res.status !== 200) {
         throw new Error(`${res.status}  status code downloading checksum`)
@@ -268,7 +242,7 @@ function install (fs, gyp, argv, callback) {
         await fs.promises.mkdir(dir, { recursive: true })
         log.verbose('streaming', name, 'to:', targetLibPath)
 
-        const res = await download(gyp, process.env, libUrl)
+        const res = await download(gyp, libUrl)
 
         if (res.status === 403 || res.status === 404) {
           if (arch === 'arm64') {
@@ -304,6 +278,13 @@ function install (fs, gyp, argv, callback) {
     return extname === '.h' || extname === '.gypi'
   }
 
+  async function rollback (err) {
+    log.warn('install', 'got an error, rolling back install')
+    // roll-back the install if anything went wrong
+    await util.promisify(gyp.commands.remove)([release.versionDir])
+    throw err
+  }
+
   /**
    * The EACCES fallback is a workaround for npm's `sudo` behavior, where
    * it drops the permissions before invoking any child processes (like
@@ -313,10 +294,10 @@ function install (fs, gyp, argv, callback) {
    * the compilation will succeed...
    */
 
-  function eaccesFallback (err) {
+  async function eaccesFallback (err) {
     const noretry = '--node_gyp_internal_noretry'
     if (argv.indexOf(noretry) !== -1) {
-      return cb(err)
+      throw err
     }
     const tmpdir = os.tmpdir()
     gyp.devDir = path.resolve(tmpdir, '.node-gyp')
@@ -331,11 +312,29 @@ function install (fs, gyp, argv, callback) {
       log.verbose('tmpdir == cwd', 'automatically will remove dev files after to save disk space')
       gyp.todo.push({ name: 'remove', args: argv })
     }
-    gyp.commands.install([noretry].concat(argv), cb)
+    return util.promisify(gyp.commands.install)([noretry].concat(argv))
+  }
+}
+
+class ShaSum extends stream.Transform {
+  constructor (callback) {
+    super()
+    this._callback = callback
+    this._digester = crypto.createHash('sha256')
+  }
+
+  _transform (chunk, _, callback) {
+    this._digester.update(chunk)
+    callback(null, chunk)
+  }
+
+  _flush (callback) {
+    this._callback(null, this._digester.digest('hex'))
+    callback()
   }
 }
 
-async function download (gyp, env, url) {
+async function download (gyp, url) {
   log.http('GET', url)
 
   const requestOpts = {
@@ -367,11 +366,11 @@ async function readCAFile (filename) {
 }
 
 module.exports = function (gyp, argv, callback) {
-  return install(fs, gyp, argv, callback)
+  install(fs, gyp, argv).then(callback, callback)
 }
 module.exports.test = {
-  download: download,
-  install: install,
-  readCAFile: readCAFile
+  download,
+  install,
+  readCAFile
 }
 module.exports.usage = 'Install node development files for the specified node version.'