diff --git a/README.md b/README.md index 7eef9291..5fe62c35 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,9 @@ Install with `npm install rimraf`, or just drop rimraf.js somewhere. * Built-in glob support removed. * Functions take arrays of paths, as well as a single path. * Native implementation used by default when available. -* New implementation on Windows, making the exponential backoff for - `EBUSY` and `ENOTEMPTY` errors no longer necessary. +* New implementation on Windows, falling back to "move then + remove" strategy when exponential backoff for `EBUSY` fails to + resolve the situation. * Simplified implementation on Posix, since the Windows affordances are not necessary there. @@ -21,13 +22,24 @@ This first parameter is a path. The second argument is an options object. Options: -* `tmp`: Temp folder to use to place files and folders for the Windows - implementation. Must be on the same physical device as the path being - deleted. Defaults to `os.tmpdir()` when that is on the same drive letter - as the path being deleted, or `${drive}:\temp` if present, or `${drive}:\` +* `preserveRoot`: If set to boolean `false`, then allow the + recursive removal of the root directory. Otherwise, this is + not allowed. +* `tmp`: Windows only. Temp folder to use to place files and + folders for the "move then remove" fallback. Must be on the + same physical device as the path being deleted. Defaults to + `os.tmpdir()` when that is on the same drive letter as the path + being deleted, or `${drive}:\temp` if present, or `${drive}:\` if not. -* `preserveRoot`: If set to boolean `false`, then allow the recursive - removal of the root directory. Otherwise, this is not allowed. +* `retries`: Windows only. Maximum number of synchronous retry + attempts in case of `EBUSY`, `EMFILE`, and `ENFILE` errors. + Default `10` +* `backoff`: Windows only. Rate of exponential backoff for async + removal in case of `EBUSY`, `EMFILE`, and `ENFILE` errors. + Should be a number greater than 1. Default `1.2` +* `maxBackoff`: Windows only. Maximum backoff time in ms to + attempt asynchronous retries in case of `EBUSY`, `EMFILE`, and + `ENFILE` errors. Default `100` Any other options are provided to the native Node.js `fs.rm` implementation when that is used. @@ -64,25 +76,42 @@ Synchronous form of `rimraf.manual()` ### `rimraf.windows(f, [opts])` JavaScript implementation of file removal appropriate for Windows -platforms, where `unlink` and `rmdir` are not atomic operations. +platforms. Works around `unlink` and `rmdir` not being atomic +operations, and `EPERM` when deleting files with certain +permission modes. -Moves all files and folders to the parent directory of `f` with a temporary -filename prior to attempting to remove them. - -Note that, in cases where the operation fails, this _may_ leave files lying -around in the parent directory with names like -`.file-basename.txt.0.123412341`. Until the Windows kernel provides a way -to perform atomic `unlink` and `rmdir` operations, this is unfortunately -unavoidable. - -To move files to a different temporary directory other than the parent, -provide `opts.tmp`. Note that this _must_ be on the same physical device -as the folder being deleted, or else the operation will fail. +First deletes all non-directory files within the tree, and then +removes all directories, which should ideally be empty by that +time. When an `ENOTEMPTY` is raised in the second pass, falls +back to the `rimraf.moveRemove` strategy as needed. ### `rimraf.windows.sync(path, [opts])` `rimraf.windowsSync(path, [opts])` Synchronous form of `rimraf.windows()` +### `rimraf.moveRemove(path, [opts])` + +Moves all files and folders to the parent directory of `path` +with a temporary filename prior to attempting to remove them. + +Note that, in cases where the operation fails, this _may_ leave +files lying around in the parent directory with names like +`.file-basename.txt.0.123412341`. Until the Windows kernel +provides a way to perform atomic `unlink` and `rmdir` operations, +this is unfortunately unavoidable. + +To move files to a different temporary directory other than the +parent, provide `opts.tmp`. Note that this _must_ be on the same +physical device as the folder being deleted, or else the +operation will fail. + +This is the slowest strategy, but most reliable on Windows +platforms. Used as a last-ditch fallback by `rimraf.windows()`. + +### `rimraf.moveRemove.sync(path, [opts])` `rimraf.moveRemoveSync(path, [opts])` + +Synchronous form of `rimraf.moveRemove()` + ### Command Line Interface ``` diff --git a/lib/bin.js b/lib/bin.js index bfdf6ab6..7bdee7d2 100755 --- a/lib/bin.js +++ b/lib/bin.js @@ -34,8 +34,9 @@ Implementation-specific options: const { resolve, parse } = require('path') const main = async (...args) => { - if (process.env.__RIMRAF_TESTING_BIN_FAIL__ === '1') + if (process.env.__RIMRAF_TESTING_BIN_FAIL__ === '1') { throw new Error('simulated rimraf failure') + } const opt = {} const paths = [] @@ -92,8 +93,9 @@ const main = async (...args) => { console.error(`unknown option: ${arg}`) runHelpForUsage() return 1 - } else + } else { paths.push(arg) + } } if (opt.preserveRoot !== false) { diff --git a/lib/default-tmp.js b/lib/default-tmp.js index 4354734a..3d5673e3 100644 --- a/lib/default-tmp.js +++ b/lib/default-tmp.js @@ -27,12 +27,14 @@ const win32DefaultTmp = async path => { const { root } = parse(path) const tmp = tmpdir() const { root: tmpRoot } = parse(tmp) - if (root.toLowerCase() === tmpRoot.toLowerCase()) + if (root.toLowerCase() === tmpRoot.toLowerCase()) { return tmp + } const driveTmp = resolve(root, '/temp') - if (await isDir(driveTmp)) + if (await isDir(driveTmp)) { return driveTmp + } return root } @@ -41,12 +43,14 @@ const win32DefaultTmpSync = path => { const { root } = parse(path) const tmp = tmpdir() const { root: tmpRoot } = parse(tmp) - if (root.toLowerCase() === tmpRoot.toLowerCase()) + if (root.toLowerCase() === tmpRoot.toLowerCase()) { return tmp + } const driveTmp = resolve(root, '/temp') - if (isDirSync(driveTmp)) + if (isDirSync(driveTmp)) { return driveTmp + } return root } diff --git a/lib/fix-eperm.js b/lib/fix-eperm.js new file mode 100644 index 00000000..3676c988 --- /dev/null +++ b/lib/fix-eperm.js @@ -0,0 +1,53 @@ +const { + promises: { chmod }, + chmodSync, +} = require('./fs.js') + +const fixEPERM = fn => async path => { + try { + return await fn(path) + } catch (er) { + if (er.code === 'ENOENT') { + return + } + if (er.code === 'EPERM') { + try { + await chmod(path, 0o666) + } catch (er2) { + if (er2.code === 'ENOENT') { + return + } + throw er + } + return await fn(path) + } + throw er + } +} + +const fixEPERMSync = fn => path => { + try { + return fn(path) + } catch (er) { + if (er.code === 'ENOENT') { + return + } + if (er.code === 'EPERM') { + try { + chmodSync(path, 0o666) + } catch (er2) { + if (er2.code === 'ENOENT') { + return + } + throw er + } + return fn(path) + } + throw er + } +} + +module.exports = { + fixEPERM, + fixEPERMSync, +} diff --git a/lib/ignore-enoent.js b/lib/ignore-enoent.js index 5abe5ca3..7ee8fd99 100644 --- a/lib/ignore-enoent.js +++ b/lib/ignore-enoent.js @@ -1,14 +1,16 @@ const ignoreENOENT = async p => p.catch(er => { - if (er.code !== 'ENOENT') + if (er.code !== 'ENOENT') { throw er + } }) const ignoreENOENTSync = fn => { try { return fn() } catch (er) { - if (er.code !== 'ENOENT') + if (er.code !== 'ENOENT') { throw er + } } } diff --git a/lib/index.js b/lib/index.js index 10cbfbeb..1344bf5a 100644 --- a/lib/index.js +++ b/lib/index.js @@ -5,6 +5,7 @@ const {rimrafNative, rimrafNativeSync} = require('./rimraf-native.js') const {rimrafManual, rimrafManualSync} = require('./rimraf-manual.js') const {rimrafWindows, rimrafWindowsSync} = require('./rimraf-windows.js') const {rimrafPosix, rimrafPosixSync} = require('./rimraf-posix.js') +const {rimrafMoveRemove, rimrafMoveRemoveSync} = require('./rimraf-move-remove.js') const {useNative, useNativeSync} = require('./use-native.js') const wrap = fn => async (path, opt) => { @@ -58,4 +59,10 @@ posix.sync = posixSync rimraf.posix = posix rimraf.posixSync = posixSync +const moveRemove = wrap(rimrafMoveRemove) +const moveRemoveSync = wrapSync(rimrafMoveRemoveSync) +moveRemove.sync = moveRemoveSync +rimraf.moveRemove = moveRemove +rimraf.moveRemoveSync = moveRemoveSync + module.exports = rimraf diff --git a/lib/retry-busy.js b/lib/retry-busy.js new file mode 100644 index 00000000..8bbe4ad3 --- /dev/null +++ b/lib/retry-busy.js @@ -0,0 +1,61 @@ +const MAXBACKOFF = 100 +const RATE = 1.2 +const MAXRETRIES = 10 + +const codes = new Set(['EMFILE', 'ENFILE', 'EBUSY']) +const retryBusy = fn => { + const method = async (path, opt, backoff = 1) => { + const mbo = opt.maxBackoff || MAXBACKOFF + const rate = Math.max(opt.backoff || RATE, RATE) + const max = opt.retries || MAXRETRIES + let retries = 0 + while (true) { + try { + return await fn(path) + } catch (er) { + if (codes.has(er.code)) { + backoff = Math.ceil(backoff * rate) + if (backoff < mbo) { + return new Promise((res, rej) => { + setTimeout(() => { + method(path, opt, backoff).then(res, rej) + }, backoff) + }) + } + if (retries < max) { + retries++ + continue + } + } + throw er + } + } + } + + return method +} + +// just retries, no async so no backoff +const retryBusySync = fn => { + const method = (path, opt) => { + const max = opt.retries || MAXRETRIES + let retries = 0 + while (true) { + try { + return fn(path) + } catch (er) { + if (codes.has(er.code) && retries < max) { + retries++ + continue + } + throw er + } + } + } + return method +} + +module.exports = { + retryBusy, + retryBusySync, +} diff --git a/lib/rimraf-move-remove.js b/lib/rimraf-move-remove.js new file mode 100644 index 00000000..99827a93 --- /dev/null +++ b/lib/rimraf-move-remove.js @@ -0,0 +1,164 @@ +// https://youtu.be/uhRWMGBjlO8?t=537 +// +// 1. readdir +// 2. for each entry +// a. if a non-empty directory, recurse +// b. if an empty directory, move to random hidden file name in $TEMP +// c. unlink/rmdir $TEMP +// +// This works around the fact that unlink/rmdir is non-atomic and takes +// a non-deterministic amount of time to complete. +// +// However, it is HELLA SLOW, like 2-10x slower than a naive recursive rm. + +const { resolve, basename, parse } = require('path') +const { defaultTmp, defaultTmpSync } = require('./default-tmp.js') + +const { + ignoreENOENT, + ignoreENOENTSync, +} = require('./ignore-enoent.js') + +const { + renameSync, + unlinkSync, + rmdirSync, + chmodSync, + promises: { + rename, + unlink, + rmdir, + chmod, + }, +} = require('./fs.js') + +const { + readdirOrError, + readdirOrErrorSync, +} = require('./readdir-or-error.js') + +// crypto.randomBytes is much slower, and Math.random() is enough here +const uniqueFilename = path => `.${basename(path)}.${Math.random()}` + +const unlinkFixEPERM = async path => unlink(path).catch(er => { + if (er.code === 'EPERM') { + return chmod(path, 0o666).then( + () => unlink(path), + er2 => { + if (er2.code === 'ENOENT') { + return + } + throw er + } + ) + } else if (er.code === 'ENOENT') { + return + } + throw er +}) + +const unlinkFixEPERMSync = path => { + try { + unlinkSync(path) + } catch (er) { + if (er.code === 'EPERM') { + try { + return chmodSync(path, 0o666) + } catch (er2) { + if (er2.code === 'ENOENT') { + return + } + throw er + } + } else if (er.code === 'ENOENT') { + return + } + throw er + } +} + +const rimrafMoveRemove = async (path, opt) => { + if (!opt.tmp) { + return rimrafMoveRemove(path, { ...opt, tmp: await defaultTmp(path) }) + } + + if (path === opt.tmp && parse(path).root !== path) { + throw new Error('cannot delete temp directory used for deletion') + } + + const entries = await readdirOrError(path) + if (!Array.isArray(entries)) { + if (entries.code === 'ENOENT') { + return + } + + if (entries.code !== 'ENOTDIR') { + throw entries + } + + return await ignoreENOENT(tmpUnlink(path, opt.tmp, unlinkFixEPERM)) + } + + await Promise.all(entries.map(entry => + rimrafMoveRemove(resolve(path, entry), opt))) + + // we don't ever ACTUALLY try to unlink /, because that can never work + // but when preserveRoot is false, we could be operating on it. + // No need to check if preserveRoot is not false. + if (opt.preserveRoot === false && path === parse(path).root) { + return + } + + return await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir)) +} + +const tmpUnlink = async (path, tmp, rm) => { + const tmpFile = resolve(tmp, uniqueFilename(path)) + await rename(path, tmpFile) + return await rm(tmpFile) +} + +const rimrafMoveRemoveSync = (path, opt) => { + if (!opt.tmp) { + return rimrafMoveRemoveSync(path, { ...opt, tmp: defaultTmpSync(path) }) + } + + if (path === opt.tmp && parse(path).root !== path) { + throw new Error('cannot delete temp directory used for deletion') + } + + const entries = readdirOrErrorSync(path) + if (!Array.isArray(entries)) { + if (entries.code === 'ENOENT') { + return + } + + if (entries.code !== 'ENOTDIR') { + throw entries + } + + return ignoreENOENTSync(() => + tmpUnlinkSync(path, opt.tmp, unlinkFixEPERMSync)) + } + + for (const entry of entries) { + rimrafMoveRemoveSync(resolve(path, entry), opt) + } + + if (opt.preserveRoot === false && path === parse(path).root) { + return + } + + return ignoreENOENTSync(() => tmpUnlinkSync(path, opt.tmp, rmdirSync)) +} + +const tmpUnlinkSync = (path, tmp, rmSync) => { + const tmpFile = resolve(tmp, uniqueFilename(path)) + renameSync(path, tmpFile) + return rmSync(tmpFile) +} + +module.exports = { + rimrafMoveRemove, + rimrafMoveRemoveSync, +} diff --git a/lib/rimraf-posix.js b/lib/rimraf-posix.js index fe2fd007..608db0ff 100644 --- a/lib/rimraf-posix.js +++ b/lib/rimraf-posix.js @@ -29,10 +29,12 @@ const { const rimrafPosix = async (path, opt) => { const entries = await readdirOrError(path) if (!Array.isArray(entries)) { - if (entries.code === 'ENOENT') + if (entries.code === 'ENOENT') { return - if (entries.code !== 'ENOTDIR') + } + if (entries.code !== 'ENOTDIR') { throw entries + } return ignoreENOENT(unlink(path)) } await Promise.all(entries.map(entry => @@ -41,8 +43,9 @@ const rimrafPosix = async (path, opt) => { // we don't ever ACTUALLY try to unlink /, because that can never work // but when preserveRoot is false, we could be operating on it. // No need to check if preserveRoot is not false. - if (opt.preserveRoot === false && path === parse(path).root) + if (opt.preserveRoot === false && path === parse(path).root) { return + } return ignoreENOENT(rmdir(path)) } @@ -50,17 +53,21 @@ const rimrafPosix = async (path, opt) => { const rimrafPosixSync = (path, opt) => { const entries = readdirOrErrorSync(path) if (!Array.isArray(entries)) { - if (entries.code === 'ENOENT') + if (entries.code === 'ENOENT') { return - if (entries.code !== 'ENOTDIR') + } + if (entries.code !== 'ENOTDIR') { throw entries + } return ignoreENOENTSync(() => unlinkSync(path)) } - for (const entry of entries) + for (const entry of entries) { rimrafPosixSync(resolve(path, entry), opt) + } - if (opt.preserveRoot === false && path === parse(path).root) + if (opt.preserveRoot === false && path === parse(path).root) { return + } return ignoreENOENTSync(() => rmdirSync(path)) } diff --git a/lib/rimraf-windows.js b/lib/rimraf-windows.js index bc49267b..c0314dce 100644 --- a/lib/rimraf-windows.js +++ b/lib/rimraf-windows.js @@ -1,16 +1,14 @@ -// https://youtu.be/uhRWMGBjlO8?t=537 +// This is the same as rimrafPosix, with the following changes: // -// 1. readdir -// 2. for each entry -// a. if a non-empty directory, recurse -// b. if an empty directory, move to random hidden file name in $TEMP -// c. unlink/rmdir $TEMP +// 1. EBUSY, ENFILE, EMFILE trigger retries and/or exponential backoff +// 2. All non-directories are removed first and then all directories are +// removed in a second sweep. +// 3. If we hit ENOTEMPTY in the second sweep, fall back to move-remove on +// the that folder. // -// This works around the fact that unlink/rmdir is non-atomic and takes -// a non-deterministic amount of time to complete. +// Note: "move then remove" is 2-10 times slower, and just as unreliable. -const { resolve, basename, parse } = require('path') -const { defaultTmp, defaultTmpSync } = require('./default-tmp.js') +const { resolve, parse } = require('path') const { ignoreENOENT, @@ -18,127 +16,125 @@ const { } = require('./ignore-enoent.js') const { - renameSync, unlinkSync, rmdirSync, - chmodSync, promises: { - rename, unlink, rmdir, - chmod, }, } = require('./fs.js') +const { fixEPERM, fixEPERMSync } = require('./fix-eperm.js') + const { readdirOrError, readdirOrErrorSync, } = require('./readdir-or-error.js') -// crypto.randomBytes is much slower, and Math.random() is enough here -const uniqueFilename = path => `.${basename(path)}.${Math.random()}` - -const unlinkFixEPERM = async path => unlink(path).catch(er => { - if (er.code === 'EPERM') { - return chmod(path, 0o666).then( - () => unlink(path), - er2 => { - if (er2.code === 'ENOENT') - return - throw er - } - ) - } else if (er.code === 'ENOENT') - return - throw er -}) - -const unlinkFixEPERMSync = path => { +const { + retryBusy, + retryBusySync, +} = require('./retry-busy.js') + +const { + rimrafMoveRemove, + rimrafMoveRemoveSync, +} = require('./rimraf-move-remove.js') + +const rimrafWindowsFile = retryBusy(fixEPERM(unlink)) +const rimrafWindowsFileSync = retryBusySync(fixEPERMSync(unlinkSync)) +const rimrafWindowsDir = retryBusy(fixEPERM(rmdir)) +const rimrafWindowsDirSync = retryBusySync(fixEPERMSync(rmdirSync)) + +const rimrafWindowsDirMoveRemoveFallback = async (path, opt) => { try { - unlinkSync(path) + await rimrafWindowsDir(path, opt) } catch (er) { - if (er.code === 'EPERM') { - try { - return chmodSync(path, 0o666) - } catch (er2) { - if (er2.code === 'ENOENT') - return - throw er - } - } else if (er.code === 'ENOENT') - return + if (er.code === 'ENOTEMPTY') { + return await rimrafMoveRemove(path, opt) + } + throw er + } +} + +const rimrafWindowsDirMoveRemoveFallbackSync = (path, opt) => { + try { + rimrafWindowsDirSync(path, opt) + } catch (er) { + if (er.code === 'ENOTEMPTY') { + return rimrafMoveRemoveSync(path, opt) + } throw er } } -const rimrafWindows = async (path, opt) => { - if (!opt.tmp) - return rimrafWindows(path, { ...opt, tmp: await defaultTmp(path) }) +const START = Symbol('start') +const CHILD = Symbol('child') +const FINISH = Symbol('finish') +const states = new Set([START, CHILD, FINISH]) - if (path === opt.tmp && parse(path).root !== path) - throw new Error('cannot delete temp directory used for deletion') +const rimrafWindows = async (path, opt, state = START) => { + if (!states.has(state)) { + throw new TypeError('invalid third argument passed to rimraf') + } const entries = await readdirOrError(path) if (!Array.isArray(entries)) { - if (entries.code === 'ENOENT') + if (entries.code === 'ENOENT') { return - - if (entries.code !== 'ENOTDIR') + } + if (entries.code !== 'ENOTDIR') { throw entries - - return await ignoreENOENT(tmpUnlink(path, opt.tmp, unlinkFixEPERM)) + } + // is a file + return ignoreENOENT(rimrafWindowsFile(path, opt)) } await Promise.all(entries.map(entry => - rimrafWindows(resolve(path, entry), opt))) + rimrafWindows(resolve(path, entry), opt, state === START ? CHILD : state))) - // we don't ever ACTUALLY try to unlink /, because that can never work - // but when preserveRoot is false, we could be operating on it. - // No need to check if preserveRoot is not false. - if (opt.preserveRoot === false && path === parse(path).root) - return - - return await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir)) -} - -const tmpUnlink = async (path, tmp, rm) => { - const tmpFile = resolve(tmp, uniqueFilename(path)) - await rename(path, tmpFile) - return await rm(tmpFile) + if (state === START) { + return rimrafWindows(path, opt, FINISH) + } else if (state === FINISH) { + if (opt.preserveRoot === false && path === parse(path).root) { + return + } + return ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + } } -const rimrafWindowsSync = (path, opt) => { - if (!opt.tmp) - return rimrafWindowsSync(path, { ...opt, tmp: defaultTmpSync(path) }) - - if (path === opt.tmp && parse(path).root !== path) - throw new Error('cannot delete temp directory used for deletion') +const rimrafWindowsSync = (path, opt, state = START) => { + if (!states.has(state)) { + throw new TypeError('invalid third argument passed to rimraf') + } const entries = readdirOrErrorSync(path) if (!Array.isArray(entries)) { - if (entries.code === 'ENOENT') + if (entries.code === 'ENOENT') { return - - if (entries.code !== 'ENOTDIR') + } + if (entries.code !== 'ENOTDIR') { throw entries - - return ignoreENOENTSync(() => - tmpUnlinkSync(path, opt.tmp, unlinkFixEPERMSync)) + } + // is a file + return ignoreENOENTSync(() => rimrafWindowsFileSync(path, opt)) } - for (const entry of entries) - rimrafWindowsSync(resolve(path, entry), opt) - - if (opt.preserveRoot === false && path === parse(path).root) - return - - return ignoreENOENTSync(() => tmpUnlinkSync(path, opt.tmp, rmdirSync)) -} + for (const entry of entries) { + const s = state === START ? CHILD : state + rimrafWindowsSync(resolve(path, entry), opt, s) + } -const tmpUnlinkSync = (path, tmp, rmSync) => { - const tmpFile = resolve(tmp, uniqueFilename(path)) - renameSync(path, tmpFile) - return rmSync(tmpFile) + if (state === START) { + return rimrafWindowsSync(path, opt, FINISH) + } else if (state === FINISH) { + if (opt.preserveRoot === false && path === parse(path).root) { + return + } + return ignoreENOENTSync(() => { + rimrafWindowsDirMoveRemoveFallbackSync(path, opt) + }) + } } module.exports = { diff --git a/tap-snapshots/test/rimraf-move-remove.js.test.cjs b/tap-snapshots/test/rimraf-move-remove.js.test.cjs new file mode 100644 index 00000000..4d9b1d74 --- /dev/null +++ b/tap-snapshots/test/rimraf-move-remove.js.test.cjs @@ -0,0 +1,60 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'use strict' +exports[`test/rimraf-move-remove.js TAP handle EPERMs on unlink by trying to chmod 0o666 async > must match snapshot 1`] = ` +Array [ + Array [ + "chmod", + "{tmpfile}", + "438", + ], +] +` + +exports[`test/rimraf-move-remove.js TAP handle EPERMs on unlink by trying to chmod 0o666 sync > must match snapshot 1`] = ` +Array [ + Array [ + "chmodSync", + "{tmpfile}", + "438", + ], +] +` + +exports[`test/rimraf-move-remove.js TAP handle EPERMs, chmod raises something other than ENOENT async > must match snapshot 1`] = ` +Array [] +` + +exports[`test/rimraf-move-remove.js TAP handle EPERMs, chmod raises something other than ENOENT sync > must match snapshot 1`] = ` +Array [ + Array [ + "chmodSync", + "{tmpfile}", + "438", + ], +] +` + +exports[`test/rimraf-move-remove.js TAP handle EPERMs, chmod returns ENOENT async > must match snapshot 1`] = ` +Array [ + Array [ + "chmod", + "{tmpfile}", + "438", + ], +] +` + +exports[`test/rimraf-move-remove.js TAP handle EPERMs, chmod returns ENOENT sync > must match snapshot 1`] = ` +Array [ + Array [ + "chmodSync", + "{tmpfile}", + "438", + ], +] +` diff --git a/tap-snapshots/test/rimraf-windows.js.test.cjs b/tap-snapshots/test/rimraf-windows.js.test.cjs index 13a5719e..ec86e1ae 100644 --- a/tap-snapshots/test/rimraf-windows.js.test.cjs +++ b/tap-snapshots/test/rimraf-windows.js.test.cjs @@ -9,7 +9,7 @@ exports[`test/rimraf-windows.js TAP handle EPERMs on unlink by trying to chmod 0 Array [ Array [ "chmod", - "{tmpfile}", + "{CWD}/test/tap-testdir-rimraf-windows-handle-EPERMs-on-unlink-by-trying-to-chmod-0o666-async/test/a", "438", ], ] @@ -19,7 +19,7 @@ exports[`test/rimraf-windows.js TAP handle EPERMs on unlink by trying to chmod 0 Array [ Array [ "chmodSync", - "{tmpfile}", + "{CWD}/test/tap-testdir-rimraf-windows-handle-EPERMs-on-unlink-by-trying-to-chmod-0o666-sync/test/a", "438", ], ] @@ -33,7 +33,7 @@ exports[`test/rimraf-windows.js TAP handle EPERMs, chmod raises something other Array [ Array [ "chmodSync", - "{tmpfile}", + "{CWD}/test/tap-testdir-rimraf-windows-handle-EPERMs-chmod-raises-something-other-than-ENOENT-sync/test/a", "438", ], ] @@ -43,7 +43,7 @@ exports[`test/rimraf-windows.js TAP handle EPERMs, chmod returns ENOENT async > Array [ Array [ "chmod", - "{tmpfile}", + "{CWD}/test/tap-testdir-rimraf-windows-handle-EPERMs-chmod-returns-ENOENT-async/test/a", "438", ], ] @@ -53,7 +53,7 @@ exports[`test/rimraf-windows.js TAP handle EPERMs, chmod returns ENOENT sync > m Array [ Array [ "chmodSync", - "{tmpfile}", + "{CWD}/test/tap-testdir-rimraf-windows-handle-EPERMs-chmod-returns-ENOENT-sync/test/a", "438", ], ] diff --git a/test/delete-many-files.js b/test/delete-many-files.js index 40a60534..ba6dd07c 100644 --- a/test/delete-many-files.js +++ b/test/delete-many-files.js @@ -20,10 +20,11 @@ const create = (path, depth = 0) => { mkdirSync(path) for (let i = START; i <= END; i++) { const c = String.fromCharCode(i) - if (depth < DEPTH && (i - START >= depth)) + if (depth < DEPTH && (i - START >= depth)) { create(resolve(path, c), depth + 1) - else + } else { writeFileSync(resolve(path, c), c) + } } return path } diff --git a/test/fs.js b/test/fs.js index fa9f6312..50883f6c 100644 --- a/test/fs.js +++ b/test/fs.js @@ -33,20 +33,23 @@ for (const method of Object.keys(fs.promises)) { // doesn't have any sync versions that aren't promisified for (const method of Object.keys(fs)) { - if (method === 'promises') + if (method === 'promises') { continue + } const m = method.replace(/Sync$/, '') t.type(fs.promises[m], Function, `fs.promises.${m} is a function`) } t.test('passing resolves promise', async t => { const fs = t.mock('../lib/fs.js', { fs: mockFSPass }) - for (const [m, fn] of Object.entries(fs.promises)) + for (const [m, fn] of Object.entries(fs.promises)) { t.same(await fn(), m, `got expected value for ${m}`) + } }) t.test('failing rejects promise', async t => { const fs = t.mock('../lib/fs.js', { fs: mockFSFail }) - for (const [m, fn] of Object.entries(fs.promises)) + for (const [m, fn] of Object.entries(fs.promises)) { t.rejects(fn(), { message: 'oops' }, `got expected value for ${m}`) + } }) diff --git a/test/rimraf-move-remove.js b/test/rimraf-move-remove.js new file mode 100644 index 00000000..783af8b8 --- /dev/null +++ b/test/rimraf-move-remove.js @@ -0,0 +1,495 @@ +const t = require('tap') +t.formatSnapshot = calls => calls.map(args => args.map(arg => + String(arg) + .split(process.cwd()) + .join('{CWD}') + .replace(/\\/g, '/').replace(/.*\/(\.[a-z]\.)[^/]*$/, '{tmpfile}'))) + +const fixture = { + a: 'a', + b: 'b', + c: { + d: 'd', + e: 'e', + f: { + g: 'g', + h: 'h', + i: { + j: 'j', + k: 'k', + l: 'l', + m: { + n: 'n', + o: 'o', + }, + }, + }, + }, +} + +t.only('actually delete some stuff', async t => { + const fs = require('../lib/fs.js') + const fsMock = { ...fs, promises: { ...fs.promises }} + + // simulate annoying windows semantics, where an unlink or rmdir + // may take an arbitrary amount of time. we only delay unlinks, + // to ensure that we will get an error when we try to rmdir. + const { + statSync, + promises: { + unlink, + }, + } = fs + + const danglers = [] + const unlinkLater = path => { + const p = new Promise((res) => { + setTimeout(() => unlink(path).then(res, res)) + }) + danglers.push(p) + } + fsMock.unlinkSync = path => unlinkLater(path) + fsMock.promises.unlink = async path => unlinkLater(path) + + // but actually do wait to clean them up, though + t.teardown(() => Promise.all(danglers)) + + const { + rimrafPosix, + rimrafPosixSync, + } = t.mock('../lib/rimraf-posix.js', { '../lib/fs.js': fsMock }) + + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { '../lib/fs.js': fsMock }) + + t.test('posix does not work here', t => { + t.test('sync', t => { + const path = t.testdir(fixture) + t.throws(() => rimrafPosixSync(path)) + t.end() + }) + t.test('async', t => { + const path = t.testdir(fixture) + t.rejects(() => rimrafPosix(path)) + t.end() + }) + t.end() + }) + + t.test('sync', t => { + const path = t.testdir(fixture) + rimrafMoveRemoveSync(path, {}) + t.throws(() => statSync(path), { code: 'ENOENT' }, 'deleted') + t.doesNotThrow(() => rimrafMoveRemoveSync(path, {}), + 'deleting a second time is OK') + t.end() + }) + + t.test('async', async t => { + const path = t.testdir(fixture) + await rimrafMoveRemove(path, {}) + t.throws(() => statSync(path), { code: 'ENOENT' }, 'deleted') + t.resolves(rimrafMoveRemove(path, {}), 'deleting a second time is OK') + }) + t.end() +}) + +t.only('throw unlink errors', async t => { + const fs = require('../lib/fs.js') + // only throw once here, or else it messes with tap's fixture cleanup + // that's probably a bug in t.mock? + let threwAsync = false + let threwSync = false + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + unlinkSync: path => { + if (threwSync) { + return fs.unlinkSync(path) + } + threwSync = true + throw Object.assign(new Error('cannot unlink'), { code: 'FOO' }) + }, + promises: { + ...fs.promises, + unlink: async path => { + if (threwAsync) { + return fs.promises.unlink(path) + } + threwAsync = true + throw Object.assign(new Error('cannot unlink'), { code: 'FOO' }) + }, + }, + }, + }) + // nest to clean up the mess + t.test('sync', t => { + const path = t.testdir({ test: fixture }) + '/test' + t.throws(() => rimrafMoveRemoveSync(path, {}), { code: 'FOO' }) + t.end() + }) + t.test('async', t => { + const path = t.testdir({ test: fixture }) + '/test' + t.rejects(rimrafMoveRemove(path, {}), { code: 'FOO' }) + t.end() + }) + t.end() +}) + +t.only('ignore ENOENT unlink errors', async t => { + const fs = require('../lib/fs.js') + const threwAsync = false + let threwSync = false + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + unlinkSync: path => { + fs.unlinkSync(path) + if (threwSync) { + return + } + threwSync = true + fs.unlinkSync(path) + }, + promises: { + ...fs.promises, + unlink: async path => { + fs.unlinkSync(path) + if (threwAsync) { + return + } + threwSync = true + fs.unlinkSync(path) + }, + }, + }, + }) + // nest to clean up the mess + t.test('sync', t => { + const path = t.testdir({ test: fixture }) + '/test' + t.doesNotThrow(() => rimrafMoveRemoveSync(path, {}), 'enoent no problems') + t.end() + }) + t.test('async', t => { + const path = t.testdir({ test: fixture }) + '/test' + t.resolves(() => rimrafMoveRemove(path, {}), 'enoent no problems') + t.end() + }) + t.end() +}) + +t.test('throw rmdir errors', async t => { + const fs = require('../lib/fs.js') + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + rmdirSync: path => { + throw Object.assign(new Error('cannot rmdir'), { code: 'FOO' }) + }, + promises: { + ...fs.promises, + rmdir: async path => { + throw Object.assign(new Error('cannot rmdir'), { code: 'FOO' }) + }, + }, + }, + }) + t.test('sync', t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.throws(() => rimrafMoveRemoveSync(path, {}), { code: 'FOO' }) + t.end() + }) + t.test('async', t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.rejects(rimrafMoveRemove(path, {}), { code: 'FOO' }) + t.end() + }) + t.end() +}) + +t.test('throw unexpected readdir errors', async t => { + const fs = require('../lib/fs.js') + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + readdirSync: path => { + throw Object.assign(new Error('cannot readdir'), { code: 'FOO' }) + }, + promises: { + ...fs.promises, + readdir: async path => { + throw Object.assign(new Error('cannot readdir'), { code: 'FOO' }) + }, + }, + }, + }) + t.test('sync', t => { + // nest to clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.throws(() => rimrafMoveRemoveSync(path, {}), { code: 'FOO' }) + t.end() + }) + t.test('async', t => { + // nest to clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.rejects(rimrafMoveRemove(path, {}), { code: 'FOO' }) + t.end() + }) + t.end() +}) + +t.test('refuse to delete the root dir', async t => { + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + path: { + ...require('path'), + dirname: path => path, + }, + }) + + // not brave enough to pass the actual c:\\ here... + t.throws(() => rimrafMoveRemoveSync('some-path', { tmp: 'some-path' }), { + message: 'cannot delete temp directory used for deletion', + }) + t.rejects(() => rimrafMoveRemove('some-path', { tmp: 'some-path' }), { + message: 'cannot delete temp directory used for deletion', + }) +}) + +t.test('handle EPERMs on unlink by trying to chmod 0o666', async t => { + const fs = require('../lib/fs.js') + const CHMODS = [] + let threwAsync = false + let threwSync = false + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + chmodSync: (...args) => { + CHMODS.push(['chmodSync', ...args]) + return fs.chmodSync(...args) + }, + unlinkSync: path => { + if (threwSync) { + return fs.unlinkSync(path) + } + threwSync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + promises: { + ...fs.promises, + unlink: async path => { + if (threwAsync) { + return fs.promises.unlink(path) + } + threwAsync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + chmod: async (...args) => { + CHMODS.push(['chmod', ...args]) + return fs.promises.chmod(...args) + }, + }, + }, + }) + + t.afterEach(() => CHMODS.length = 0) + + t.test('sync', t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + rimrafMoveRemoveSync(path, {}) + t.matchSnapshot(CHMODS) + t.end() + }) + t.test('async', async t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + await rimrafMoveRemove(path, {}) + t.matchSnapshot(CHMODS) + t.end() + }) + t.end() +}) + +t.test('handle EPERMs, chmod returns ENOENT', async t => { + const fs = require('../lib/fs.js') + const CHMODS = [] + let threwAsync = false + let threwSync = false + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + chmodSync: (...args) => { + CHMODS.push(['chmodSync', ...args]) + try { + fs.unlinkSync(args[0]) + } catch (_) {} + return fs.chmodSync(...args) + }, + unlinkSync: path => { + if (threwSync) { + return fs.unlinkSync(path) + } + threwSync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + promises: { + ...fs.promises, + unlink: async path => { + if (threwAsync) { + return fs.promises.unlink(path) + } + threwAsync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + chmod: async (...args) => { + CHMODS.push(['chmod', ...args]) + try { + fs.unlinkSync(args[0]) + } catch (_) {} + return fs.promises.chmod(...args) + }, + }, + }, + }) + + t.afterEach(() => CHMODS.length = 0) + + t.test('sync', t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + rimrafMoveRemoveSync(path, {}) + t.matchSnapshot(CHMODS) + t.end() + }) + t.test('async', async t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + await rimrafMoveRemove(path, {}) + t.matchSnapshot(CHMODS) + t.end() + }) + t.end() +}) + +t.test('handle EPERMs, chmod raises something other than ENOENT', async t => { + const fs = require('../lib/fs.js') + const CHMODS = [] + let threwAsync = false + let threwSync = false + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + '../lib/fs.js': { + ...fs, + chmodSync: (...args) => { + CHMODS.push(['chmodSync', ...args]) + try { + fs.unlinkSync(args[0]) + } catch (_) {} + throw new Error('cannot chmod', { code: 'FOO' }) + }, + unlinkSync: path => { + if (threwSync) { + return fs.unlinkSync(path) + } + threwSync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + promises: { + ...fs.promises, + unlink: async path => { + if (threwAsync) { + return fs.promises.unlink(path) + } + threwAsync = true + throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) + }, + chmod: async (...args) => { + CHMODS.push(['chmod', ...args]) + try { + fs.unlinkSync(args[0]) + } catch (_) {} + throw new Error('cannot chmod', { code: 'FOO' }) + }, + }, + }, + }) + + t.afterEach(() => CHMODS.length = 0) + + t.test('sync', t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.throws(() => rimrafMoveRemoveSync(path, {}), { code: 'EPERM' }) + t.matchSnapshot(CHMODS) + t.end() + }) + t.test('async', async t => { + // nest it so that we clean up the mess + const path = t.testdir({ test: fixture }) + '/test' + t.rejects(rimrafMoveRemove(path, {}), { code: 'EPERM' }) + t.matchSnapshot(CHMODS) + t.end() + }) + t.end() +}) + +t.test('rimraffing root, do not actually rmdir root', async t => { + const fs = require('../lib/fs.js') + let ROOT = null + const { parse } = require('path') + const { + rimrafMoveRemove, + rimrafMoveRemoveSync, + } = t.mock('../lib/rimraf-move-remove.js', { + path: { + ...require('path'), + parse: (path) => { + const p = parse(path) + if (path === ROOT) { + p.root = path + } + return p + }, + }, + }) + t.test('async', async t => { + ROOT = t.testdir(fixture) + await rimrafMoveRemove(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.test('sync', async t => { + ROOT = t.testdir(fixture) + rimrafMoveRemoveSync(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.end() +}) diff --git a/test/rimraf-posix.js b/test/rimraf-posix.js index c04edf9f..21af27ff 100644 --- a/test/rimraf-posix.js +++ b/test/rimraf-posix.js @@ -179,8 +179,9 @@ t.test('rimraffing root, do not actually rmdir root', async t => { ...require('path'), parse: (path) => { const p = parse(path) - if (path === ROOT) + if (path === ROOT) { p.root = path + } return p }, }, diff --git a/test/rimraf-windows.js b/test/rimraf-windows.js index 5639d1a8..64428f0b 100644 --- a/test/rimraf-windows.js +++ b/test/rimraf-windows.js @@ -43,8 +43,8 @@ t.only('actually delete some stuff', async t => { const danglers = [] const unlinkLater = path => { - const p = new Promise((res, rej) => { - setTimeout(() => unlink(path).then(res, rej)) + const p = new Promise((res) => { + setTimeout(() => unlink(path).then(res, res), 50) }) danglers.push(p) } @@ -109,16 +109,18 @@ t.only('throw unlink errors', async t => { '../lib/fs.js': { ...fs, unlinkSync: path => { - if (threwSync) + if (threwSync) { return fs.unlinkSync(path) + } threwSync = true throw Object.assign(new Error('cannot unlink'), { code: 'FOO' }) }, promises: { ...fs.promises, unlink: async path => { - if (threwAsync) + if (threwAsync) { return fs.promises.unlink(path) + } threwAsync = true throw Object.assign(new Error('cannot unlink'), { code: 'FOO' }) }, @@ -151,8 +153,9 @@ t.only('ignore ENOENT unlink errors', async t => { ...fs, unlinkSync: path => { fs.unlinkSync(path) - if (threwSync) + if (threwSync) { return + } threwSync = true fs.unlinkSync(path) }, @@ -160,8 +163,9 @@ t.only('ignore ENOENT unlink errors', async t => { ...fs.promises, unlink: async path => { fs.unlinkSync(path) - if (threwAsync) + if (threwAsync) { return + } threwSync = true fs.unlinkSync(path) }, @@ -250,26 +254,6 @@ t.test('throw unexpected readdir errors', async t => { t.end() }) -t.test('refuse to delete the root dir', async t => { - const { - rimrafWindows, - rimrafWindowsSync, - } = t.mock('../lib/rimraf-windows.js', { - path: { - ...require('path'), - dirname: path => path, - }, - }) - - // not brave enough to pass the actual c:\\ here... - t.throws(() => rimrafWindowsSync('some-path', { tmp: 'some-path' }), { - message: 'cannot delete temp directory used for deletion', - }) - t.rejects(() => rimrafWindows('some-path', { tmp: 'some-path' }), { - message: 'cannot delete temp directory used for deletion', - }) -}) - t.test('handle EPERMs on unlink by trying to chmod 0o666', async t => { const fs = require('../lib/fs.js') const CHMODS = [] @@ -286,16 +270,18 @@ t.test('handle EPERMs on unlink by trying to chmod 0o666', async t => { return fs.chmodSync(...args) }, unlinkSync: path => { - if (threwSync) + if (threwSync) { return fs.unlinkSync(path) + } threwSync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, promises: { ...fs.promises, unlink: async path => { - if (threwAsync) + if (threwAsync) { return fs.promises.unlink(path) + } threwAsync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, @@ -345,16 +331,18 @@ t.test('handle EPERMs, chmod returns ENOENT', async t => { return fs.chmodSync(...args) }, unlinkSync: path => { - if (threwSync) + if (threwSync) { return fs.unlinkSync(path) + } threwSync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, promises: { ...fs.promises, unlink: async path => { - if (threwAsync) + if (threwAsync) { return fs.promises.unlink(path) + } threwAsync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, @@ -407,16 +395,18 @@ t.test('handle EPERMs, chmod raises something other than ENOENT', async t => { throw new Error('cannot chmod', { code: 'FOO' }) }, unlinkSync: path => { - if (threwSync) + if (threwSync) { return fs.unlinkSync(path) + } threwSync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, promises: { ...fs.promises, unlink: async path => { - if (threwAsync) + if (threwAsync) { return fs.promises.unlink(path) + } threwAsync = true throw Object.assign(new Error('cannot unlink'), { code: 'EPERM' }) }, @@ -462,8 +452,9 @@ t.test('rimraffing root, do not actually rmdir root', async t => { ...require('path'), parse: (path) => { const p = parse(path) - if (path === ROOT) + if (path === ROOT) { p.root = path + } return p }, }, @@ -482,3 +473,13 @@ t.test('rimraffing root, do not actually rmdir root', async t => { }) t.end() }) + +t.test('do not allow third arg', async t => { + const ROOT = t.testdir(fixture) + const { + rimrafWindows, + rimrafWindowsSync, + } = require('../lib/rimraf-windows.js') + t.rejects(rimrafWindows(ROOT, {}, true)) + t.throws(() => rimrafWindowsSync(ROOT, {}, true)) +})