feat: do not alter file ownership (npm#59)

BREAKING CHANGE: this module no longer attempts to change file ownership automatically

lib/check-bin.js

@@ -4,9 +4,7 @@ const isWindows = require('./is-windows.js')
 const binTarget = require('./bin-target.js')
 const { resolve, dirname } = require('path')
 const readCmdShim = require('read-cmd-shim')
-const fs = require('fs')
-const { promisify } = require('util')
-const readlink = promisify(fs.readlink)
+const { readlink } = require('fs/promises')
 
 const checkBin = async ({ bin, path, top, global, force }) => {
   // always ok to clobber when forced

lib/fix-bin.js

@@ -1,16 +1,14 @@
 // make sure that bins are executable, and that they don't have
 // windows line-endings on the hashbang line.
-const fs = require('fs')
-const { promisify } = require('util')
+const {
+  chmod,
+  open,
+  readFile,
+} = require('fs/promises')
 
 const execMode = 0o777 & (~process.umask())
 
 const writeFileAtomic = require('write-file-atomic')
-const open = promisify(fs.open)
-const close = promisify(fs.close)
-const read = promisify(fs.read)
-const chmod = promisify(fs.chmod)
-const readFile = promisify(fs.readFile)
 
 const isWindowsHashBang = buf =>
   buf[0] === '#'.charCodeAt(0) &&
@@ -19,16 +17,16 @@ const isWindowsHashBang = buf =>
 
 const isWindowsHashbangFile = file => {
   const FALSE = () => false
-  return open(file, 'r').then(fd => {
+  return open(file, 'r').then(fh => {
     const buf = Buffer.alloc(2048)
-    return read(fd, buf, 0, 2048, 0)
+    return fh.read(buf, 0, 2048, 0)
       .then(
         () => {
           const isWHB = isWindowsHashBang(buf)
-          return close(fd).then(() => isWHB, () => isWHB)
+          return fh.close().then(() => isWHB, () => isWHB)
         },
         // don't leak FD if read() fails
-        () => close(fd).then(FALSE, FALSE)
+        () => fh.close().then(FALSE, FALSE)
       )
   }, FALSE)
 }

lib/link-gently.js

@@ -4,29 +4,25 @@
 // if there's a symlink already, pointing into our pkg, remove it first
 // then create the symlink
 
-const { promisify } = require('util')
 const { resolve, dirname } = require('path')
-const mkdirp = require('mkdirp-infer-owner')
-const fs = require('fs')
-const symlink = promisify(fs.symlink)
-const readlink = promisify(fs.readlink)
-const lstat = promisify(fs.lstat)
+const { lstat, mkdir, readlink, rm, symlink } = require('fs/promises')
 const throwNonEnoent = er => {
   if (er.code !== 'ENOENT') {
     throw er
   }
 }
 
+const rmOpts = {
+  recursive: true,
+  force: true,
+}
+
 // even in --force mode, we never create a link over a link we've
 // already created.  you can have multiple packages in a tree trying
 // to contend for the same bin, or the same manpage listed multiple times,
 // which creates a race condition and nondeterminism.
 const seen = new Set()
 
-// disable glob in our rimraf calls
-const rimraf = promisify(require('rimraf'))
-const rm = path => rimraf(path, { glob: false })
-
 const SKIP = Symbol('skip - missing or already installed')
 const CLOBBER = Symbol('clobber - ours or in forceful mode')
 
@@ -52,7 +48,7 @@ const linkGently = async ({ path, to, from, absFrom, force }) => {
     // exists! maybe clobber if we can
     if (stTo) {
       if (!stTo.isSymbolicLink()) {
-        return force && rm(to).then(() => CLOBBER)
+        return force && rm(to, rmOpts).then(() => CLOBBER)
       }
 
       return readlink(to).then(target => {
@@ -62,14 +58,14 @@ const linkGently = async ({ path, to, from, absFrom, force }) => {
 
         target = resolve(dirname(to), target)
         if (target.indexOf(path) === 0 || force) {
-          return rm(to).then(() => CLOBBER)
+          return rm(to, rmOpts).then(() => CLOBBER)
         }
         // neither skip nor clobber
         return false
       })
     } else {
       // doesn't exist, dir might not either
-      return mkdirp(dirname(to))
+      return mkdir(dirname(to), { recursive: true })
     }
   })
     .then(skipOrClobber => {
@@ -78,7 +74,7 @@ const linkGently = async ({ path, to, from, absFrom, force }) => {
       }
       return symlink(from, to, 'file').catch(er => {
         if (skipOrClobber === CLOBBER || force) {
-          return rm(to).then(() => symlink(from, to, 'file'))
+          return rm(to, rmOpts).then(() => symlink(from, to, 'file'))
         }
         throw er
       }).then(() => true)

lib/shim-bin.js

@@ -1,7 +1,5 @@
-const { promisify } = require('util')
 const { resolve, dirname } = require('path')
-const fs = require('fs')
-const lstat = promisify(fs.lstat)
+const { lstat } = require('fs/promises')
 const throwNonEnoent = er => {
   if (er.code !== 'ENOENT') {
     throw er

package.json

@@ -23,17 +23,14 @@
   ],
   "license": "ISC",
   "dependencies": {
-    "cmd-shim": "^5.0.0",
-    "mkdirp-infer-owner": "^2.0.0",
+    "cmd-shim": "^6.0.0",
     "npm-normalize-package-bin": "^2.0.0",
     "read-cmd-shim": "^3.0.0",
-    "rimraf": "^3.0.0",
     "write-file-atomic": "^4.0.0"
   },
   "devDependencies": {
     "@npmcli/eslint-config": "^3.0.1",
     "@npmcli/template-oss": "4.5.1",
-    "mkdirp": "^1.0.3",
     "require-inject": "^1.4.4",
     "tap": "^16.0.1"
   },

test/fix-bin.js

@@ -29,14 +29,18 @@ t.test('dont fix non-windows hashbang file', async t => {
 
 t.test('failure to read means not a windows hash bang file', async t => {
   const fsMock = {
-    ...fs,
-    read: (a, b, c, d, e, cb) => {
-      fsMock.read = null
-      process.nextTick(() => cb(new Error('witaf')))
+    ...fs.promises,
+    open: async (...args) => {
+      const fh = await fs.promises.open(...args)
+      fh.read = async () => {
+        throw new Error('witaf')
+      }
+      return fh
     },
   }
+
   const mockedFixBin = requireInject('../lib/fix-bin.js', {
-    fs: fsMock,
+    'fs/promises': fsMock,
   })
 
   const dir = t.testdir({
@@ -52,14 +56,17 @@ t.test('failure to read means not a windows hash bang file', async t => {
 
 t.test('failure to close is ignored', async t => {
   const fsMock = {
-    ...fs,
-    close: (fd, cb) => {
-      fsMock.close = fs.close
-      process.nextTick(() => cb(new Error('witaf')))
+    ...fs.promises,
+    open: async (...args) => {
+      const fh = await fs.promises.open(...args)
+      fh.close = async () => {
+        throw new Error('witaf')
+      }
+      return fh
     },
   }
   const mockedFixBin = requireInject('../lib/fix-bin.js', {
-    fs: fsMock,
+    'fs/promises': fsMock,
   })
 
   const dir = t.testdir({

test/shim-bin.js

@@ -3,7 +3,6 @@ const requireInject = require('require-inject')
 const fs = require('fs')
 const { statSync } = fs
 const path = require('path').win32
-const mkdirp = require('mkdirp')
 
 t.test('basic shim bin', async t => {
   const dir = t.testdir({
@@ -15,7 +14,7 @@ t.test('basic shim bin', async t => {
     },
     notashim: 'definitely not',
   })
-  const shimBin = requireInject('../lib/shim-bin.js', { path, mkdirp })
+  const shimBin = requireInject('../lib/shim-bin.js', { path })
   await shimBin({
     path: `${dir}/pkg`,
     to: `${dir}/bin/hello`,
@@ -83,10 +82,9 @@ t.test('eperm on stat', async t => {
   })
   const shimBin = requireInject('../lib/shim-bin.js', {
     path,
-    mkdirp,
-    fs: {
-      ...fs,
-      lstat: (_, cb) => cb(Object.assign(new Error('wakawaka'), {
+    'fs/promises': {
+      ...fs.promises,
+      lstat: () => Promise.reject(Object.assign(new Error('wakawaka'), {
         code: 'EPERM',
       })),
     },
@@ -113,7 +111,6 @@ t.test('strange enoent from read-cmd-shim', async t => {
   })
   const shimBin = requireInject('../lib/shim-bin.js', {
     path,
-    mkdirp,
     'read-cmd-shim': () => Promise.reject(Object.assign(new Error('xyz'), {
       code: 'ENOENT',
     })),
@@ -163,7 +160,6 @@ t.test('unknown error from read-cmd-shim', async t => {
   })
   const shimBin = requireInject('../lib/shim-bin.js', {
     path,
-    mkdirp,
     'read-cmd-shim': () => Promise.reject(Object.assign(new Error('xyz'), {
       code: 'ELDERGAWDS',
     })),