From 3734005d8aac5cd79fe95af6f477cfd2d7c3373a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vin=C3=ADcius=20Louren=C3=A7o?= <contact@viniciusl.com.br>
Date: Sun, 7 Apr 2024 19:04:23 -0300
Subject: [PATCH] fix(perf): remove glob dependency

---
 lib/utils/log-file.js      | 40 +++++++++++++++++++++--------
 test/lib/utils/log-file.js | 52 +++++++++++++++-----------------------
 2 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/lib/utils/log-file.js b/lib/utils/log-file.js
index 8c06f5647e761..62b191a0d9ebf 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,39 @@ 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', '')
+
+      const files = await fs.readdir(
+        dirname(logPath), {
+          withFileTypes: true,
+          encoding: 'utf-8',
+        })
+      const logFiles = []
+
+      for (const file of files) {
+        if (!file.isFile()) {
+          continue
+        }
+
+        const genericFileName = file.name.replace(/\d/g, 'd')
+        const filePath = join(file.path, 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 +237,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')