From 032419276c4939f0e2c545635c0612b3c3215327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Juli=C3=A1n=20Merelo=20Guerv=C3=B3s?= Date: Tue, 28 Nov 2023 15:07:14 +0100 Subject: [PATCH 1/7] Skip creation of log directory if it's set to an empty string After all, it can fail. This goes mainly to fix #7032 --- lib/npm.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/npm.js b/lib/npm.js index 14706629e79c2..82c57c17c8006 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -216,11 +216,13 @@ class Npm { fs.mkdir(this.cache, { recursive: true }) .catch((e) => log.verbose('cache', `could not create cache: ${e}`))) - // its ok if this fails. user might have specified an invalid dir + // it's ok if this fails. user might have specified an invalid dir // which we will tell them about at the end - await this.time('npm:load:mkdirplogs', () => - fs.mkdir(this.logsDir, { recursive: true }) - .catch((e) => log.verbose('logfile', `could not create logs-dir: ${e}`))) + if ( this.logsDir !== '' ) { + await this.time('npm:load:mkdirplogs', () => + fs.mkdir(this.logsDir, { recursive: true }) + .catch((e) => log.verbose('logfile', `could not create logs-dir: ${e}`))) + } // note: this MUST be shorter than the actual argv length, because it // uses the same memory, so node will truncate it if it's too long. From 30b1fbc9b4f700288de161a2e3b87ecf0b3ed8da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Juli=C3=A1n=20Merelo=20Guerv=C3=B3s?= Date: Tue, 28 Nov 2023 17:17:14 +0100 Subject: [PATCH 2/7] Differentiate between empty and undefined values --- lib/npm.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/npm.js b/lib/npm.js index 82c57c17c8006..d8e2a841469e0 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -353,7 +353,8 @@ class Npm { } get logsDir () { - return this.config.get('logs-dir') || join(this.cache, '_logs') + const theDir = this.config.get('logs-dir') === undefined ? '_logs' : this.config.get('logs-dir'); + return join(this.cache, theDir) } get logPath () { From 5f9a88b61ee2ef721c3ace268cf1c526bc74ad34 Mon Sep 17 00:00:00 2001 From: JJ Merelo Date: Wed, 29 Nov 2023 12:23:33 +0100 Subject: [PATCH 3/7] Restores original --- lib/npm.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/npm.js b/lib/npm.js index d8e2a841469e0..82c57c17c8006 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -353,8 +353,7 @@ class Npm { } get logsDir () { - const theDir = this.config.get('logs-dir') === undefined ? '_logs' : this.config.get('logs-dir'); - return join(this.cache, theDir) + return this.config.get('logs-dir') || join(this.cache, '_logs') } get logPath () { From e7182ceef4c68ee6ca3b9aaec117571810621cbf Mon Sep 17 00:00:00 2001 From: JJ Merelo Date: Wed, 29 Nov 2023 12:31:48 +0100 Subject: [PATCH 4/7] Using logs-max as suggested --- lib/npm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/npm.js b/lib/npm.js index 82c57c17c8006..3322131d8df44 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -218,7 +218,7 @@ class Npm { // it's ok if this fails. user might have specified an invalid dir // which we will tell them about at the end - if ( this.logsDir !== '' ) { + if (this.config.get('logs-max') > 0) { await this.time('npm:load:mkdirplogs', () => fs.mkdir(this.logsDir, { recursive: true }) .catch((e) => log.verbose('logfile', `could not create logs-dir: ${e}`))) From b714b56ad507abc84235f8acb8ad97aa57aea4f7 Mon Sep 17 00:00:00 2001 From: JJ Merelo Date: Wed, 29 Nov 2023 12:45:15 +0100 Subject: [PATCH 5/7] :white_check_mark: checks that the task is not called --- test/lib/utils/exit-handler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index 3eb5840985b8f..fac1ef3a3f241 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -344,12 +344,13 @@ t.test('no logs dir', async (t) => { const { exitHandler, logs } = await mockExitHandler(t, { config: { 'logs-max': 0 }, }) - + console.warn('logs', logs); await exitHandler(new Error()) t.match(logs.error.filter(([t]) => t === ''), [ ['', 'Log files were not written due to the config logs-max=0'], ]) + t.match(logs.filter(([_,task]) => task === 'npm.load.mkdirplogs'), []) }) t.test('timers fail to write', async (t) => { From b058c94d78133602741baab0f926d774eabcf74a Mon Sep 17 00:00:00 2001 From: JJ Merelo Date: Wed, 29 Nov 2023 12:45:49 +0100 Subject: [PATCH 6/7] Deletes de:bug: --- test/lib/utils/exit-handler.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index fac1ef3a3f241..b4930b53e8465 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -344,7 +344,6 @@ t.test('no logs dir', async (t) => { const { exitHandler, logs } = await mockExitHandler(t, { config: { 'logs-max': 0 }, }) - console.warn('logs', logs); await exitHandler(new Error()) t.match(logs.error.filter(([t]) => t === ''), [ From fa42c5431586b19be3dc9ec0f2439389c4d20845 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 29 Nov 2023 07:48:49 -0800 Subject: [PATCH 7/7] fixup: linting --- test/lib/utils/exit-handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index b4930b53e8465..b48f96d581775 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -349,7 +349,7 @@ t.test('no logs dir', async (t) => { t.match(logs.error.filter(([t]) => t === ''), [ ['', 'Log files were not written due to the config logs-max=0'], ]) - t.match(logs.filter(([_,task]) => task === 'npm.load.mkdirplogs'), []) + t.match(logs.filter(([_, task]) => task === 'npm.load.mkdirplogs'), []) }) t.test('timers fail to write', async (t) => {