diff --git a/lib/utils/log-file.js b/lib/utils/log-file.js index 8c06f5647e761..1a46b7da0d660 100644 --- a/lib/utils/log-file.js +++ b/lib/utils/log-file.js @@ -1,7 +1,6 @@ const os = require('os') const { join, dirname, basename } = require('path') const { format } = require('util') -const { glob } = require('glob') const { Minipass } = require('minipass') const fsMiniPass = require('fs-minipass') const fs = require('fs/promises') @@ -9,7 +8,6 @@ const log = require('./log-shim') const Display = require('./display') const padZero = (n, length) => n.toString().padStart(length.toString().length, '0') -const globify = pattern => pattern.split('\\').join('/') class LogFiles { // Default to a plain minipass stream so we can buffer @@ -199,17 +197,41 @@ class LogFiles { try { const logPath = this.#getLogFilePath() - const logGlob = join(dirname(logPath), basename(logPath) + const patternFileName = basename(logPath) // tell glob to only match digits - .replace(/\d/g, '[0123456789]') + .replace(/\d/g, 'd') // Handle the old (prior to 8.2.0) log file names which did not have a // counter suffix - .replace(/-\.log$/, '*.log') - ) + .replace('-.log', '') + + let files = await fs.readdir( + dirname(logPath), { + withFileTypes: true, + encoding: 'utf-8', + }) + files = files.sort((a, b) => basename(a.name).localeCompare(basename(b.name), 'en')) + + const logFiles = [] + + for (const file of files) { + if (!file.isFile()) { + continue + } + + const genericFileName = file.name.replace(/\d/g, 'd') + const filePath = join(dirname(logPath), basename(file.name)) + + // Always ignore the currently written files + if ( + genericFileName.includes(patternFileName) + && genericFileName.endsWith('.log') + && !this.#files.includes(filePath) + ) { + logFiles.push(filePath) + } + } - // Always ignore the currently written files - const files = await glob(globify(logGlob), { ignore: this.#files.map(globify), silent: true }) - const toDelete = files.length - this.#logsMax + const toDelete = logFiles.length - this.#logsMax if (toDelete <= 0) { return @@ -217,7 +239,7 @@ class LogFiles { log.silly('logfile', `start cleaning logs, removing ${toDelete} files`) - for (const file of files.slice(0, toDelete)) { + for (const file of logFiles.slice(0, toDelete)) { try { await fs.rm(file, { force: true }) } catch (e) { diff --git a/test/lib/utils/log-file.js b/test/lib/utils/log-file.js index c02f338a84ee0..f34dda8f52433 100644 --- a/test/lib/utils/log-file.js +++ b/test/lib/utils/log-file.js @@ -57,8 +57,10 @@ const loadLogFile = async (t, { buffer = [], mocks, testdir = {}, ...options } = logFile, LogFile, readLogs: async () => { - const logDir = await fs.readdir(root) - const logFiles = logDir.map((f) => path.join(root, f)) + const logDir = await fs.readdir(root, { withFileTypes: true }) + const logFiles = logDir + .filter(f => f.isFile()) + .map((f) => path.join(root, f.name)) .filter((f) => _fs.existsSync(f)) return Promise.all(logFiles.map(async (f) => { const content = await fs.readFile(f, 'utf8') @@ -202,6 +204,22 @@ t.test('cleans logs', async t => { t.equal(logs.length, logsMax + 1) }) +t.test('cleans logs even when find folder inside logs folder', async t => { + const logsMax = 5 + const { readLogs } = await loadLogFile(t, { + logsMax, + testdir: { + ...makeOldLogs(10), + ignore_folder: { + 'ignored-file.txt': 'hello', + }, + }, + }) + + const logs = await readLogs() + t.equal(logs.length, logsMax + 1) +}) + t.test('doesnt clean current log by default', async t => { const logsMax = 1 const { readLogs, logFile } = await loadLogFile(t, { @@ -240,35 +258,6 @@ t.test('doesnt need to clean', async t => { t.equal(logs.length, oldLogs + 1) }) -t.test('glob error', async t => { - const { readLogs } = await loadLogFile(t, { - logsMax: 5, - mocks: { - glob: { glob: () => { - throw new Error('bad glob') - } }, - }, - }) - - const logs = await readLogs() - t.equal(logs.length, 1) - t.match(last(logs).content, /error cleaning log files .* bad glob/) -}) - -t.test('do not log cleaning errors when logging is disabled', async t => { - const { readLogs } = await loadLogFile(t, { - logsMax: 0, - mocks: { - glob: () => { - throw new Error('should not be logged') - }, - }, - }) - - const logs = await readLogs() - t.equal(logs.length, 0) -}) - t.test('cleans old style logs too', async t => { const logsMax = 5 const oldLogs = 10 @@ -290,6 +279,7 @@ t.test('rimraf error', async t => { testdir: makeOldLogs(oldLogs), mocks: { 'fs/promises': { + readdir: fs.readdir, rm: async (...args) => { if (count >= 3) { throw new Error('bad rimraf') diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index b09ecc478f64f..c731f72c4e578 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -2,7 +2,6 @@ const { walkUp } = require('walk-up-path') const ini = require('ini') const nopt = require('nopt') -const mapWorkspaces = require('@npmcli/map-workspaces') const rpj = require('read-package-json-fast') const log = require('proc-log') @@ -704,6 +703,7 @@ class Config { continue } + const mapWorkspaces = require('@npmcli/map-workspaces') const workspaces = await mapWorkspaces({ cwd: p, pkg }) for (const w of workspaces.values()) { if (w === this.localPrefix) {