From acf64a7cffae5bd568f63f4c1b24f7852b62c26e Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Thu, 21 Nov 2024 21:37:00 +0100 Subject: [PATCH] fix: cache local environment values (#120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yes this is a bit dirty because the report is now generated once at the beginning of a command which could be considered a side effect (but normally treeshaking will only include it only if needed), but the whole process.report is a weirdly built API meant for debugging and not really meant to be used in a normal program But somehow when the call is done in the beginning of the process, this call is very fast, but when it's called after having run a http request (as is currently the case, a single call takes a lot more time to complete, it takes 40s or more on my system when called at the end of npm upgrade) but only a few milliseconds when called at the beginning (this is why it's best to run it outside the function at the beginning of the process as a side effect instead of calling getReport on demand and cache the result) Here is a log of console.time('report') and console.timeEnd('report') before and after the getReport call when run in the end of the upgrade command: ā ¼report: 3:10.573 (m:ss.mmm) when run in the beginning of the process at the top level report: 1.943ms This fixes npm hanging https://github.com/npm/cli/issues/4028, https://github.com/npm/cli/issues/7814, https://github.com/npm/cli/issues/7868 --- lib/current-env.js | 36 +++++++++++-- test/check-platform.js | 111 ++++++++++++++++++++++++++++++----------- 2 files changed, 114 insertions(+), 33 deletions(-) diff --git a/lib/current-env.js b/lib/current-env.js index 9babde1..31f154a 100644 --- a/lib/current-env.js +++ b/lib/current-env.js @@ -1,5 +1,6 @@ const process = require('node:process') const nodeOs = require('node:os') +const fs = require('node:fs') function isMusl (file) { return file.includes('libc.musl-') || file.includes('ld-musl-') @@ -13,12 +14,23 @@ function cpu () { return process.arch } -function libc (osName) { - // this is to make it faster on non linux machines - if (osName !== 'linux') { +const LDD_PATH = '/usr/bin/ldd' +function getFamilyFromFilesystem () { + try { + const content = fs.readFileSync(LDD_PATH, 'utf-8') + if (content.includes('musl')) { + return 'musl' + } + if (content.includes('GNU C Library')) { + return 'glibc' + } + return null + } catch { return undefined } - let family +} + +function getFamilyFromReport () { const originalExclude = process.report.excludeNetwork process.report.excludeNetwork = true const report = process.report.getReport() @@ -27,6 +39,22 @@ function libc (osName) { family = 'glibc' } else if (Array.isArray(report.sharedObjects) && report.sharedObjects.some(isMusl)) { family = 'musl' + } else { + family = null + } + return family +} + +let family +function libc (osName) { + if (osName !== 'linux') { + return undefined + } + if (family === undefined) { + family = getFamilyFromFilesystem() + if (family === undefined) { + family = getFamilyFromReport() + } } return family } diff --git a/test/check-platform.js b/test/check-platform.js index 3d262d6..09c41e6 100644 --- a/test/check-platform.js +++ b/test/check-platform.js @@ -94,30 +94,59 @@ t.test('wrong libc with overridden libc', async t => }), { code: 'EBADPLATFORM' })) t.test('libc', (t) => { - let PLATFORM = '' - - const _processPlatform = Object.getOwnPropertyDescriptor(process, 'platform') - Object.defineProperty(process, 'platform', { - enumerable: true, - configurable: true, - get: () => PLATFORM, - }) - + let noCacheChckPtfm + let PLATFORM = 'linux' let REPORT = {} - const _processReport = process.report.getReport - process.report.getReport = () => REPORT - - t.teardown(() => { - Object.defineProperty(process, 'platform', _processPlatform) - process.report.getReport = _processReport - }) + let readFileSync + let noCache = true + + function withCache (cb) { + noCache = false + cb() + noCache = true + withoutLibcCache() + } + + function withoutLibcCache () { + readFileSync = () => { + throw new Error('File not found') + } + const original = t.mock('..', { + '../lib/current-env': t.mock('../lib/current-env', { + 'node:fs': { + readFileSync: () => { + return readFileSync() + }, + }, + 'node:process': Object.defineProperty({ + report: { + getReport: () => REPORT, + }, + }, 'platform', { + enumerable: true, + get: () => PLATFORM, + }), + }), + }).checkPlatform + noCacheChckPtfm = (...args) => { + try { + original(...args) + } finally { + if (noCache) { + withoutLibcCache() + } + } + } + } + + withoutLibcCache() t.test('fails when not in linux', (t) => { PLATFORM = 'darwin' - t.throws(() => checkPlatform({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, 'fails for glibc when not in linux') - t.throws(() => checkPlatform({ libc: 'musl' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'musl' }), { code: 'EBADPLATFORM' }, 'fails for musl when not in linux') t.end() }) @@ -125,17 +154,38 @@ t.test('libc', (t) => { t.test('glibc', (t) => { PLATFORM = 'linux' + withCache(() => { + readFileSync = () => 'this ldd file contains GNU C Library' + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'glibc' }), + 'allows glibc on glibc from ldd file') + + readFileSync = () => { + throw new Error('File not found') + } + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'glibc' }), 'allows glibc from ldd file cache') + }) + REPORT = {} - t.throws(() => checkPlatform({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, 'fails when report is missing header property') REPORT = { header: {} } - t.throws(() => checkPlatform({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, 'fails when header is missing glibcVersionRuntime property') - REPORT = { header: { glibcVersionRuntime: '1' } } - t.doesNotThrow(() => checkPlatform({ libc: 'glibc' }), 'allows glibc on glibc') - t.throws(() => checkPlatform({ libc: 'musl' }), { code: 'EBADPLATFORM' }, + withCache(() => { + REPORT = { header: { glibcVersionRuntime: '1' } } + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'glibc' }), 'allows glibc on glibc') + + REPORT = {} + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'glibc' }), 'allows glibc from report cache') + }) + + readFileSync = () => 'this ldd file is unsupported' + t.throws(() => noCacheChckPtfm({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, + 'fails when ldd file exists but is not something known') + + t.throws(() => noCacheChckPtfm({ libc: 'musl' }), { code: 'EBADPLATFORM' }, 'does not allow musl on glibc') t.end() @@ -144,25 +194,28 @@ t.test('libc', (t) => { t.test('musl', (t) => { PLATFORM = 'linux' + readFileSync = () => 'this ldd file contains musl' + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'musl' }), 'allows musl on musl from ldd file') + REPORT = {} - t.throws(() => checkPlatform({ libc: 'musl' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'musl' }), { code: 'EBADPLATFORM' }, 'fails when report is missing sharedObjects property') REPORT = { sharedObjects: {} } - t.throws(() => checkPlatform({ libc: 'musl' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'musl' }), { code: 'EBADPLATFORM' }, 'fails when sharedObjects property is not an array') REPORT = { sharedObjects: [] } - t.throws(() => checkPlatform({ libc: 'musl' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'musl' }), { code: 'EBADPLATFORM' }, 'fails when sharedObjects does not contain musl') REPORT = { sharedObjects: ['ld-musl-foo'] } - t.doesNotThrow(() => checkPlatform({ libc: 'musl' }), 'allows musl on musl as ld-musl-') + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'musl' }), 'allows musl on musl as ld-musl-') REPORT = { sharedObjects: ['libc.musl-'] } - t.doesNotThrow(() => checkPlatform({ libc: 'musl' }), 'allows musl on musl as libc.musl-') + t.doesNotThrow(() => noCacheChckPtfm({ libc: 'musl' }), 'allows musl on musl as libc.musl-') - t.throws(() => checkPlatform({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, + t.throws(() => noCacheChckPtfm({ libc: 'glibc' }), { code: 'EBADPLATFORM' }, 'does not allow glibc on musl') t.end()