From a5f6405861b5c881568a70dfff695b60417f83db Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 3 Oct 2024 19:08:07 -0700 Subject: [PATCH 01/50] benchmark: get cjs benchmarks to run correctly --- .prettierignore | 2 +- benchmark/.npmrc | 2 ++ benchmark/create-fixture.js | 11 ++++++----- benchmark/index.js | 9 ++++++++- benchmark/package.json | 6 ++++++ benchmark/print-results.js | 8 ++++---- benchmark/rimrafs.js | 25 +++++++++++++------------ benchmark/run-test.js | 29 +++++++++++++++++------------ 8 files changed, 57 insertions(+), 35 deletions(-) create mode 100644 benchmark/.npmrc create mode 100644 benchmark/package.json diff --git a/.prettierignore b/.prettierignore index d289a030..47415c69 100644 --- a/.prettierignore +++ b/.prettierignore @@ -8,4 +8,4 @@ /tap-snapshots /.nyc_output /coverage -/benchmark +/benchmark/old-rimraf diff --git a/benchmark/.npmrc b/benchmark/.npmrc new file mode 100644 index 00000000..42eec1e1 --- /dev/null +++ b/benchmark/.npmrc @@ -0,0 +1,2 @@ +install-links=true +package-lock=false diff --git a/benchmark/create-fixture.js b/benchmark/create-fixture.js index 99e7348a..9117f8a3 100644 --- a/benchmark/create-fixture.js +++ b/benchmark/create-fixture.js @@ -1,6 +1,8 @@ const { writeFile: writeFile_ } = require('fs') -const writeFile = async (path, data) => new Promise((res, rej) => - writeFile_(path, data, er => er ? rej(er) : res())) +const writeFile = async (path, data) => + new Promise((res, rej) => + writeFile_(path, data, er => (er ? rej(er) : res())), + ) const { mkdirp } = require('mkdirp') const { resolve } = require('path') @@ -9,10 +11,9 @@ const create = async (path, start, end, maxDepth, depth = 0) => { const promises = [] for (let i = start; i <= end; i++) { const c = String.fromCharCode(i) - if (depth < maxDepth && (i-start >= depth)) + if (depth < maxDepth && i - start >= depth) await create(resolve(path, c), start, end, maxDepth, depth + 1) - else - promises.push(writeFile(resolve(path, c), c)) + else promises.push(writeFile(resolve(path, c), c)) } await Promise.all(promises) return path diff --git a/benchmark/index.js b/benchmark/index.js index 90928a20..3ef07ac1 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,8 +1,15 @@ +const { spawnSync } = require('child_process') +const { existsSync } = require('fs') +const { resolve } = require('path') const cases = require('./rimrafs.js') const runTest = require('./run-test.js') const print = require('./print-results.js') -const rimraf = require('../') +if (!existsSync(resolve(__dirname, 'node_modules'))) { + spawnSync('npm', ['install'], { cwd: __dirname }) +} + +const rimraf = require('rimraf') const main = async () => { // cleanup first. since the windows impl works on all platforms, // use that. it's only relevant if the folder exists anyway. diff --git a/benchmark/package.json b/benchmark/package.json new file mode 100644 index 00000000..17007e50 --- /dev/null +++ b/benchmark/package.json @@ -0,0 +1,6 @@ +{ + "type": "commonjs", + "dependencies": { + "rimraf": "../" + } +} diff --git a/benchmark/print-results.js b/benchmark/print-results.js index 831e3309..b8afe8c0 100644 --- a/benchmark/print-results.js +++ b/benchmark/print-results.js @@ -11,7 +11,7 @@ const variance = list => { const stddev = list => { const v = variance(list) if (isNaN(v)) { - console.error({list, v}) + console.error({ list, v }) throw new Error('wat?') } return sqrt(variance(list)) @@ -28,8 +28,8 @@ const nums = list => ({ }) const printEr = er => `${er.code ? er.code + ': ' : ''}${er.message}` -const failures = list => list.length === 0 ? {} - : { failures: list.map(er => printEr(er)).join('\n') } +const failures = list => + list.length === 0 ? {} : { failures: list.map(er => printEr(er)).join('\n') } const table = results => { const table = {} @@ -49,7 +49,7 @@ const table = results => { } // sort by mean time return Object.entries(table) - .sort(([, {mean:a}], [, {mean:b}]) => a - b) + .sort(([, { mean: a }], [, { mean: b }]) => a - b) .reduce((set, [key, val]) => { set[key] = val return set diff --git a/benchmark/rimrafs.js b/benchmark/rimrafs.js index aefbabfe..688c21b3 100644 --- a/benchmark/rimrafs.js +++ b/benchmark/rimrafs.js @@ -1,6 +1,6 @@ // just disable the glob option, and promisify it, for apples-to-apples comp const oldRimraf = () => { - const {promisify} = require('util') + const { promisify } = require('util') const oldRimraf = require('./old-rimraf') const pOldRimraf = promisify(oldRimraf) const rimraf = path => pOldRimraf(path, { disableGlob: true }) @@ -10,15 +10,15 @@ const oldRimraf = () => { const { spawn, spawnSync } = require('child_process') const systemRmRf = () => { - const rimraf = path => new Promise((res, rej) => { - const proc = spawn('rm', ['-rf', path]) - proc.on('close', (code, signal) => { - if (code || signal) - rej(Object.assign(new Error('command failed'), { code, signal })) - else - res() + const rimraf = path => + new Promise((res, rej) => { + const proc = spawn('rm', ['-rf', path]) + proc.on('close', (code, signal) => { + if (code || signal) + rej(Object.assign(new Error('command failed'), { code, signal })) + else res() + }) }) - }) rimraf.sync = path => { const result = spawnSync('rm', ['-rf', path]) if (result.status || result.signal) { @@ -31,10 +31,11 @@ const systemRmRf = () => { return rimraf } +const { native, posix, windows } = require('rimraf') module.exports = { - native: require('../').native, - posix: require('../').posix, - windows: require('../').windows, + native, + posix, + windows, old: oldRimraf(), system: systemRmRf(), } diff --git a/benchmark/run-test.js b/benchmark/run-test.js index a9fe6da4..a1ee2402 100644 --- a/benchmark/run-test.js +++ b/benchmark/run-test.js @@ -7,12 +7,11 @@ const cases = require('./rimrafs.js') const create = require('./create-fixture.js') -const hrToMS = hr => Math.round(hr[0]*1e9 + hr[1]) / 1e6 +const hrToMS = hr => Math.round(hr[0] * 1e9 + hr[1]) / 1e6 -const runTest = async (type) => { +const runTest = async type => { const rimraf = cases[type] - if (!rimraf) - throw new Error('unknown rimraf type: ' + type) + if (!rimraf) throw new Error('unknown rimraf type: ' + type) const opt = { start: START, @@ -62,10 +61,12 @@ const runTest = async (type) => { const startAsync = process.hrtime() for (const path of asyncPaths) { const start = process.hrtime() - await rimraf(path).then( - () => asyncTimes.push(hrToMS(process.hrtime(start))), - er => asyncFails.push(er) - ).then(() => process.stderr.write('.')) + await rimraf(path) + .then( + () => asyncTimes.push(hrToMS(process.hrtime(start))), + er => asyncFails.push(er), + ) + .then(() => process.stderr.write('.')) } const asyncTotal = hrToMS(process.hrtime(startAsync)) console.error('done! (%j ms, %j failed)', asyncTotal, asyncFails.length) @@ -77,10 +78,14 @@ const runTest = async (type) => { const paraRuns = [] for (const path of paraPaths) { const start = process.hrtime() - paraRuns.push(rimraf(path).then( - () => paraTimes.push(hrToMS(process.hrtime(start))), - er => paraFails.push(er) - ).then(() => process.stderr.write('.'))) + paraRuns.push( + rimraf(path) + .then( + () => paraTimes.push(hrToMS(process.hrtime(start))), + er => paraFails.push(er), + ) + .then(() => process.stderr.write('.')), + ) } await Promise.all(paraRuns) const paraTotal = hrToMS(process.hrtime(startPara)) From 31092140ebbe26bda3c66e4ba70bc22273ef7635 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 3 Oct 2024 20:27:37 -0700 Subject: [PATCH 02/50] benchmark: convert to esm --- benchmark/.npmrc | 2 -- benchmark/create-fixture.js | 10 +++++----- benchmark/index.js | 19 ++++++------------- benchmark/package.json | 6 ------ benchmark/print-results.js | 2 +- benchmark/rimrafs.js | 11 ++++++----- benchmark/run-test.js | 6 +++--- 7 files changed, 21 insertions(+), 35 deletions(-) delete mode 100644 benchmark/.npmrc delete mode 100644 benchmark/package.json diff --git a/benchmark/.npmrc b/benchmark/.npmrc deleted file mode 100644 index 42eec1e1..00000000 --- a/benchmark/.npmrc +++ /dev/null @@ -1,2 +0,0 @@ -install-links=true -package-lock=false diff --git a/benchmark/create-fixture.js b/benchmark/create-fixture.js index 9117f8a3..3ce172ef 100644 --- a/benchmark/create-fixture.js +++ b/benchmark/create-fixture.js @@ -1,10 +1,10 @@ -const { writeFile: writeFile_ } = require('fs') +import { writeFile as writeFile_ } from 'fs' const writeFile = async (path, data) => new Promise((res, rej) => writeFile_(path, data, er => (er ? rej(er) : res())), ) -const { mkdirp } = require('mkdirp') -const { resolve } = require('path') +import { mkdirp } from 'mkdirp' +import { resolve } from 'path' const create = async (path, start, end, maxDepth, depth = 0) => { await mkdirp(path) @@ -19,7 +19,7 @@ const create = async (path, start, end, maxDepth, depth = 0) => { return path } -module.exports = async ({ start, end, depth, name }) => { - const path = resolve(__dirname, 'fixtures', name, 'test') +export default async ({ start, end, depth, name }) => { + const path = resolve(import.meta.dirname, 'fixtures', name, 'test') return await create(path, start.charCodeAt(0), end.charCodeAt(0), depth) } diff --git a/benchmark/index.js b/benchmark/index.js index 3ef07ac1..d5a45d75 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,24 +1,17 @@ -const { spawnSync } = require('child_process') -const { existsSync } = require('fs') -const { resolve } = require('path') -const cases = require('./rimrafs.js') -const runTest = require('./run-test.js') -const print = require('./print-results.js') +import cases from './rimrafs.js' +import runTest from './run-test.js' +import print from './print-results.js' -if (!existsSync(resolve(__dirname, 'node_modules'))) { - spawnSync('npm', ['install'], { cwd: __dirname }) -} - -const rimraf = require('rimraf') +import * as rimraf from '../dist/esm/index.js' const main = async () => { // cleanup first. since the windows impl works on all platforms, // use that. it's only relevant if the folder exists anyway. - rimraf.sync(__dirname + '/fixtures') + rimraf.sync(import.meta.dirname + '/fixtures') const results = {} for (const name of Object.keys(cases)) { results[name] = await runTest(name) } - rimraf.sync(__dirname + '/fixtures') + rimraf.sync(import.meta.dirname + '/fixtures') return results } diff --git a/benchmark/package.json b/benchmark/package.json deleted file mode 100644 index 17007e50..00000000 --- a/benchmark/package.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "type": "commonjs", - "dependencies": { - "rimraf": "../" - } -} diff --git a/benchmark/print-results.js b/benchmark/print-results.js index b8afe8c0..61f7783c 100644 --- a/benchmark/print-results.js +++ b/benchmark/print-results.js @@ -62,4 +62,4 @@ const print = results => { console.table(table(results)) } -module.exports = print +export default print diff --git a/benchmark/rimrafs.js b/benchmark/rimrafs.js index 688c21b3..f4da741f 100644 --- a/benchmark/rimrafs.js +++ b/benchmark/rimrafs.js @@ -1,14 +1,15 @@ // just disable the glob option, and promisify it, for apples-to-apples comp +import { promisify } from 'util' +import { createRequire } from 'module' const oldRimraf = () => { - const { promisify } = require('util') - const oldRimraf = require('./old-rimraf') + const oldRimraf = createRequire(import.meta.filename)('./old-rimraf') const pOldRimraf = promisify(oldRimraf) const rimraf = path => pOldRimraf(path, { disableGlob: true }) const sync = path => oldRimraf.sync(path, { disableGlob: true }) return Object.assign(rimraf, { sync }) } -const { spawn, spawnSync } = require('child_process') +import { spawn, spawnSync } from 'child_process' const systemRmRf = () => { const rimraf = path => new Promise((res, rej) => { @@ -31,8 +32,8 @@ const systemRmRf = () => { return rimraf } -const { native, posix, windows } = require('rimraf') -module.exports = { +import { native, posix, windows } from 'rimraf' +export default { native, posix, windows, diff --git a/benchmark/run-test.js b/benchmark/run-test.js index a1ee2402..4a2f0b8b 100644 --- a/benchmark/run-test.js +++ b/benchmark/run-test.js @@ -3,9 +3,9 @@ const END = process.env.RIMRAF_TEST_END_CHAR || 'f' const DEPTH = +process.env.RIMRAF_TEST_DEPTH || 5 const N = +process.env.RIMRAF_TEST_ITERATIONS || 7 -const cases = require('./rimrafs.js') +import cases from './rimrafs.js' -const create = require('./create-fixture.js') +import create from './create-fixture.js' const hrToMS = hr => Math.round(hr[0] * 1e9 + hr[1]) / 1e6 @@ -102,4 +102,4 @@ const runTest = async type => { })) } -module.exports = runTest +export default runTest From efeafd695a43fe58c07d0beb410531e1c86f2ce4 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 3 Oct 2024 20:48:48 -0700 Subject: [PATCH 03/50] benchmark: add options to filter and compare benchmarks --- .github/workflows/benchmark.yml | 6 +- .gitignore | 1 + benchmark/index.js | 136 +++++++++++++++++++++++--- benchmark/parse-results.js | 64 +++++++++++++ benchmark/print-results.js | 65 ------------- benchmark/rimrafs.js | 4 +- benchmark/run-test.js | 164 +++++++++++++++++++------------- 7 files changed, 291 insertions(+), 149 deletions(-) create mode 100644 benchmark/parse-results.js delete mode 100644 benchmark/print-results.js diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index d5ddb0ea..7fa2543b 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -31,8 +31,4 @@ jobs: run: npm install - name: benchmark - run: node benchmark/index.js - env: - RIMRAF_TEST_START_CHAR: a - RIMRAF_TEST_END_CHAR: f - RIMRAF_TEST_DEPTH: 5 + run: node benchmark/index.js --start-char=a --end-char=f --depth=5 diff --git a/.gitignore b/.gitignore index 5ae2c2bf..181a4d04 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ !/typedoc.json !/tsconfig-*.json !/.prettierignore +/benchmark-*.json diff --git a/benchmark/index.js b/benchmark/index.js index d5a45d75..acf1c85a 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,18 +1,132 @@ -import cases from './rimrafs.js' -import runTest from './run-test.js' -import print from './print-results.js' +import rimrafs, { names as rimrafNames } from './rimrafs.js' +import runTest, { names as runTestNames } from './run-test.js' +import parse from './parse-results.js' +import { sync as rimrafSync } from '../dist/esm/index.js' +import { parseArgs } from 'util' +import assert from 'assert' +import { readFileSync, writeFileSync } from 'fs' + +const parseOptions = () => { + const { values } = parseArgs({ + options: { + cases: { + type: 'string', + short: 'c', + multiple: true, + }, + 'omit-cases': { + type: 'string', + short: 'o', + multiple: true, + }, + 'start-char': { + type: 'string', + default: 'a', + }, + 'end-char': { + type: 'string', + default: 'f', + }, + depth: { + type: 'string', + default: '5', + }, + iterations: { + type: 'string', + default: '7', + }, + compare: { + type: 'string', + }, + save: { + type: 'boolean', + }, + }, + }) + + if (values.compare) { + const { results, options } = JSON.parse( + readFileSync(values.compare, 'utf8'), + ) + return { + ...options, + save: false, + compare: results, + } + } + + const allNames = new Set([...rimrafNames, ...runTestNames]) + const partition = (name, defaults = [new Set(), new Set()]) => { + const options = values[name] ?? [] + assert( + options.every(c => allNames.has(c)), + new TypeError(`invalid ${name}`, { + cause: { + found: options, + wanted: [...allNames], + }, + }), + ) + const found = options.reduce( + (acc, k) => { + acc[rimrafNames.has(k) ? 0 : 1].add(k) + return acc + }, + [new Set(), new Set()], + ) + return [ + found[0].size ? found[0] : defaults[0], + found[1].size ? found[1] : defaults[1], + ] + } + + const cases = partition('cases', [rimrafNames, runTestNames]) + for (const [i, omitCase] of Object.entries(partition('omit-cases'))) { + for (const o of omitCase) { + cases[i].delete(o) + } + } + + return { + rimraf: [...cases[0]], + runTest: [...cases[1]], + start: values['start-char'], + end: values['end-char'], + depth: +values.depth, + iterations: +values.iterations, + save: values.save, + compare: null, + } +} -import * as rimraf from '../dist/esm/index.js' const main = async () => { // cleanup first. since the windows impl works on all platforms, // use that. it's only relevant if the folder exists anyway. - rimraf.sync(import.meta.dirname + '/fixtures') - const results = {} - for (const name of Object.keys(cases)) { - results[name] = await runTest(name) + rimrafSync(import.meta.dirname + '/fixtures') + const data = {} + const { save, compare, ...options } = parseOptions() + for (const [name, rimraf] of Object.entries(rimrafs)) { + if (options.rimraf.includes(name)) { + data[name] = await runTest(name, rimraf, options) + } + } + rimrafSync(import.meta.dirname + '/fixtures') + const { results, entries } = parse(data, compare) + if (save) { + const f = `benchmark-${Date.now()}.json` + writeFileSync(f, JSON.stringify({ options, results }, 0, 2)) + console.log(`results saved to ${f}`) + } else { + console.log(JSON.stringify(results, null, 2)) } - rimraf.sync(import.meta.dirname + '/fixtures') - return results + console.table( + entries + .sort(([, { mean: a }], [, { mean: b }]) => a - b) + .reduce((set, [key, val]) => { + set[key] = val + return set + }, {}), + ) } -main().then(print) +main() diff --git a/benchmark/parse-results.js b/benchmark/parse-results.js new file mode 100644 index 00000000..12d39dbf --- /dev/null +++ b/benchmark/parse-results.js @@ -0,0 +1,64 @@ +const sum = list => list.reduce((a, b) => a + b) +const mean = list => sum(list) / list.length +const median = list => list.sort()[Math.floor(list.length / 2)] +const max = list => list.sort()[list.length - 1] +const min = list => list.sort()[0] +const sqrt = n => Math.pow(n, 0.5) +const variance = list => { + const m = mean(list) + return mean(list.map(n => Math.pow(n - m, 2))) +} +const stddev = list => { + const v = variance(list) + if (isNaN(v)) { + throw new Error('wat?', { cause: { list, v } }) + } + return sqrt(variance(list)) +} +const comp = (v1, v2) => { + if (v1 === undefined) { + return {} + } + return { + 'old mean': v1.mean, + '% +/-': round(((v2.mean - v1.mean) / v1.mean) * 100), + } +} + +const round = n => Math.round(n * 1e3) / 1e3 + +const nums = list => ({ + mean: round(mean(list)), + median: round(median(list)), + stddev: round(stddev(list)), + max: round(max(list)), + min: round(min(list)), +}) + +const printEr = er => `${er.code ? er.code + ': ' : ''}${er.message}` + +const parseResults = (data, compare) => { + const results = {} + const table = {} + + for (const [rimrafName, rimrafData] of Object.entries(data)) { + results[rimrafName] = {} + for (const [runTestName, { times, fails }] of Object.entries(rimrafData)) { + const result = nums(times) + const failures = fails.map(printEr) + results[rimrafName][runTestName] = { ...result, times, failures } + table[`${rimrafName} ${runTestName}`] = { + ...result, + ...comp(compare?.[rimrafName]?.[runTestName], result), + ...(failures.length ? { failures: failures.join('\n') } : {}), + } + } + } + + return { + results, + entries: Object.entries(table), + } +} + +export default parseResults diff --git a/benchmark/print-results.js b/benchmark/print-results.js deleted file mode 100644 index 61f7783c..00000000 --- a/benchmark/print-results.js +++ /dev/null @@ -1,65 +0,0 @@ -const sum = list => list.reduce((a, b) => a + b) -const mean = list => sum(list) / list.length -const median = list => list.sort()[Math.floor(list.length / 2)] -const max = list => list.sort()[list.length - 1] -const min = list => list.sort()[0] -const sqrt = n => Math.pow(n, 0.5) -const variance = list => { - const m = mean(list) - return mean(list.map(n => Math.pow(n - m, 2))) -} -const stddev = list => { - const v = variance(list) - if (isNaN(v)) { - console.error({ list, v }) - throw new Error('wat?') - } - return sqrt(variance(list)) -} - -const round = n => Math.round(n * 1e3) / 1e3 - -const nums = list => ({ - mean: round(mean(list)), - median: round(median(list)), - stddev: round(stddev(list)), - max: round(max(list)), - min: round(min(list)), -}) - -const printEr = er => `${er.code ? er.code + ': ' : ''}${er.message}` -const failures = list => - list.length === 0 ? {} : { failures: list.map(er => printEr(er)).join('\n') } - -const table = results => { - const table = {} - for (const [type, data] of Object.entries(results)) { - table[`${type} sync`] = { - ...nums(data.syncTimes), - ...failures(data.syncFails), - } - table[`${type} async`] = { - ...nums(data.asyncTimes), - ...failures(data.asyncFails), - } - table[`${type} parallel`] = { - ...nums(data.paraTimes), - ...failures(data.paraFails), - } - } - // sort by mean time - return Object.entries(table) - .sort(([, { mean: a }], [, { mean: b }]) => a - b) - .reduce((set, [key, val]) => { - set[key] = val - return set - }, {}) -} - -const print = results => { - console.log(JSON.stringify(results, 0, 2)) - console.log('Results sorted by fastest mean value') - console.table(table(results)) -} - -export default print diff --git a/benchmark/rimrafs.js b/benchmark/rimrafs.js index f4da741f..8de19cea 100644 --- a/benchmark/rimrafs.js +++ b/benchmark/rimrafs.js @@ -33,10 +33,12 @@ const systemRmRf = () => { } import { native, posix, windows } from 'rimraf' -export default { +const cases = { native, posix, windows, old: oldRimraf(), system: systemRmRf(), } +export const names = new Set(Object.keys(cases)) +export default cases diff --git a/benchmark/run-test.js b/benchmark/run-test.js index 4a2f0b8b..f64a74ee 100644 --- a/benchmark/run-test.js +++ b/benchmark/run-test.js @@ -1,105 +1,135 @@ -const START = process.env.RIMRAF_TEST_START_CHAR || 'a' -const END = process.env.RIMRAF_TEST_END_CHAR || 'f' -const DEPTH = +process.env.RIMRAF_TEST_DEPTH || 5 -const N = +process.env.RIMRAF_TEST_ITERATIONS || 7 - -import cases from './rimrafs.js' - import create from './create-fixture.js' -const hrToMS = hr => Math.round(hr[0] * 1e9 + hr[1]) / 1e6 +const TESTS = { + sync: 'sync', + async: 'async', + parallel: 'parallel', +} -const runTest = async type => { - const rimraf = cases[type] - if (!rimraf) throw new Error('unknown rimraf type: ' + type) +const hrToMS = hr => Math.round(hr[0] * 1e9 + hr[1]) / 1e6 - const opt = { - start: START, - end: END, - depth: DEPTH, - } - console.error(`\nrunning test for ${type}, iterations=${N} %j...`, opt) +const runTest = async ( + type, + rimraf, + { runTest: cases, start, end, depth, iterations }, +) => { + console.error(`\nrunning test for ${type}, %j`, { + start, + end, + depth, + iterations, + }) // first, create all fixtures const syncPaths = [] const asyncPaths = [] const paraPaths = [] - process.stderr.write('creating fixtures...') - for (let i = 0; i < N; i++) { + process.stderr.write('creating fixtures') + for (let i = 0; i < iterations; i++) { const [syncPath, asyncPath, paraPath] = await Promise.all([ - create({ name: `${type}/sync/${i}`, ...opt }), - create({ name: `${type}/async/${i}`, ...opt }), - create({ name: `${type}/para/${i}`, ...opt }), + cases.includes(TESTS.sync) ? + create({ name: `${type}/sync/${i}`, start, end, depth }) + : null, + cases.includes(TESTS.async) ? + create({ name: `${type}/async/${i}`, start, end, depth }) + : null, + cases.includes(TESTS.parallel) ? + create({ name: `${type}/para/${i}`, start, end, depth }) + : null, ]) - syncPaths.push(syncPath) - asyncPaths.push(asyncPath) - paraPaths.push(paraPath) + syncPath && syncPaths.push(syncPath) + asyncPath && asyncPaths.push(asyncPath) + paraPath && paraPaths.push(paraPath) process.stderr.write('.') } - console.error('done!') + process.stderr.write('done!\n') const syncTimes = [] const syncFails = [] - process.stderr.write('running sync tests...') - const startSync = process.hrtime() - for (const path of syncPaths) { - const start = process.hrtime() - try { - rimraf.sync(path) - syncTimes.push(hrToMS(process.hrtime(start))) - } catch (er) { - syncFails.push(er) + if (syncPaths.length) { + process.stderr.write('running sync tests') + const startSync = process.hrtime() + for (const path of syncPaths) { + const start = process.hrtime() + try { + rimraf.sync(path) + syncTimes.push(hrToMS(process.hrtime(start))) + } catch (er) { + syncFails.push(er) + } + process.stderr.write('.') } - process.stderr.write('.') + const syncTotal = hrToMS(process.hrtime(startSync)) + console.error('done! (%j ms, %j failed)', syncTotal, syncFails.length) } - const syncTotal = hrToMS(process.hrtime(startSync)) - console.error('done! (%j ms, %j failed)', syncTotal, syncFails.length) const asyncTimes = [] const asyncFails = [] - process.stderr.write('running async tests...') - const startAsync = process.hrtime() - for (const path of asyncPaths) { - const start = process.hrtime() - await rimraf(path) - .then( + if (asyncPaths.length) { + process.stderr.write('running async tests') + const startAsync = process.hrtime() + for (const path of asyncPaths) { + const start = process.hrtime() + await rimraf(path).then( () => asyncTimes.push(hrToMS(process.hrtime(start))), er => asyncFails.push(er), ) - .then(() => process.stderr.write('.')) + process.stderr.write('.') + } + const asyncTotal = hrToMS(process.hrtime(startAsync)) + console.error('done! (%j ms, %j failed)', asyncTotal, asyncFails.length) } - const asyncTotal = hrToMS(process.hrtime(startAsync)) - console.error('done! (%j ms, %j failed)', asyncTotal, asyncFails.length) const paraTimes = [] const paraFails = [] - process.stderr.write('running parallel tests...') - const startPara = process.hrtime() - const paraRuns = [] - for (const path of paraPaths) { - const start = process.hrtime() - paraRuns.push( - rimraf(path) - .then( + if (paraPaths.length) { + process.stderr.write('running parallel tests') + const startPara = process.hrtime() + const paraRuns = [] + for (const path of paraPaths) { + process.stderr.write('.') + const start = process.hrtime() + paraRuns.push( + rimraf(path).then( () => paraTimes.push(hrToMS(process.hrtime(start))), er => paraFails.push(er), - ) - .then(() => process.stderr.write('.')), - ) + ), + ) + } + await Promise.all(paraRuns) + const paraTotal = hrToMS(process.hrtime(startPara)) + console.error('done! (%j ms, %j failed)', paraTotal, paraFails.length) } - await Promise.all(paraRuns) - const paraTotal = hrToMS(process.hrtime(startPara)) - console.error('done! (%j ms, %j failed)', paraTotal, paraFails.length) + process.stderr.write('\n') // wait a tick to let stderr to clear return Promise.resolve().then(() => ({ - syncTimes, - syncFails, - asyncTimes, - asyncFails, - paraTimes, - paraFails, + ...(syncPaths.length ? + { + sync: { + times: syncTimes, + fails: syncFails, + }, + } + : {}), + ...(asyncPaths.length ? + { + async: { + times: asyncTimes, + fails: asyncFails, + }, + } + : {}), + ...(paraPaths.length ? + { + parallel: { + times: paraTimes, + fails: paraFails, + }, + } + : {}), })) } +export const names = new Set(Object.values(TESTS)) export default runTest From 9cbab08392bfb85f356d6a26c04c673b58d22578 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 16:54:27 -0700 Subject: [PATCH 04/50] wip --- src/rimraf-windows.ts | 17 ++++++++++------- test/integration/eperm.ts | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 test/integration/eperm.ts diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 30624986..d35a7f2a 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -3,8 +3,8 @@ // 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. +// 3. If we hit ENOTEMPTY or EPERM in the second sweep, fall back to +// move-remove on the that folder. // // Note: "move then remove" is 2-10 times slower, and just as unreliable. @@ -38,7 +38,8 @@ const rimrafWindowsDirMoveRemoveFallback = async ( try { return await rimrafWindowsDirRetry(path, options) } catch (er) { - if ((er as NodeJS.ErrnoException)?.code === 'ENOTEMPTY') { + const code = (er as NodeJS.ErrnoException)?.code + if (code === 'ENOTEMPTY') { return await rimrafMoveRemove(path, options) } throw er @@ -57,8 +58,8 @@ const rimrafWindowsDirMoveRemoveFallbackSync = ( try { return rimrafWindowsDirRetrySync(path, options) } catch (er) { - const fer = er as NodeJS.ErrnoException - if (fer?.code === 'ENOTEMPTY') { + const code = (er as NodeJS.ErrnoException)?.code + if (code === 'ENOTEMPTY') { return rimrafMoveRemoveSync(path, options) } throw er @@ -76,7 +77,8 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { try { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { - if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true + const code = (er as NodeJS.ErrnoException)?.code + if (code === 'ENOENT') return true throw er } } @@ -88,7 +90,8 @@ export const rimrafWindowsSync = (path: string, opt: RimrafSyncOptions) => { try { return rimrafWindowsDirSync(path, opt, lstatSync(path), START) } catch (er) { - if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true + const code = (er as NodeJS.ErrnoException)?.code + if (code === 'ENOENT') return true throw er } } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts new file mode 100644 index 00000000..b7ba3821 --- /dev/null +++ b/test/integration/eperm.ts @@ -0,0 +1,33 @@ +import { mkdirSync, readdirSync } from 'fs' +import { resolve } from 'path' +import { glob } from 'glob' +import t from 'tap' +import { rimraf } from '../../src/index.js' + +// Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 +// that this test would throw EPERM errors consistently in Windows CI environments. I'm not sure +// how much of the test structure is relevant to the error but I've copied it as closely as possible. +// https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 +t.test('does not throw EPERM - async', async t => { + const dir = t.testdir() + const nested = resolve(dir, 'a/b/c/nested.js') + const totalAttempts = 200 + + let count = 0 + while (count !== totalAttempts) { + mkdirSync(nested, { recursive: true }) + const entries = [] + for (const entry of await glob('**/*', { cwd: dir, dot: true }).then(r => + r.sort((a, b) => b.localeCompare(a)), + )) { + await rimraf(resolve(dir, entry), { glob: false }) + entries.push(entry) + } + t.strictSame(entries, ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a']) + + count += 1 + } + + t.strictSame(readdirSync(dir), []) + t.equal(count, totalAttempts) +}) From b02875214fc7d89cf454fbcf395e58fd7d24fe58 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 16:57:31 -0700 Subject: [PATCH 05/50] fix paths --- test/integration/eperm.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index b7ba3821..c3a92354 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,5 +1,5 @@ import { mkdirSync, readdirSync } from 'fs' -import { resolve } from 'path' +import { resolve, sep } from 'path' import { glob } from 'glob' import t from 'tap' import { rimraf } from '../../src/index.js' @@ -23,7 +23,10 @@ t.test('does not throw EPERM - async', async t => { await rimraf(resolve(dir, entry), { glob: false }) entries.push(entry) } - t.strictSame(entries, ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a']) + t.strictSame( + entries, + ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a'].map(p => p.replaceAll('/', sep)), + ) count += 1 } From 8644b652d819f7eab515f1fc9c79c1d03437d2ef Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:07:21 -0700 Subject: [PATCH 06/50] promise all --- test/integration/eperm.ts | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index c3a92354..fe1cf809 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,15 +1,20 @@ -import { mkdirSync, readdirSync } from 'fs' -import { resolve, sep } from 'path' +import { mkdirSync, readdirSync, realpathSync } from 'fs' +import { resolve, sep, join } from 'path' import { glob } from 'glob' import t from 'tap' +import os from 'node:os' import { rimraf } from '../../src/index.js' +import { randomBytes } from 'crypto' // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. I'm not sure // how much of the test structure is relevant to the error but I've copied it as closely as possible. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('does not throw EPERM - async', async t => { - const dir = t.testdir() + const dir = join( + realpathSync(os.tmpdir()), + `_${randomBytes(6).toString('hex')}`, + ) const nested = resolve(dir, 'a/b/c/nested.js') const totalAttempts = 200 @@ -17,15 +22,13 @@ t.test('does not throw EPERM - async', async t => { while (count !== totalAttempts) { mkdirSync(nested, { recursive: true }) const entries = [] - for (const entry of await glob('**/*', { cwd: dir, dot: true }).then(r => - r.sort((a, b) => b.localeCompare(a)), - )) { - await rimraf(resolve(dir, entry), { glob: false }) - entries.push(entry) - } + const files = await glob('**/*', { cwd: dir, dot: true }).then(r => + r.sort((a, b) => b.localeCompare(a)).map(p => resolve(dir, p)), + ) + await Promise.all(files.map(f => rimraf(f, { glob: false }))) t.strictSame( - entries, - ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a'].map(p => p.replaceAll('/', sep)), + files, + ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a'].map(p => resolve(dir, p)), ) count += 1 From 82034be16f3dedd9551711a814a52c18500b983e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:17:55 -0700 Subject: [PATCH 07/50] dont sort --- test/integration/eperm.ts | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index fe1cf809..530aa79d 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,5 +1,5 @@ import { mkdirSync, readdirSync, realpathSync } from 'fs' -import { resolve, sep, join } from 'path' +import { sep, join } from 'path' import { glob } from 'glob' import t from 'tap' import os from 'node:os' @@ -15,22 +15,18 @@ t.test('does not throw EPERM - async', async t => { realpathSync(os.tmpdir()), `_${randomBytes(6).toString('hex')}`, ) - const nested = resolve(dir, 'a/b/c/nested.js') + const nested = join('a/b/c/d/e/f/g') + const expected = nested + .split(sep) + .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) const totalAttempts = 200 let count = 0 while (count !== totalAttempts) { - mkdirSync(nested, { recursive: true }) - const entries = [] - const files = await glob('**/*', { cwd: dir, dot: true }).then(r => - r.sort((a, b) => b.localeCompare(a)).map(p => resolve(dir, p)), - ) - await Promise.all(files.map(f => rimraf(f, { glob: false }))) - t.strictSame( - files, - ['a/b/c/nested.js', 'a/b/c', 'a/b', 'a'].map(p => resolve(dir, p)), - ) - + mkdirSync(join(dir, nested), { recursive: true }) + const entries = await glob('**/*', { cwd: dir, dot: true }) + await Promise.all(entries.map(e => rimraf(join(dir, e), { glob: false }))) + t.strictSame(entries, expected) count += 1 } From 25d26c6ac555f8215a9dc8ae920f41ff44cfb791 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:26:20 -0700 Subject: [PATCH 08/50] randomize array order before rimraf --- test/integration/eperm.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 530aa79d..edde213d 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,6 +1,6 @@ import { mkdirSync, readdirSync, realpathSync } from 'fs' import { sep, join } from 'path' -import { glob } from 'glob' +import { globSync } from 'glob' import t from 'tap' import os from 'node:os' import { rimraf } from '../../src/index.js' @@ -11,25 +11,34 @@ import { randomBytes } from 'crypto' // how much of the test structure is relevant to the error but I've copied it as closely as possible. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('does not throw EPERM - async', async t => { + const iterations = 200 + const depth = 7 + const dir = join( realpathSync(os.tmpdir()), `_${randomBytes(6).toString('hex')}`, ) - const nested = join('a/b/c/d/e/f/g') + const nested = join( + ...new Array(depth).fill(0).map((_, i) => (10 + i).toString(36)), + ) const expected = nested .split(sep) .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) - const totalAttempts = 200 let count = 0 - while (count !== totalAttempts) { + while (count !== iterations) { mkdirSync(join(dir, nested), { recursive: true }) - const entries = await glob('**/*', { cwd: dir, dot: true }) + const entries = globSync('**/*', { cwd: dir, dot: true }).sort( + () => 0.5 - Math.random(), + ) await Promise.all(entries.map(e => rimraf(join(dir, e), { glob: false }))) - t.strictSame(entries, expected) + t.strictSame( + entries.sort((a, b) => a.localeCompare(b, 'en')), + expected, + ) count += 1 } t.strictSame(readdirSync(dir), []) - t.equal(count, totalAttempts) + t.equal(count, iterations) }) From 7457f3cb269466715ea6e5d566f929411e219604 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:28:58 -0700 Subject: [PATCH 09/50] dont use tmpdir --- test/integration/eperm.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index edde213d..ad73e4d8 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,10 +1,8 @@ -import { mkdirSync, readdirSync, realpathSync } from 'fs' +import t from 'tap' +import { mkdirSync, readdirSync } from 'fs' import { sep, join } from 'path' import { globSync } from 'glob' -import t from 'tap' -import os from 'node:os' import { rimraf } from '../../src/index.js' -import { randomBytes } from 'crypto' // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. I'm not sure @@ -14,10 +12,7 @@ t.test('does not throw EPERM - async', async t => { const iterations = 200 const depth = 7 - const dir = join( - realpathSync(os.tmpdir()), - `_${randomBytes(6).toString('hex')}`, - ) + const dir = t.testdir() const nested = join( ...new Array(depth).fill(0).map((_, i) => (10 + i).toString(36)), ) @@ -28,12 +23,12 @@ t.test('does not throw EPERM - async', async t => { let count = 0 while (count !== iterations) { mkdirSync(join(dir, nested), { recursive: true }) - const entries = globSync('**/*', { cwd: dir, dot: true }).sort( + const del = globSync('**/*', { cwd: dir, dot: true }).sort( () => 0.5 - Math.random(), ) - await Promise.all(entries.map(e => rimraf(join(dir, e), { glob: false }))) + await Promise.all(del.map(d => rimraf(join(dir, d), { glob: false }))) t.strictSame( - entries.sort((a, b) => a.localeCompare(b, 'en')), + del.sort((a, b) => a.localeCompare(b, 'en')), expected, ) count += 1 From 4d78f558787e6a8aa1463abc17186fa5903f91fe Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:33:40 -0700 Subject: [PATCH 10/50] test readdir contents in each loop --- test/integration/eperm.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index ad73e4d8..56c5a026 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -5,8 +5,7 @@ import { globSync } from 'glob' import { rimraf } from '../../src/index.js' // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 -// that this test would throw EPERM errors consistently in Windows CI environments. I'm not sure -// how much of the test structure is relevant to the error but I've copied it as closely as possible. +// that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('does not throw EPERM - async', async t => { const iterations = 200 @@ -31,9 +30,9 @@ t.test('does not throw EPERM - async', async t => { del.sort((a, b) => a.localeCompare(b, 'en')), expected, ) + t.strictSame(readdirSync(dir), []) count += 1 } - t.strictSame(readdirSync(dir), []) t.equal(count, iterations) }) From a233db82d27b5b24c109dbda30833471368f5ddb Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:37:54 -0700 Subject: [PATCH 11/50] sort fns --- test/integration/eperm.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 56c5a026..4a92cccd 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -4,13 +4,16 @@ import { sep, join } from 'path' import { globSync } from 'glob' import { rimraf } from '../../src/index.js' +const rando = (arr: T[]): T[] => [...arr].sort(() => 0.5 - Math.random()) +const sortAlpha = (arr: string[]) => + [...arr].sort((a, b) => a.localeCompare(b, 'en')) + // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('does not throw EPERM - async', async t => { const iterations = 200 const depth = 7 - const dir = t.testdir() const nested = join( ...new Array(depth).fill(0).map((_, i) => (10 + i).toString(36)), @@ -22,17 +25,11 @@ t.test('does not throw EPERM - async', async t => { let count = 0 while (count !== iterations) { mkdirSync(join(dir, nested), { recursive: true }) - const del = globSync('**/*', { cwd: dir, dot: true }).sort( - () => 0.5 - Math.random(), - ) - await Promise.all(del.map(d => rimraf(join(dir, d), { glob: false }))) - t.strictSame( - del.sort((a, b) => a.localeCompare(b, 'en')), - expected, - ) + const del = rando(globSync('**/*', { cwd: dir })) + await Promise.all(del.map(d => rimraf(join(dir, d)))) + t.strictSame(sortAlpha(del), expected) t.strictSame(readdirSync(dir), []) count += 1 } - t.equal(count, iterations) }) From 603e5693e3bb8b7043a5276b39e4f8de43cc0d31 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:44:05 -0700 Subject: [PATCH 12/50] move readdir back out --- test/integration/eperm.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 4a92cccd..6d283dc0 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -11,10 +11,10 @@ const sortAlpha = (arr: string[]) => // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('does not throw EPERM - async', async t => { +t.test('does not throw EPERM', async t => { const iterations = 200 const depth = 7 - const dir = t.testdir() + const cwd = t.testdir() const nested = join( ...new Array(depth).fill(0).map((_, i) => (10 + i).toString(36)), ) @@ -24,12 +24,12 @@ t.test('does not throw EPERM - async', async t => { let count = 0 while (count !== iterations) { - mkdirSync(join(dir, nested), { recursive: true }) - const del = rando(globSync('**/*', { cwd: dir })) - await Promise.all(del.map(d => rimraf(join(dir, d)))) + mkdirSync(join(cwd, nested), { recursive: true }) + const del = rando(globSync('**/*', { cwd })) + await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))) t.strictSame(sortAlpha(del), expected) - t.strictSame(readdirSync(dir), []) count += 1 } + t.strictSame(readdirSync(cwd), []) t.equal(count, iterations) }) From 2fec6116acd1ef5db0f85496a36a1520f8e9fa58 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:46:59 -0700 Subject: [PATCH 13/50] faster testing for now --- .github/workflows/ci.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73c0e451..08b2e7cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,16 +6,17 @@ jobs: build: strategy: matrix: - node-version: [20.x, 22.x] + node-version: [22.x] + # node-version: [20.x, 22.x] platform: - - os: ubuntu-latest - shell: bash - - os: macos-latest - shell: bash + # - os: ubuntu-latest + # shell: bash + # - os: macos-latest + # shell: bash - os: windows-latest shell: bash - - os: windows-latest - shell: powershell + # - os: windows-latest + # shell: powershell fail-fast: false runs-on: ${{ matrix.platform.os }} From 7af67b39358eb6277a1c6e552f57922a90651e05 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:49:47 -0700 Subject: [PATCH 14/50] try again --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08b2e7cb..cdf5cf0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI -on: [push, pull_request] +on: [pull_request] jobs: build: From bbefba8adcc974992d8fe38eb9fa7e9f71d894a0 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:53:35 -0700 Subject: [PATCH 15/50] more --- .github/workflows/ci.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdf5cf0f..c585c99f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,13 +1,12 @@ name: CI -on: [pull_request] +on: [push, pull_request] jobs: build: strategy: matrix: - node-version: [22.x] - # node-version: [20.x, 22.x] + node-version: [20.x, 22.x] platform: # - os: ubuntu-latest # shell: bash @@ -15,8 +14,8 @@ jobs: # shell: bash - os: windows-latest shell: bash - # - os: windows-latest - # shell: powershell + - os: windows-latest + shell: powershell fail-fast: false runs-on: ${{ matrix.platform.os }} From 080cb3625dfaa202286b06db08ec78bac18c6ff7 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 17:57:05 -0700 Subject: [PATCH 16/50] pr only --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c585c99f..8b7ac672 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI -on: [push, pull_request] +on: [pull_request] jobs: build: From 85babe4538bba52b73256ce5095a4ca063ee3ed2 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:08:28 -0700 Subject: [PATCH 17/50] just one file --- .github/workflows/ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b7ac672..86081f6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,11 @@ jobs: - os: windows-latest shell: bash - os: windows-latest - shell: powershell + shell: bash + - os: windows-latest + shell: bash + - os: windows-latest + shell: bash fail-fast: false runs-on: ${{ matrix.platform.os }} @@ -36,7 +40,7 @@ jobs: run: npm install - name: Run Tests - run: npm test -- -t0 -c + run: npm test -- test/integration/eperm.ts --disable-coverage env: RIMRAF_TEST_START_CHAR: a RIMRAF_TEST_END_CHAR: f From 8ca13b6356b214fdf00f9c2088980fea5e57384e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:12:58 -0700 Subject: [PATCH 18/50] fallback to move remove on eperm --- src/rimraf-windows.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index d35a7f2a..1c731ba4 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -39,7 +39,7 @@ const rimrafWindowsDirMoveRemoveFallback = async ( return await rimrafWindowsDirRetry(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOTEMPTY') { + if (code === 'ENOTEMPTY' || code === 'EPERM') { return await rimrafMoveRemove(path, options) } throw er From 699df22223930b3d623f6093a1ecd1f68fcfdcdc Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:22:02 -0700 Subject: [PATCH 19/50] trace --- src/rimraf-windows.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 1c731ba4..172361e4 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -39,6 +39,7 @@ const rimrafWindowsDirMoveRemoveFallback = async ( return await rimrafWindowsDirRetry(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code + console.trace(code) if (code === 'ENOTEMPTY' || code === 'EPERM') { return await rimrafMoveRemove(path, options) } From b9be53d215e312c51ae8b734bac2133b9d047935 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:46:17 -0700 Subject: [PATCH 20/50] trace 2 --- src/rimraf-windows.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 172361e4..a9cb10fc 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -60,7 +60,8 @@ const rimrafWindowsDirMoveRemoveFallbackSync = ( return rimrafWindowsDirRetrySync(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOTEMPTY') { + console.trace(code) + if (code === 'ENOTEMPTY' || code === 'EPERM') { return rimrafMoveRemoveSync(path, options) } throw er From ad8249e5b3c7a681b87362a4bcb3d042d7684b07 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:55:55 -0700 Subject: [PATCH 21/50] log error --- test/integration/eperm.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 6d283dc0..1f26a864 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -26,7 +26,12 @@ t.test('does not throw EPERM', async t => { while (count !== iterations) { mkdirSync(join(cwd, nested), { recursive: true }) const del = rando(globSync('**/*', { cwd })) - await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))) + try { + await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))) + } catch (e) { + console.error(e) + throw e + } t.strictSame(sortAlpha(del), expected) count += 1 } From 91ae56ee58a2d5d3dfe3760e39ec7cac1685a56b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 18:58:30 -0700 Subject: [PATCH 22/50] log in rimrafWindows --- src/rimraf-windows.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index a9cb10fc..c89cb4e0 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -80,7 +80,8 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOENT') return true + console.trace(code) + if (code === 'ENOTEMPTY' || code === 'EPERM') return true throw er } } From b3828a06f93b1bf8c747000901538b35f46746d4 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 19:05:08 -0700 Subject: [PATCH 23/50] log entries --- src/rimraf-windows.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index c89cb4e0..3092eac8 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -39,8 +39,7 @@ const rimrafWindowsDirMoveRemoveFallback = async ( return await rimrafWindowsDirRetry(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - console.trace(code) - if (code === 'ENOTEMPTY' || code === 'EPERM') { + if (code === 'ENOTEMPTY') { return await rimrafMoveRemove(path, options) } throw er @@ -60,8 +59,7 @@ const rimrafWindowsDirMoveRemoveFallbackSync = ( return rimrafWindowsDirRetrySync(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - console.trace(code) - if (code === 'ENOTEMPTY' || code === 'EPERM') { + if (code === 'ENOTEMPTY') { return rimrafMoveRemoveSync(path, options) } throw er @@ -80,8 +78,7 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - console.trace(code) - if (code === 'ENOTEMPTY' || code === 'EPERM') return true + if (code === 'ENOTEMPTY') return true throw er } } @@ -110,6 +107,7 @@ const rimrafWindowsDir = async ( } const entries = ent.isDirectory() ? await readdirOrError(path) : null + console.trace({ entries }) if (!Array.isArray(entries)) { // this can only happen if lstat/readdir lied, or if the dir was // swapped out with a file at just the right moment. From 2183ea9a06478fd011ec60d8ecff321c73427361 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 19:31:10 -0700 Subject: [PATCH 24/50] ci --- .github/workflows/ci.yml | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86081f6a..3335ced1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI -on: [pull_request] +on: [push, pull_request] jobs: build: @@ -14,12 +14,8 @@ jobs: # shell: bash - os: windows-latest shell: bash - - os: windows-latest - shell: bash - - os: windows-latest - shell: bash - - os: windows-latest - shell: bash + # - os: windows-latest + # shell: powershell fail-fast: false runs-on: ${{ matrix.platform.os }} @@ -40,8 +36,17 @@ jobs: run: npm install - name: Run Tests - run: npm test -- test/integration/eperm.ts --disable-coverage + run: npm test -- -t0 -c env: RIMRAF_TEST_START_CHAR: a RIMRAF_TEST_END_CHAR: f RIMRAF_TEST_DEPTH: 5 + + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c + - run: npm test -- -t0 -c From 363332d0a0b87381fcafd872a798134ffbf174bf Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 19:32:15 -0700 Subject: [PATCH 25/50] ci --- .github/workflows/ci.yml | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3335ced1..20e74d3a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,17 +36,21 @@ jobs: run: npm install - name: Run Tests - run: npm test -- -t0 -c - env: - RIMRAF_TEST_START_CHAR: a - RIMRAF_TEST_END_CHAR: f - RIMRAF_TEST_DEPTH: 5 - - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c - - run: npm test -- -t0 -c + run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage + - run: npm test -- test/integration/eperm.ts --disable-coverage From ee0c42b38bd616a7a1ff4e7e228b92cf6edbbd92 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 19:39:04 -0700 Subject: [PATCH 26/50] return true on eperm --- src/rimraf-windows.ts | 5 ++++- test/integration/eperm.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 3092eac8..ec368218 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -107,7 +107,6 @@ const rimrafWindowsDir = async ( } const entries = ent.isDirectory() ? await readdirOrError(path) : null - console.trace({ entries }) if (!Array.isArray(entries)) { // this can only happen if lstat/readdir lied, or if the dir was // swapped out with a file at just the right moment. @@ -116,6 +115,10 @@ const rimrafWindowsDir = async ( if (entries.code === 'ENOENT') { return true } + if (entries.code === 'EPERM') { + console.trace(entries) + return true + } if (entries.code !== 'ENOTDIR') { throw entries } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 1f26a864..8e5d7bb6 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -32,9 +32,9 @@ t.test('does not throw EPERM', async t => { console.error(e) throw e } + t.strictSame(readdirSync(cwd), []) t.strictSame(sortAlpha(del), expected) count += 1 } - t.strictSame(readdirSync(cwd), []) t.equal(count, iterations) }) From fff80310c99819cce21bd507919745628738391b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 19:56:43 -0700 Subject: [PATCH 27/50] trace in windowsRemoveDir only --- src/rimraf-windows.ts | 5 +---- test/integration/eperm.ts | 10 ++++------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index ec368218..39c0ec3d 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -78,6 +78,7 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code + console.trace(code) if (code === 'ENOTEMPTY') return true throw er } @@ -115,10 +116,6 @@ const rimrafWindowsDir = async ( if (entries.code === 'ENOENT') { return true } - if (entries.code === 'EPERM') { - console.trace(entries) - return true - } if (entries.code !== 'ENOTDIR') { throw entries } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 8e5d7bb6..9c75c304 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -26,12 +26,10 @@ t.test('does not throw EPERM', async t => { while (count !== iterations) { mkdirSync(join(cwd, nested), { recursive: true }) const del = rando(globSync('**/*', { cwd })) - try { - await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))) - } catch (e) { - console.error(e) - throw e - } + const res = await Promise.all( + del.map(d => rimraf(join(cwd, d), { glob: false })), + ) + t.ok(res.every(Boolean)) t.strictSame(readdirSync(cwd), []) t.strictSame(sortAlpha(del), expected) count += 1 From c8e865747165c76b5797d2f6b540dab08f827fce Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 20:04:26 -0700 Subject: [PATCH 28/50] log stack --- src/rimraf-windows.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 39c0ec3d..b9516ef9 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -78,6 +78,8 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code + console.error(er) + console.error((er as NodeJS.ErrnoException).stack) console.trace(code) if (code === 'ENOTEMPTY') return true throw er From 979b863186c76c6540db3d2bab91024b6e11a29d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 4 Oct 2024 21:30:17 -0700 Subject: [PATCH 29/50] log catches --- src/rimraf-windows.ts | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index b9516ef9..87a1c4e2 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -132,16 +132,27 @@ const rimrafWindowsDir = async ( } const s = state === START ? CHILD : state - const removedAll = ( - await Promise.all( - entries.map(ent => - rimrafWindowsDir(resolve(path, ent.name), opt, ent, s), - ), - ) - ).reduce((a, b) => a && b, true) + let removedAll = false + try { + removedAll = ( + await Promise.all( + entries.map(ent => + rimrafWindowsDir(resolve(path, ent.name), opt, ent, s), + ), + ) + ).reduce((a, b) => a && b, true) + } catch (e) { + console.trace(e) + throw e + } if (state === START) { - return rimrafWindowsDir(path, opt, ent, FINISH) + try { + return await rimrafWindowsDir(path, opt, ent, FINISH) + } catch (e) { + console.trace(e) + throw e + } } else if (state === FINISH) { if (opt.preserveRoot === false && path === parse(path).root) { return false @@ -152,7 +163,12 @@ const rimrafWindowsDir = async ( if (opt.filter && !(await opt.filter(path, ent))) { return false } - await ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + try { + await ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + } catch (e) { + console.trace(e) + throw e + } } return true } From 756238f3ceb85b513e580ef3806b17155f738935 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 11:00:10 -0700 Subject: [PATCH 30/50] make sure this still fails --- src/rimraf-windows.ts | 37 +++++++++---------------------------- test/integration/eperm.ts | 38 +++++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 41 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 87a1c4e2..f27cc57e 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -78,9 +78,6 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - console.error(er) - console.error((er as NodeJS.ErrnoException).stack) - console.trace(code) if (code === 'ENOTEMPTY') return true throw er } @@ -132,27 +129,16 @@ const rimrafWindowsDir = async ( } const s = state === START ? CHILD : state - let removedAll = false - try { - removedAll = ( - await Promise.all( - entries.map(ent => - rimrafWindowsDir(resolve(path, ent.name), opt, ent, s), - ), - ) - ).reduce((a, b) => a && b, true) - } catch (e) { - console.trace(e) - throw e - } + const removedAll = ( + await Promise.all( + entries.map(ent => + rimrafWindowsDir(resolve(path, ent.name), opt, ent, s), + ), + ) + ).reduce((a, b) => a && b, true) if (state === START) { - try { - return await rimrafWindowsDir(path, opt, ent, FINISH) - } catch (e) { - console.trace(e) - throw e - } + return rimrafWindowsDir(path, opt, ent, FINISH) } else if (state === FINISH) { if (opt.preserveRoot === false && path === parse(path).root) { return false @@ -163,12 +149,7 @@ const rimrafWindowsDir = async ( if (opt.filter && !(await opt.filter(path, ent))) { return false } - try { - await ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) - } catch (e) { - console.trace(e) - throw e - } + await ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) } return true } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 9c75c304..8543cab0 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,37 +1,49 @@ import t from 'tap' -import { mkdirSync, readdirSync } from 'fs' +import { mkdirSync, readdirSync, writeFileSync } from 'fs' import { sep, join } from 'path' import { globSync } from 'glob' import { rimraf } from '../../src/index.js' +import { randomBytes } from 'crypto' -const rando = (arr: T[]): T[] => [...arr].sort(() => 0.5 - Math.random()) -const sortAlpha = (arr: string[]) => +const sort = (arr: string[]) => [...arr].sort((a, b) => a.localeCompare(b, 'en')) +const letters = (d: number) => + new Array(d).fill(0).map((_, i) => (10 + i).toString(36)) // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('does not throw EPERM', async t => { const iterations = 200 - const depth = 7 + const dirDepth = 7 + const fileCount = 10 + const fileSizeMb = 0.1 + const cwd = t.testdir() - const nested = join( - ...new Array(depth).fill(0).map((_, i) => (10 + i).toString(36)), - ) - const expected = nested + const nested = join(...letters(dirDepth)) + const files = letters(fileCount).map(f => `file_${f}`) + const dirs = nested .split(sep) .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) + const expected = sort(dirs.flatMap(d => [d, ...files.map(f => join(d, f))])) let count = 0 while (count !== iterations) { mkdirSync(join(cwd, nested), { recursive: true }) - const del = rando(globSync('**/*', { cwd })) - const res = await Promise.all( - del.map(d => rimraf(join(cwd, d), { glob: false })), + for (const dir of dirs) { + for (const f of files) { + writeFileSync(join(cwd, dir, f), randomBytes(1024 * 1024 * fileSizeMb)) + } + } + + const del = globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()) + t.strictSame(sort(del), expected) + t.strictSame( + await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))), + new Array(del.length).fill(true), ) - t.ok(res.every(Boolean)) + t.strictSame(readdirSync(cwd), []) - t.strictSame(sortAlpha(del), expected) count += 1 } t.equal(count, iterations) From ee57e13d410a70f74947eaa2654d0bd191a01948 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 11:00:30 -0700 Subject: [PATCH 31/50] only one ci many times --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20e74d3a..379f4995 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,12 +1,12 @@ name: CI -on: [push, pull_request] +on: [pull_request] jobs: build: strategy: matrix: - node-version: [20.x, 22.x] + node-version: [22.x] platform: # - os: ubuntu-latest # shell: bash From fd7327d88979d45af8b7f4005aeea65db612c9ac Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 11:16:31 -0700 Subject: [PATCH 32/50] use fs.promises #134 --- src/fs.ts | 174 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 103 insertions(+), 71 deletions(-) diff --git a/src/fs.ts b/src/fs.ts index 12ff2077..3787a1bb 100644 --- a/src/fs.ts +++ b/src/fs.ts @@ -1,8 +1,99 @@ -// promisify ourselves, because older nodes don't have fs.promises +// // promisify ourselves, because older nodes don't have fs.promises + +// import fs, { Dirent } from 'fs' + +// // sync ones just take the sync version from node +// export { +// chmodSync, +// mkdirSync, +// renameSync, +// rmdirSync, +// rmSync, +// statSync, +// lstatSync, +// unlinkSync, +// } from 'fs' + +// import { readdirSync as rdSync } from 'fs' +// export const readdirSync = (path: fs.PathLike): Dirent[] => +// rdSync(path, { withFileTypes: true }) + +// // unrolled for better inlining, this seems to get better performance +// // than something like: +// // const makeCb = (res, rej) => (er, ...d) => er ? rej(er) : res(...d) +// // which would be a bit cleaner. + +// const chmod = (path: fs.PathLike, mode: fs.Mode): Promise => +// new Promise((res, rej) => +// fs.chmod(path, mode, (er, ...d: any[]) => (er ? rej(er) : res(...d))), +// ) + +// const mkdir = ( +// path: fs.PathLike, +// options?: +// | fs.Mode +// | (fs.MakeDirectoryOptions & { recursive?: boolean | null }) +// | undefined +// | null, +// ): Promise => +// new Promise((res, rej) => +// fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))), +// ) + +// const readdir = (path: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.readdir(path, { withFileTypes: true }, (er, data) => +// er ? rej(er) : res(data), +// ), +// ) + +// const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.rename(oldPath, newPath, (er, ...d: any[]) => +// er ? rej(er) : res(...d), +// ), +// ) + +// const rm = (path: fs.PathLike, options: fs.RmOptions): Promise => +// new Promise((res, rej) => +// fs.rm(path, options, (er, ...d: any[]) => (er ? rej(er) : res(...d))), +// ) + +// const rmdir = (path: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.rmdir(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), +// ) + +// const stat = (path: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.stat(path, (er, data) => (er ? rej(er) : res(data))), +// ) + +// const lstat = (path: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.lstat(path, (er, data) => (er ? rej(er) : res(data))), +// ) + +// const unlink = (path: fs.PathLike): Promise => +// new Promise((res, rej) => +// fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), +// ) + +// export const promises = { +// chmod, +// mkdir, +// readdir, +// rename, +// rm, +// rmdir, +// stat, +// lstat, +// unlink, +// } import fs, { Dirent } from 'fs' +import fsPromises from 'fs/promises' -// sync ones just take the sync version from node export { chmodSync, mkdirSync, @@ -14,79 +105,20 @@ export { unlinkSync, } from 'fs' -import { readdirSync as rdSync } from 'fs' export const readdirSync = (path: fs.PathLike): Dirent[] => - rdSync(path, { withFileTypes: true }) - -// unrolled for better inlining, this seems to get better performance -// than something like: -// const makeCb = (res, rej) => (er, ...d) => er ? rej(er) : res(...d) -// which would be a bit cleaner. - -const chmod = (path: fs.PathLike, mode: fs.Mode): Promise => - new Promise((res, rej) => - fs.chmod(path, mode, (er, ...d: any[]) => (er ? rej(er) : res(...d))), - ) - -const mkdir = ( - path: fs.PathLike, - options?: - | fs.Mode - | (fs.MakeDirectoryOptions & { recursive?: boolean | null }) - | undefined - | null, -): Promise => - new Promise((res, rej) => - fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))), - ) + fs.readdirSync(path, { withFileTypes: true }) const readdir = (path: fs.PathLike): Promise => - new Promise((res, rej) => - fs.readdir(path, { withFileTypes: true }, (er, data) => - er ? rej(er) : res(data), - ), - ) - -const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise => - new Promise((res, rej) => - fs.rename(oldPath, newPath, (er, ...d: any[]) => - er ? rej(er) : res(...d), - ), - ) - -const rm = (path: fs.PathLike, options: fs.RmOptions): Promise => - new Promise((res, rej) => - fs.rm(path, options, (er, ...d: any[]) => (er ? rej(er) : res(...d))), - ) - -const rmdir = (path: fs.PathLike): Promise => - new Promise((res, rej) => - fs.rmdir(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), - ) - -const stat = (path: fs.PathLike): Promise => - new Promise((res, rej) => - fs.stat(path, (er, data) => (er ? rej(er) : res(data))), - ) - -const lstat = (path: fs.PathLike): Promise => - new Promise((res, rej) => - fs.lstat(path, (er, data) => (er ? rej(er) : res(data))), - ) - -const unlink = (path: fs.PathLike): Promise => - new Promise((res, rej) => - fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), - ) + fsPromises.readdir(path, { withFileTypes: true }) export const promises = { - chmod, - mkdir, + chmod: fsPromises.chmod, + mkdir: fsPromises.mkdir, readdir, - rename, - rm, - rmdir, - stat, - lstat, - unlink, + rename: fsPromises.rename, + rm: fsPromises.rm, + rmdir: fsPromises.rmdir, + stat: fsPromises.stat, + lstat: fsPromises.lstat, + unlink: fsPromises.unlink, } From 39a050982a6ac058584bdad0d40c12ffa6ad5782 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 11:21:20 -0700 Subject: [PATCH 33/50] trace entries --- src/rimraf-windows.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index f27cc57e..b8f979ad 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -112,6 +112,7 @@ const rimrafWindowsDir = async ( // swapped out with a file at just the right moment. /* c8 ignore start */ if (entries) { + console.trace(entries) if (entries.code === 'ENOENT') { return true } From 7465ac43c53c505dfcebef428eb70afea9a6cfbf Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 16:30:41 -0700 Subject: [PATCH 34/50] fallback to move remove on eperm when getting dir entries --- src/rimraf-windows.ts | 5 ++++- test/integration/eperm.ts | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index b8f979ad..94491e61 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -112,10 +112,13 @@ const rimrafWindowsDir = async ( // swapped out with a file at just the right moment. /* c8 ignore start */ if (entries) { - console.trace(entries) if (entries.code === 'ENOENT') { return true } + if (entries.code === 'EPERM') { + console.trace(entries) + return ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + } if (entries.code !== 'ENOTDIR') { throw entries } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 8543cab0..f98ffee1 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -2,7 +2,7 @@ import t from 'tap' import { mkdirSync, readdirSync, writeFileSync } from 'fs' import { sep, join } from 'path' import { globSync } from 'glob' -import { rimraf } from '../../src/index.js' +import { windows } from '../../src/index.js' import { randomBytes } from 'crypto' const sort = (arr: string[]) => @@ -13,7 +13,7 @@ const letters = (d: number) => // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('does not throw EPERM', async t => { +t.test('windows does not throw EPERM', async t => { const iterations = 200 const dirDepth = 7 const fileCount = 10 @@ -39,7 +39,7 @@ t.test('does not throw EPERM', async t => { const del = globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()) t.strictSame(sort(del), expected) t.strictSame( - await Promise.all(del.map(d => rimraf(join(cwd, d), { glob: false }))), + await Promise.all(del.map(d => windows(join(cwd, d), { glob: false }))), new Array(del.length).fill(true), ) From 405ac9809e6682be55e76292c840ee19ad4cab20 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 16:36:16 -0700 Subject: [PATCH 35/50] fallback to move remove on eperm in fallback --- src/rimraf-windows.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 94491e61..5a7da818 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -39,7 +39,8 @@ const rimrafWindowsDirMoveRemoveFallback = async ( return await rimrafWindowsDirRetry(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOTEMPTY') { + if (code === 'ENOTEMPTY' || code === 'EPERM') { + console.trace(er) return await rimrafMoveRemove(path, options) } throw er From ec01c6a80980a81e08e972f782bfde01faa56e1f Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 16:41:46 -0700 Subject: [PATCH 36/50] moar --- test/integration/eperm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index f98ffee1..9a60ac39 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -14,7 +14,7 @@ const letters = (d: number) => // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('windows does not throw EPERM', async t => { - const iterations = 200 + const iterations = 1000 const dirDepth = 7 const fileCount = 10 const fileSizeMb = 0.1 From 52e99cbfadfa7ba1ad0398c22445699c1d154295 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 19:31:17 -0700 Subject: [PATCH 37/50] timeout --- test/integration/eperm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 9a60ac39..b1e89108 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -13,7 +13,7 @@ const letters = (d: number) => // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('windows does not throw EPERM', async t => { +t.test('windows does not throw EPERM', { timeout: 1000 * 60 * 2 }, async t => { const iterations = 1000 const dirDepth = 7 const fileCount = 10 From e459db4ab931aa63a58276b45dfcfde751377be6 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 5 Oct 2024 19:41:27 -0700 Subject: [PATCH 38/50] timeout again --- package.json | 3 +++ test/integration/eperm.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 68b06e17..358c9444 100644 --- a/package.json +++ b/package.json @@ -85,5 +85,8 @@ "rmdir", "recursive" ], + "tap": { + "timeout": 600 + }, "module": "./dist/esm/index.js" } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index b1e89108..9a60ac39 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -13,7 +13,7 @@ const letters = (d: number) => // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('windows does not throw EPERM', { timeout: 1000 * 60 * 2 }, async t => { +t.test('windows does not throw EPERM', async t => { const iterations = 1000 const dirDepth = 7 const fileCount = 10 From 40998388c96ba41168d94aaac874ef0e5bfb42d4 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 11:20:54 -0700 Subject: [PATCH 39/50] refactor eperm integration test makes it show less TAP output and be configurable based on ci/local --- test/integration/eperm.ts | 137 +++++++++++++++++++++++++++++--------- 1 file changed, 105 insertions(+), 32 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 9a60ac39..e431db48 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -1,50 +1,123 @@ -import t from 'tap' +import t, { Test } from 'tap' import { mkdirSync, readdirSync, writeFileSync } from 'fs' import { sep, join } from 'path' import { globSync } from 'glob' import { windows } from '../../src/index.js' import { randomBytes } from 'crypto' +import assert from 'assert' -const sort = (arr: string[]) => - [...arr].sort((a, b) => a.localeCompare(b, 'en')) -const letters = (d: number) => - new Array(d).fill(0).map((_, i) => (10 + i).toString(36)) - -// Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 -// that this test would throw EPERM errors consistently in Windows CI environments. -// https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('windows does not throw EPERM', async t => { - const iterations = 1000 - const dirDepth = 7 - const fileCount = 10 - const fileSizeMb = 0.1 +const arrSame = (arr1: string[], arr2: string[]) => { + const s = (a: string[]) => [...a].sort().join(',') + return s(arr1) === s(arr2) +} +const setup = ( + t: Test, + { + iterations, + depth, + files: fileCount, + fileKb, + }: { + iterations: number + depth: number + files: number + fileKb: number + }, +) => { + let count = 0 const cwd = t.testdir() - const nested = join(...letters(dirDepth)) + + const letters = (length: number) => + Array.from({ length }).map((_, i) => (10 + i).toString(36)) + + const deepestDir = join(...letters(depth)) const files = letters(fileCount).map(f => `file_${f}`) - const dirs = nested + + const dirs = deepestDir .split(sep) .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) - const expected = sort(dirs.flatMap(d => [d, ...files.map(f => join(d, f))])) - let count = 0 - while (count !== iterations) { - mkdirSync(join(cwd, nested), { recursive: true }) - for (const dir of dirs) { - for (const f of files) { - writeFileSync(join(cwd, dir, f), randomBytes(1024 * 1024 * fileSizeMb)) + const expected = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) + + return { + next: () => { + if (count === iterations) { + return false } - } + count += 1 + return true + }, + cwd, + expected, + writeFixtures: () => { + mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) + for (const dir of dirs) { + for (const f of files) { + writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) + } + } + // randomize results from glob so that when running Promise.all(rimraf) + // on the result it will potentially delete parent directories before + // child directories and their files. This seems to make EPERM errors + // more likely on Windows. + return globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()) + }, + } +} + +// Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 +// that this test would throw EPERM errors consistently in Windows CI environments. +// https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 +t.test('windows does not throw EPERM', async t => { + const { next, cwd, expected, writeFixtures } = setup( + t, + process.env.CI ? + { + iterations: 1000, + depth: 10, + files: 10, + fileKb: 10, + } + : { + iterations: 200, + depth: 7, + files: 1, + fileKb: 0, + }, + ) + + while (next()) { + const toDelete = writeFixtures() + + // throw instead of using tap assertions to cut down on output + // when running many iterations + assert( + arrSame(toDelete, expected), + new Error(`glob result is not expected`, { + cause: { + found: toDelete, + wanted: expected, + }, + }), + ) - const del = globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()) - t.strictSame(sort(del), expected) - t.strictSame( - await Promise.all(del.map(d => windows(join(cwd, d), { glob: false }))), - new Array(del.length).fill(true), + const notDeleted = ( + await Promise.all( + toDelete.map(d => + windows(join(cwd, d), { glob: false }).then(r => [d, r] as const), + ), + ) + ).filter(([, v]) => v !== true) + assert( + !notDeleted.length, + new Error(`some entries were not deleted`, { + cause: { + found: notDeleted, + }, + }), ) - t.strictSame(readdirSync(cwd), []) - count += 1 + assert(!readdirSync(cwd).length, new Error(`dir is not empty`)) } - t.equal(count, iterations) }) From 5b5370ad22b69bf9740f6ec36704732f8c22c6ca Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 12:30:39 -0700 Subject: [PATCH 40/50] better errors in failing iterations --- test/integration/eperm.ts | 117 +++++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 53 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index e431db48..8e325aef 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -25,7 +25,6 @@ const setup = ( fileKb: number }, ) => { - let count = 0 const cwd = t.testdir() const letters = (length: number) => @@ -40,84 +39,96 @@ const setup = ( const expected = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) - return { - next: () => { - if (count === iterations) { - return false - } - count += 1 - return true - }, - cwd, - expected, - writeFixtures: () => { - mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) - for (const dir of dirs) { - for (const f of files) { - writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) + t.plan(2) + let count = 0 + + return [ + { cwd, expected }, + function* () { + while (count !== iterations) { + count += 1 + + mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) + for (const dir of dirs) { + for (const f of files) { + writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) + } + } + + // use custom error to throw instead of using tap assertions to cut down on output + // when running many iterations + class RunError extends Error { + constructor(message: string, c?: Error | Record) { + super(message, { + cause: { + count, + ...(c instanceof Error ? { error: c } : c), + }, + }) + } } + + yield [ + // randomize results from glob so that when running Promise.all(rimraf) + // on the result it will potentially delete parent directories before + // child directories and their files. This seems to make EPERM errors + // more likely on Windows. + globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()), + RunError, + ] as const } - // randomize results from glob so that when running Promise.all(rimraf) - // on the result it will potentially delete parent directories before - // child directories and their files. This seems to make EPERM errors - // more likely on Windows. - return globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()) + + t.equal(count, iterations, 'ran all iterations') + t.strictSame(globSync('**/*', { cwd }), [], 'no more files') }, - } + ] as const } // Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('windows does not throw EPERM', async t => { - const { next, cwd, expected, writeFixtures } = setup( + const [{ cwd, expected }, run] = setup( t, process.env.CI ? { iterations: 1000, - depth: 10, - files: 10, - fileKb: 10, + depth: 15, + files: 7, + fileKb: 100, } : { iterations: 200, - depth: 7, - files: 1, - fileKb: 0, + depth: 8, + files: 3, + fileKb: 10, }, ) - while (next()) { - const toDelete = writeFixtures() - - // throw instead of using tap assertions to cut down on output - // when running many iterations + for (const [matches, RunError] of run()) { assert( - arrSame(toDelete, expected), - new Error(`glob result is not expected`, { - cause: { - found: toDelete, - wanted: expected, - }, + arrSame(matches, expected), + new RunError(`glob result is not expected`, { + found: matches, + wanted: expected, }), ) - const notDeleted = ( - await Promise.all( - toDelete.map(d => - windows(join(cwd, d), { glob: false }).then(r => [d, r] as const), - ), - ) - ).filter(([, v]) => v !== true) + const result = await Promise.all( + matches.map(d => + windows(join(cwd, d), { glob: false }).then(r => [d, r] as const), + ), + ).catch(e => { + throw new RunError(`rimraf.windows error`, e) + }) + assert( - !notDeleted.length, - new Error(`some entries were not deleted`, { - cause: { - found: notDeleted, - }, + result.every(([, v]) => v === true), + new RunError(`some entries were not deleted`, { + found: result, }), ) - assert(!readdirSync(cwd).length, new Error(`dir is not empty`)) + assert(!readdirSync(cwd).length, new RunError(`dir is not empty`)) } }) From 97dd8b1e0c2cf0868fb916d576bb58f42c78161a Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 12:37:48 -0700 Subject: [PATCH 41/50] only log leftover entries to error cause --- test/integration/eperm.ts | 77 +++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 8e325aef..b05655a8 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -25,35 +25,23 @@ const setup = ( fileKb: number }, ) => { - const cwd = t.testdir() - const letters = (length: number) => Array.from({ length }).map((_, i) => (10 + i).toString(36)) - - const deepestDir = join(...letters(depth)) const files = letters(fileCount).map(f => `file_${f}`) - - const dirs = deepestDir + const dirs = join(...letters(depth)) .split(sep) .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) - - const expected = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) + const entries = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) t.plan(2) - let count = 0 + const cwd = t.testdir() + let iteration = 0 return [ - { cwd, expected }, + cwd, function* () { - while (count !== iterations) { - count += 1 - - mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) - for (const dir of dirs) { - for (const f of files) { - writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) - } - } + while (iteration !== iterations) { + iteration += 1 // use custom error to throw instead of using tap assertions to cut down on output // when running many iterations @@ -61,24 +49,40 @@ const setup = ( constructor(message: string, c?: Error | Record) { super(message, { cause: { - count, + iteration, ...(c instanceof Error ? { error: c } : c), }, }) } } - yield [ - // randomize results from glob so that when running Promise.all(rimraf) - // on the result it will potentially delete parent directories before - // child directories and their files. This seems to make EPERM errors - // more likely on Windows. - globSync('**/*', { cwd }).sort(() => 0.5 - Math.random()), - RunError, - ] as const + mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) + for (const dir of dirs) { + for (const f of files) { + writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) + } + } + + // randomize results from glob so that when running Promise.all(rimraf) + // on the result it will potentially delete parent directories before + // child directories and their files. This seems to make EPERM errors + // more likely on Windows. + const matches = globSync('**/*', { cwd }).sort( + () => 0.5 - Math.random(), + ) + + assert( + arrSame(matches, entries), + new RunError(`glob result does not match expected`, { + found: matches, + wanted: entries, + }), + ) + + yield [matches, RunError] as const } - t.equal(count, iterations, 'ran all iterations') + t.equal(iteration, iterations, 'ran all iterations') t.strictSame(globSync('**/*', { cwd }), [], 'no more files') }, ] as const @@ -88,7 +92,7 @@ const setup = ( // that this test would throw EPERM errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 t.test('windows does not throw EPERM', async t => { - const [{ cwd, expected }, run] = setup( + const [cwd, run] = setup( t, process.env.CI ? { @@ -106,14 +110,6 @@ t.test('windows does not throw EPERM', async t => { ) for (const [matches, RunError] of run()) { - assert( - arrSame(matches, expected), - new RunError(`glob result is not expected`, { - found: matches, - wanted: expected, - }), - ) - const result = await Promise.all( matches.map(d => windows(join(cwd, d), { glob: false }).then(r => [d, r] as const), @@ -122,10 +118,11 @@ t.test('windows does not throw EPERM', async t => { throw new RunError(`rimraf.windows error`, e) }) + const notDeleted = result.filter(([, v]) => v !== true) assert( - result.every(([, v]) => v === true), + !notDeleted.length, new RunError(`some entries were not deleted`, { - found: result, + found: notDeleted, }), ) From 84c4e451d9522fbf2fe120ba1812c7775793998d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 12:45:21 -0700 Subject: [PATCH 42/50] individual rimraf.windows errors --- test/integration/eperm.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index b05655a8..06e72bdc 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -46,11 +46,11 @@ const setup = ( // use custom error to throw instead of using tap assertions to cut down on output // when running many iterations class RunError extends Error { - constructor(message: string, c?: Error | Record) { + constructor(message: string, c?: Record) { super(message, { cause: { iteration, - ...(c instanceof Error ? { error: c } : c), + ...c, }, }) } @@ -79,7 +79,7 @@ const setup = ( }), ) - yield [matches, RunError] as const + yield [matches.map(m => join(cwd, m)), RunError] as const } t.equal(iteration, iterations, 'ran all iterations') @@ -111,14 +111,16 @@ t.test('windows does not throw EPERM', async t => { for (const [matches, RunError] of run()) { const result = await Promise.all( - matches.map(d => - windows(join(cwd, d), { glob: false }).then(r => [d, r] as const), + matches.map(path => + windows(path) + .then(deleted => ({ path, deleted })) + .catch(error => { + throw new RunError(`rimraf.windows error`, { error, path }) + }), ), - ).catch(e => { - throw new RunError(`rimraf.windows error`, e) - }) + ) - const notDeleted = result.filter(([, v]) => v !== true) + const notDeleted = result.filter(({ deleted }) => deleted !== true) assert( !notDeleted.length, new RunError(`some entries were not deleted`, { From 19dd96a1bf8433e4cea8b3561a5318550642ff3e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 13:09:46 -0700 Subject: [PATCH 43/50] await eperm and return true --- src/rimraf-windows.ts | 13 +++++------ test/integration/eperm.ts | 45 ++++++++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 5a7da818..896c9b40 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -40,7 +40,6 @@ const rimrafWindowsDirMoveRemoveFallback = async ( } catch (er) { const code = (er as NodeJS.ErrnoException)?.code if (code === 'ENOTEMPTY' || code === 'EPERM') { - console.trace(er) return await rimrafMoveRemove(path, options) } throw er @@ -60,7 +59,7 @@ const rimrafWindowsDirMoveRemoveFallbackSync = ( return rimrafWindowsDirRetrySync(path, options) } catch (er) { const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOTEMPTY') { + if (code === 'ENOTEMPTY' || code === 'EPERM') { return rimrafMoveRemoveSync(path, options) } throw er @@ -78,8 +77,7 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { try { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { - const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOTEMPTY') return true + if ((er as NodeJS.ErrnoException)?.code === 'ENOTEMPTY') return true throw er } } @@ -91,8 +89,7 @@ export const rimrafWindowsSync = (path: string, opt: RimrafSyncOptions) => { try { return rimrafWindowsDirSync(path, opt, lstatSync(path), START) } catch (er) { - const code = (er as NodeJS.ErrnoException)?.code - if (code === 'ENOENT') return true + if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true throw er } } @@ -117,8 +114,8 @@ const rimrafWindowsDir = async ( return true } if (entries.code === 'EPERM') { - console.trace(entries) - return ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + await ignoreENOENT(rimrafWindowsDirMoveRemoveFallback(path, opt)) + return true } if (entries.code !== 'ENOTDIR') { throw entries diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 06e72bdc..25c83c2f 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -33,7 +33,6 @@ const setup = ( .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) const entries = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) - t.plan(2) const cwd = t.testdir() let iteration = 0 @@ -82,8 +81,7 @@ const setup = ( yield [matches.map(m => join(cwd, m)), RunError] as const } - t.equal(iteration, iterations, 'ran all iterations') - t.strictSame(globSync('**/*', { cwd }), [], 'no more files') + return [iteration, iterations] as const }, ] as const } @@ -109,18 +107,27 @@ t.test('windows does not throw EPERM', async t => { }, ) - for (const [matches, RunError] of run()) { - const result = await Promise.all( - matches.map(path => - windows(path) - .then(deleted => ({ path, deleted })) - .catch(error => { - throw new RunError(`rimraf.windows error`, { error, path }) - }), - ), - ) + let i + const r = run() + while ((i = r.next())) { + if (i.done) { + i = i.value + break + } - const notDeleted = result.filter(({ deleted }) => deleted !== true) + const [matches, RunError] = i.value + + const notDeleted = ( + await Promise.all( + matches.map(path => + windows(path) + .then(deleted => ({ path, deleted })) + .catch(error => { + throw new RunError(`rimraf.windows error`, { error, path }) + }), + ), + ) + ).filter(({ deleted }) => deleted !== true) assert( !notDeleted.length, new RunError(`some entries were not deleted`, { @@ -128,6 +135,14 @@ t.test('windows does not throw EPERM', async t => { }), ) - assert(!readdirSync(cwd).length, new RunError(`dir is not empty`)) + const actual = readdirSync(cwd) + assert( + !actual.length, + new RunError(`dir is not empty`, { + actual, + }), + ) } + + t.equal(i[0], i[1], `ran all ${i[1]} iterations`) }) From 84793c203a6cdb05400f468f5bd8bc62c6ba5fab Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 13:32:54 -0700 Subject: [PATCH 44/50] try without fs.promises --- src/fs.ts | 174 ++++++++++++++++++++++-------------------------------- 1 file changed, 71 insertions(+), 103 deletions(-) diff --git a/src/fs.ts b/src/fs.ts index 3787a1bb..12ff2077 100644 --- a/src/fs.ts +++ b/src/fs.ts @@ -1,99 +1,8 @@ -// // promisify ourselves, because older nodes don't have fs.promises - -// import fs, { Dirent } from 'fs' - -// // sync ones just take the sync version from node -// export { -// chmodSync, -// mkdirSync, -// renameSync, -// rmdirSync, -// rmSync, -// statSync, -// lstatSync, -// unlinkSync, -// } from 'fs' - -// import { readdirSync as rdSync } from 'fs' -// export const readdirSync = (path: fs.PathLike): Dirent[] => -// rdSync(path, { withFileTypes: true }) - -// // unrolled for better inlining, this seems to get better performance -// // than something like: -// // const makeCb = (res, rej) => (er, ...d) => er ? rej(er) : res(...d) -// // which would be a bit cleaner. - -// const chmod = (path: fs.PathLike, mode: fs.Mode): Promise => -// new Promise((res, rej) => -// fs.chmod(path, mode, (er, ...d: any[]) => (er ? rej(er) : res(...d))), -// ) - -// const mkdir = ( -// path: fs.PathLike, -// options?: -// | fs.Mode -// | (fs.MakeDirectoryOptions & { recursive?: boolean | null }) -// | undefined -// | null, -// ): Promise => -// new Promise((res, rej) => -// fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))), -// ) - -// const readdir = (path: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.readdir(path, { withFileTypes: true }, (er, data) => -// er ? rej(er) : res(data), -// ), -// ) - -// const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.rename(oldPath, newPath, (er, ...d: any[]) => -// er ? rej(er) : res(...d), -// ), -// ) - -// const rm = (path: fs.PathLike, options: fs.RmOptions): Promise => -// new Promise((res, rej) => -// fs.rm(path, options, (er, ...d: any[]) => (er ? rej(er) : res(...d))), -// ) - -// const rmdir = (path: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.rmdir(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), -// ) - -// const stat = (path: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.stat(path, (er, data) => (er ? rej(er) : res(data))), -// ) - -// const lstat = (path: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.lstat(path, (er, data) => (er ? rej(er) : res(data))), -// ) - -// const unlink = (path: fs.PathLike): Promise => -// new Promise((res, rej) => -// fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), -// ) - -// export const promises = { -// chmod, -// mkdir, -// readdir, -// rename, -// rm, -// rmdir, -// stat, -// lstat, -// unlink, -// } +// promisify ourselves, because older nodes don't have fs.promises import fs, { Dirent } from 'fs' -import fsPromises from 'fs/promises' +// sync ones just take the sync version from node export { chmodSync, mkdirSync, @@ -105,20 +14,79 @@ export { unlinkSync, } from 'fs' +import { readdirSync as rdSync } from 'fs' export const readdirSync = (path: fs.PathLike): Dirent[] => - fs.readdirSync(path, { withFileTypes: true }) + rdSync(path, { withFileTypes: true }) + +// unrolled for better inlining, this seems to get better performance +// than something like: +// const makeCb = (res, rej) => (er, ...d) => er ? rej(er) : res(...d) +// which would be a bit cleaner. + +const chmod = (path: fs.PathLike, mode: fs.Mode): Promise => + new Promise((res, rej) => + fs.chmod(path, mode, (er, ...d: any[]) => (er ? rej(er) : res(...d))), + ) + +const mkdir = ( + path: fs.PathLike, + options?: + | fs.Mode + | (fs.MakeDirectoryOptions & { recursive?: boolean | null }) + | undefined + | null, +): Promise => + new Promise((res, rej) => + fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))), + ) const readdir = (path: fs.PathLike): Promise => - fsPromises.readdir(path, { withFileTypes: true }) + new Promise((res, rej) => + fs.readdir(path, { withFileTypes: true }, (er, data) => + er ? rej(er) : res(data), + ), + ) + +const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise => + new Promise((res, rej) => + fs.rename(oldPath, newPath, (er, ...d: any[]) => + er ? rej(er) : res(...d), + ), + ) + +const rm = (path: fs.PathLike, options: fs.RmOptions): Promise => + new Promise((res, rej) => + fs.rm(path, options, (er, ...d: any[]) => (er ? rej(er) : res(...d))), + ) + +const rmdir = (path: fs.PathLike): Promise => + new Promise((res, rej) => + fs.rmdir(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), + ) + +const stat = (path: fs.PathLike): Promise => + new Promise((res, rej) => + fs.stat(path, (er, data) => (er ? rej(er) : res(data))), + ) + +const lstat = (path: fs.PathLike): Promise => + new Promise((res, rej) => + fs.lstat(path, (er, data) => (er ? rej(er) : res(data))), + ) + +const unlink = (path: fs.PathLike): Promise => + new Promise((res, rej) => + fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))), + ) export const promises = { - chmod: fsPromises.chmod, - mkdir: fsPromises.mkdir, + chmod, + mkdir, readdir, - rename: fsPromises.rename, - rm: fsPromises.rm, - rmdir: fsPromises.rmdir, - stat: fsPromises.stat, - lstat: fsPromises.lstat, - unlink: fsPromises.unlink, + rename, + rm, + rmdir, + stat, + lstat, + unlink, } From b4acd32131a4df7acda343843fe4771d577d94df Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 14:30:14 -0700 Subject: [PATCH 45/50] add same behavior for windows sync --- src/fs.ts | 32 ++++++++ src/rimraf-windows.ts | 8 +- test/integration/eperm.ts | 164 ++++++++++++++++++++++++++------------ 3 files changed, 150 insertions(+), 54 deletions(-) diff --git a/src/fs.ts b/src/fs.ts index 12ff2077..84f32844 100644 --- a/src/fs.ts +++ b/src/fs.ts @@ -90,3 +90,35 @@ export const promises = { lstat, unlink, } + +// import fs, { Dirent } from 'fs' +// import fsPromises from 'fs/promises' + +// export { +// chmodSync, +// mkdirSync, +// renameSync, +// rmdirSync, +// rmSync, +// statSync, +// lstatSync, +// unlinkSync, +// } from 'fs' + +// export const readdirSync = (path: fs.PathLike): Dirent[] => +// fs.readdirSync(path, { withFileTypes: true }) + +// const readdir = (path: fs.PathLike): Promise => +// fsPromises.readdir(path, { withFileTypes: true }) + +// export const promises = { +// chmod: fsPromises.chmod, +// mkdir: fsPromises.mkdir, +// readdir, +// rename: fsPromises.rename, +// rm: fsPromises.rm, +// rmdir: fsPromises.rmdir, +// stat: fsPromises.stat, +// lstat: fsPromises.lstat, +// unlink: fsPromises.unlink, +// } diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 896c9b40..53396e88 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -77,7 +77,7 @@ export const rimrafWindows = async (path: string, opt: RimrafAsyncOptions) => { try { return await rimrafWindowsDir(path, opt, await lstat(path), START) } catch (er) { - if ((er as NodeJS.ErrnoException)?.code === 'ENOTEMPTY') return true + if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true throw er } } @@ -171,6 +171,12 @@ const rimrafWindowsDirSync = ( if (entries.code === 'ENOENT') { return true } + if (entries.code === 'EPERM') { + ignoreENOENTSync(() => { + rimrafWindowsDirMoveRemoveFallbackSync(path, opt) + }) + return true + } if (entries.code !== 'ENOTDIR') { throw entries } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 25c83c2f..2e36bf3f 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -2,7 +2,7 @@ import t, { Test } from 'tap' import { mkdirSync, readdirSync, writeFileSync } from 'fs' import { sep, join } from 'path' import { globSync } from 'glob' -import { windows } from '../../src/index.js' +import { windows, windowsSync } from '../../src/index.js' import { randomBytes } from 'crypto' import assert from 'assert' @@ -25,29 +25,30 @@ const setup = ( fileKb: number }, ) => { + const cwd = t.testdir() + const letters = (length: number) => Array.from({ length }).map((_, i) => (10 + i).toString(36)) const files = letters(fileCount).map(f => `file_${f}`) const dirs = join(...letters(depth)) .split(sep) .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) - const entries = dirs.flatMap(d => [d, ...files.map(f => join(d, f))]) + const entries = dirs + .flatMap(d => [d, ...files.map(f => join(d, f))]) + .map(d => join(cwd, d)) - const cwd = t.testdir() let iteration = 0 - return [ cwd, function* () { while (iteration !== iterations) { - iteration += 1 - - // use custom error to throw instead of using tap assertions to cut down on output - // when running many iterations + // use custom error to throw instead of using tap assertions to cut down + // on output when running many iterations class RunError extends Error { constructor(message: string, c?: Record) { super(message, { cause: { + testName: t.name, iteration, ...c, }, @@ -55,6 +56,14 @@ const setup = ( } } + const actual = readdirSync(cwd) + assert( + !actual.length, + new RunError(`dir is not empty`, { + found: actual, + }), + ) + mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) for (const dir of dirs) { for (const f of files) { @@ -66,9 +75,9 @@ const setup = ( // on the result it will potentially delete parent directories before // child directories and their files. This seems to make EPERM errors // more likely on Windows. - const matches = globSync('**/*', { cwd }).sort( - () => 0.5 - Math.random(), - ) + const matches = globSync('**/*', { cwd }) + .sort(() => 0.5 - Math.random()) + .map(m => join(cwd, m)) assert( arrSame(matches, entries), @@ -78,7 +87,9 @@ const setup = ( }), ) - yield [matches.map(m => join(cwd, m)), RunError] as const + iteration += 1 + + yield [matches, RunError] as const } return [iteration, iterations] as const @@ -86,12 +97,12 @@ const setup = ( ] as const } -// Copied from sindresorhus/del since it was reported in https://github.com/isaacs/rimraf/pull/314 -// that this test would throw EPERM errors consistently in Windows CI environments. +// Copied from sindresorhus/del since it was reported in +// https://github.com/isaacs/rimraf/pull/314 that this test would throw EPERM +// errors consistently in Windows CI environments. // https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 -t.test('windows does not throw EPERM', async t => { - const [cwd, run] = setup( - t, +t.test('windows does not throw EPERM', t => { + const options = process.env.CI ? { iterations: 1000, @@ -104,45 +115,92 @@ t.test('windows does not throw EPERM', async t => { depth: 8, files: 3, fileKb: 10, - }, - ) - - let i - const r = run() - while ((i = r.next())) { - if (i.done) { - i = i.value - break - } + } + + t.test('sync', t => { + const [cwd, run] = setup(t, options) + + let i + const r = run() + while ((i = r.next())) { + if (i.done) { + i = i.value + break + } - const [matches, RunError] = i.value - - const notDeleted = ( - await Promise.all( - matches.map(path => - windows(path) - .then(deleted => ({ path, deleted })) - .catch(error => { - throw new RunError(`rimraf.windows error`, { error, path }) - }), - ), + const [matches, RunError] = i.value + const result = matches + .map(path => { + try { + return { + path, + deleted: windowsSync(path), + } + } catch (error) { + throw new RunError('rimraf error', { error, path }) + } + }) + .filter(({ deleted }) => deleted !== true) + assert( + !result.length, + new RunError(`some entries were not deleted`, { + found: result, + }), ) - ).filter(({ deleted }) => deleted !== true) - assert( - !notDeleted.length, - new RunError(`some entries were not deleted`, { - found: notDeleted, - }), - ) + } - const actual = readdirSync(cwd) - assert( - !actual.length, - new RunError(`dir is not empty`, { - actual, - }), + t.strictSame(readdirSync(cwd), []) + t.equal(i[0], i[1], `ran all ${i[1]} iterations`) + t.end() + }) + + t.test('async', async t => { + const [cwd, run] = setup( + t, + process.env.CI ? + { + iterations: 1000, + depth: 15, + files: 7, + fileKb: 100, + } + : { + iterations: 200, + depth: 8, + files: 3, + fileKb: 10, + }, ) - } - t.equal(i[0], i[1], `ran all ${i[1]} iterations`) + let i + const r = run() + while ((i = r.next())) { + if (i.done) { + i = i.value + break + } + + const [matches, RunError] = i.value + const result = ( + await Promise.all( + matches.map(path => + windows(path) + .then(deleted => ({ path, deleted })) + .catch(error => { + throw new RunError('rimraf error', { error, path }) + }), + ), + ) + ).filter(({ deleted }) => deleted !== true) + assert( + !result.length, + new RunError(`some entries were not deleted`, { + found: result, + }), + ) + } + + t.strictSame(readdirSync(cwd), []) + t.equal(i[0], i[1], `ran all ${i[1]} iterations`) + }) }) From 6ca0829ddc38f8646d30a582db6941fb939db08b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 14:34:12 -0700 Subject: [PATCH 46/50] end test properly --- test/integration/eperm.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index 2e36bf3f..a36318f2 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -203,4 +203,6 @@ t.test('windows does not throw EPERM', t => { t.strictSame(readdirSync(cwd), []) t.equal(i[0], i[1], `ran all ${i[1]} iterations`) }) + + t.end() }) From 03fb0ec967d91eb3f5d4ccd09ae7a04c5674c875 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 14:52:56 -0700 Subject: [PATCH 47/50] trace on file eperm --- .github/workflows/ci.yml | 36 ++++++++++++------------ src/rimraf-windows.ts | 9 +++++- test/integration/eperm.ts | 58 ++++++++++++++++----------------------- 3 files changed, 49 insertions(+), 54 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 379f4995..05cfcf12 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,21 +36,21 @@ jobs: run: npm install - name: Run Tests - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage - - run: npm test -- test/integration/eperm.ts --disable-coverage + run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 53396e88..889798d4 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -126,7 +126,14 @@ const rimrafWindowsDir = async ( return false } // is a file - await ignoreENOENT(rimrafWindowsFile(path, opt)) + try { + await ignoreENOENT(rimrafWindowsFile(path, opt)) + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code === 'EPERM') { + console.trace(err) + } + throw err + } return true } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index a36318f2..f04b8162 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -25,7 +25,7 @@ const setup = ( fileKb: number }, ) => { - const cwd = t.testdir() + const dir = t.testdir() const letters = (length: number) => Array.from({ length }).map((_, i) => (10 + i).toString(36)) @@ -35,11 +35,11 @@ const setup = ( .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) const entries = dirs .flatMap(d => [d, ...files.map(f => join(d, f))]) - .map(d => join(cwd, d)) + .map(d => join(dir, d)) let iteration = 0 return [ - cwd, + dir, function* () { while (iteration !== iterations) { // use custom error to throw instead of using tap assertions to cut down @@ -56,7 +56,7 @@ const setup = ( } } - const actual = readdirSync(cwd) + const actual = readdirSync(dir) assert( !actual.length, new RunError(`dir is not empty`, { @@ -64,10 +64,10 @@ const setup = ( }), ) - mkdirSync(join(cwd, dirs.at(-1)!), { recursive: true }) + mkdirSync(join(dir, dirs.at(-1)!), { recursive: true }) for (const dir of dirs) { for (const f of files) { - writeFileSync(join(cwd, dir, f), randomBytes(1024 * fileKb)) + writeFileSync(join(dir, dir, f), randomBytes(1024 * fileKb)) } } @@ -75,9 +75,9 @@ const setup = ( // on the result it will potentially delete parent directories before // child directories and their files. This seems to make EPERM errors // more likely on Windows. - const matches = globSync('**/*', { cwd }) + const matches = globSync('**/*', { cwd: dir }) .sort(() => 0.5 - Math.random()) - .map(m => join(cwd, m)) + .map(m => join(dir, m)) assert( arrSame(matches, entries), @@ -92,7 +92,7 @@ const setup = ( yield [matches, RunError] as const } - return [iteration, iterations] as const + return [iteration, iterations] }, ] as const } @@ -118,7 +118,7 @@ t.test('windows does not throw EPERM', t => { } t.test('sync', t => { - const [cwd, run] = setup(t, options) + const [dir, run] = setup(t, options) let i const r = run() @@ -149,28 +149,13 @@ t.test('windows does not throw EPERM', t => { ) } - t.strictSame(readdirSync(cwd), []) + t.strictSame(readdirSync(dir), []) t.equal(i[0], i[1], `ran all ${i[1]} iterations`) t.end() }) t.test('async', async t => { - const [cwd, run] = setup( - t, - process.env.CI ? - { - iterations: 1000, - depth: 15, - files: 7, - fileKb: 100, - } - : { - iterations: 200, - depth: 8, - files: 3, - fileKb: 10, - }, - ) + const [dir, run] = setup(t, options) let i const r = run() @@ -183,13 +168,16 @@ t.test('windows does not throw EPERM', t => { const [matches, RunError] = i.value const result = ( await Promise.all( - matches.map(path => - windows(path) - .then(deleted => ({ path, deleted })) - .catch(error => { - throw new RunError('rimraf error', { error, path }) - }), - ), + matches.map(async path => { + try { + return { + path, + deleted: await windows(path), + } + } catch (error) { + throw new RunError('rimraf error', { error, path }) + } + }), ) ).filter(({ deleted }) => deleted !== true) assert( @@ -200,7 +188,7 @@ t.test('windows does not throw EPERM', t => { ) } - t.strictSame(readdirSync(cwd), []) + t.strictSame(readdirSync(dir), []) t.equal(i[0], i[1], `ran all ${i[1]} iterations`) }) From 62709a07a80b6ea22402c64d343578e824f8e4bf Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 14:57:04 -0700 Subject: [PATCH 48/50] fix --- test/integration/eperm.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index f04b8162..c92aeced 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -65,9 +65,9 @@ const setup = ( ) mkdirSync(join(dir, dirs.at(-1)!), { recursive: true }) - for (const dir of dirs) { + for (const d of dirs) { for (const f of files) { - writeFileSync(join(dir, dir, f), randomBytes(1024 * fileKb)) + writeFileSync(join(dir, d, f), randomBytes(1024 * fileKb)) } } From 1c03a98bde0f55249bf6d5855dfb515326cb8383 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 15:08:15 -0700 Subject: [PATCH 49/50] log things inside fix-eperm --- src/fix-eperm.ts | 9 ++- test/integration/eperm.ts | 115 +++++++++++++++++++------------------- 2 files changed, 64 insertions(+), 60 deletions(-) diff --git a/src/fix-eperm.ts b/src/fix-eperm.ts index 5e7d5fed..ed179ed8 100644 --- a/src/fix-eperm.ts +++ b/src/fix-eperm.ts @@ -11,16 +11,23 @@ export const fixEPERM = return } if (fer?.code === 'EPERM') { + console.log('fixEPERM error', er) try { await chmod(path, 0o666) } catch (er2) { + console.log('fixEPERM chmod error', er2) const fer2 = er2 as NodeJS.ErrnoException if (fer2?.code === 'ENOENT') { return } throw er } - return await fn(path) + try { + return await fn(path) + } catch (er3) { + console.log('fixEPERM after chmox', er3) + throw er3 + } } throw er } diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts index c92aeced..419b6909 100644 --- a/test/integration/eperm.ts +++ b/test/integration/eperm.ts @@ -38,63 +38,64 @@ const setup = ( .map(d => join(dir, d)) let iteration = 0 - return [ - dir, - function* () { - while (iteration !== iterations) { - // use custom error to throw instead of using tap assertions to cut down - // on output when running many iterations - class RunError extends Error { - constructor(message: string, c?: Record) { - super(message, { - cause: { - testName: t.name, - iteration, - ...c, - }, - }) - } + return function* () { + while (iteration !== iterations) { + // use custom error to throw instead of using tap assertions to cut down + // on output when running many iterations + class RunError extends Error { + constructor(message: string, c?: Record) { + super(message, { + cause: { + testName: t.name, + iteration, + ...c, + }, + }) } + } - const actual = readdirSync(dir) - assert( - !actual.length, - new RunError(`dir is not empty`, { - found: actual, - }), - ) + const actual = readdirSync(dir) + assert( + !actual.length, + new RunError(`dir is not empty`, { + found: actual, + }), + ) - mkdirSync(join(dir, dirs.at(-1)!), { recursive: true }) - for (const d of dirs) { - for (const f of files) { - writeFileSync(join(dir, d, f), randomBytes(1024 * fileKb)) - } + mkdirSync(join(dir, dirs.at(-1)!), { recursive: true }) + for (const d of dirs) { + for (const f of files) { + writeFileSync(join(dir, d, f), randomBytes(1024 * fileKb)) } + } - // randomize results from glob so that when running Promise.all(rimraf) - // on the result it will potentially delete parent directories before - // child directories and their files. This seems to make EPERM errors - // more likely on Windows. - const matches = globSync('**/*', { cwd: dir }) - .sort(() => 0.5 - Math.random()) - .map(m => join(dir, m)) - - assert( - arrSame(matches, entries), - new RunError(`glob result does not match expected`, { - found: matches, - wanted: entries, - }), - ) + // randomize results from glob so that when running Promise.all(rimraf) + // on the result it will potentially delete parent directories before + // child directories and their files. This seems to make EPERM errors + // more likely on Windows. + const matches = globSync('**/*', { cwd: dir }) + .sort(() => 0.5 - Math.random()) + .map(m => join(dir, m)) - iteration += 1 + assert( + arrSame(matches, entries), + new RunError(`glob result does not match expected`, { + found: matches, + wanted: entries, + }), + ) - yield [matches, RunError] as const - } + iteration += 1 - return [iteration, iterations] - }, - ] as const + yield [matches, RunError] as const + } + + return { + contents: readdirSync(dir), + iteration, + iterations, + } + } } // Copied from sindresorhus/del since it was reported in @@ -118,10 +119,8 @@ t.test('windows does not throw EPERM', t => { } t.test('sync', t => { - const [dir, run] = setup(t, options) - let i - const r = run() + const r = setup(t, options)() while ((i = r.next())) { if (i.done) { i = i.value @@ -149,16 +148,14 @@ t.test('windows does not throw EPERM', t => { ) } - t.strictSame(readdirSync(dir), []) - t.equal(i[0], i[1], `ran all ${i[1]} iterations`) + t.strictSame(i.contents, []) + t.equal(i.iteration, i.iterations, `ran all ${i.iteration} iterations`) t.end() }) t.test('async', async t => { - const [dir, run] = setup(t, options) - let i - const r = run() + const r = setup(t, options)() while ((i = r.next())) { if (i.done) { i = i.value @@ -188,8 +185,8 @@ t.test('windows does not throw EPERM', t => { ) } - t.strictSame(readdirSync(dir), []) - t.equal(i[0], i[1], `ran all ${i[1]} iterations`) + t.strictSame(i.contents, []) + t.equal(i.iteration, i.iterations, `ran all ${i.iteration} iterations`) }) t.end() From a82232244499277fff082d047605fa982f88292e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 6 Oct 2024 15:26:11 -0700 Subject: [PATCH 50/50] log less things --- src/fix-eperm.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/fix-eperm.ts b/src/fix-eperm.ts index ed179ed8..6fc18887 100644 --- a/src/fix-eperm.ts +++ b/src/fix-eperm.ts @@ -11,11 +11,9 @@ export const fixEPERM = return } if (fer?.code === 'EPERM') { - console.log('fixEPERM error', er) try { await chmod(path, 0o666) } catch (er2) { - console.log('fixEPERM chmod error', er2) const fer2 = er2 as NodeJS.ErrnoException if (fer2?.code === 'ENOENT') { return