Skip to content

Commit

Permalink
fix: cache local environment values (#120)
Browse files Browse the repository at this point in the history
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 npm/cli#4028,
npm/cli#7814,
npm/cli#7868
  • Loading branch information
Tofandel authored Nov 21, 2024
1 parent 4751cb4 commit acf64a7
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 33 deletions.
36 changes: 32 additions & 4 deletions lib/current-env.js
Original file line number Diff line number Diff line change
@@ -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-')
Expand All @@ -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()
Expand All @@ -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
}
Expand Down
111 changes: 82 additions & 29 deletions test/check-platform.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,48 +94,98 @@ 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()
})

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()
Expand All @@ -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()
Expand Down

0 comments on commit acf64a7

Please sign in to comment.