Skip to content

Commit

Permalink
feat: use proc-log and drop support for log property (#104)
Browse files Browse the repository at this point in the history
It replaces the only use of `npmlog.level` with a boolean `silent` which
is now used to to suppress `@npmcli/run-script` banners instead.

BREAKING CHANGE: this drops support for the `log` property and instead
all logs will be emitted on the process object via `proc-log`.
  • Loading branch information
lukekarrys committed Feb 14, 2022
1 parent 0413eff commit 26e01b0
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 55 deletions.
6 changes: 2 additions & 4 deletions README.md
Expand Up @@ -146,10 +146,6 @@ resolved, and other properties, as they are determined.
`0o666`. See "Extracted File Modes" below.
* `dmode` Minimum permission mode for extracted directories. Defaults to
`0o777`. See "Extracted File Modes" below.
* `log` A logger object with methods for various log levels. Typically,
this will be [`npmlog`](http://npm.im/npmlog) in the npm CLI use case,
but if not specified, the default is a logger that emits `'log'` events
on the `process` object.
* `preferOnline` Prefer to revalidate cache entries, even when it would not
be strictly necessary. Default `false`.
* `before` When picking a manifest from a packument, only consider
Expand All @@ -170,6 +166,8 @@ resolved, and other properties, as they are determined.
calls. This allows you to easily avoid hitting the registry multiple
times (even just to validate the cache) for a given packument, since it
is unlikely to change in the span of a single command.
* `silent` A boolean that determines whether the banner is displayed
when calling `@npmcli/run-script`.


### Advanced API
Expand Down
5 changes: 2 additions & 3 deletions lib/dir.js
Expand Up @@ -38,10 +38,9 @@ class DirFetcher extends Fetcher {
// but this function is *also* run when installing git deps
const stdio = this.opts.foregroundScripts ? 'inherit' : 'pipe'

// hide the banner if loglevel is silent, or if prepare running
// hide the banner if silent opt is passed in, or if prepare running
// in the background.
const banner = this.opts.log && this.opts.log.level === 'silent' ? false
: stdio === 'inherit'
const banner = this.opts.silent ? false : stdio === 'inherit'

return runScript({
pkg: mani,
Expand Down
17 changes: 8 additions & 9 deletions lib/fetcher.js
Expand Up @@ -9,7 +9,7 @@ const { promisify } = require('util')
const { basename, dirname } = require('path')
const rimraf = promisify(require('rimraf'))
const tar = require('tar')
const procLog = require('./util/proc-log.js')
const log = require('proc-log')
const retry = require('promise-retry')
const fsm = require('fs-minipass')
const cacache = require('cacache')
Expand Down Expand Up @@ -91,7 +91,6 @@ class FetcherBase {
// the process's umask setting do its job. but if configured, we do
// respect it.
this.umask = opts.umask || 0
this.log = opts.log || procLog

this.preferOnline = !!opts.preferOnline
this.preferOffline = !!opts.preferOffline
Expand Down Expand Up @@ -304,7 +303,7 @@ class FetcherBase {
this.resolved
) ? streamHandler(this[_tarballFromCache]()).catch(er => {
if (this.isDataCorruptionError(er)) {
this.log.warn('tarball', `cached data for ${
log.warn('tarball', `cached data for ${
this.spec
} (${this.integrity}) seems to be corrupted. Refreshing cache.`)
return this.cleanupCached().then(() => {
Expand All @@ -320,7 +319,7 @@ class FetcherBase {
if (!this.isRetriableError(er)) {
throw er
}
this.log.silly('tarball', `no local data for ${
log.silly('tarball', `no local data for ${
this.spec
}. Extracting by manifest.`)
}
Expand All @@ -332,7 +331,7 @@ class FetcherBase {
// IS possible that the fetch subsystem accessed the cache, and the
// entry got blown away or something. Try one more time to be sure.
if (this.isRetriableError(er)) {
this.log.warn('tarball', `tarball data for ${
log.warn('tarball', `tarball data for ${
this.spec
} (${this.integrity}) seems to be corrupted. Trying again.`)
return this.cleanupCached().then(() => tryAgain(er))
Expand Down Expand Up @@ -425,8 +424,8 @@ class FetcherBase {
})

extractor.on('error', er => {
this.log.warn('tar', er.message)
this.log.silly('tar', er)
log.warn('tar', er.message)
log.silly('tar', er)
reject(er)
})

Expand Down Expand Up @@ -482,8 +481,8 @@ class FetcherBase {
strip: 1,
onwarn: /* istanbul ignore next - we can trust that tar logs */
(code, msg, data) => {
this.log.warn('tar', code, msg)
this.log.silly('tar', code, msg, data)
log.warn('tar', code, msg)
log.silly('tar', code, msg, data)
},
uid,
gid,
Expand Down
3 changes: 2 additions & 1 deletion lib/git.js
Expand Up @@ -8,6 +8,7 @@ const pickManifest = require('npm-pick-manifest')
const npa = require('npm-package-arg')
const Minipass = require('minipass')
const cacache = require('cacache')
const log = require('proc-log')
const npm = require('./util/npm.js')

const _resolvedFromRepo = Symbol('_resolvedFromRepo')
Expand Down Expand Up @@ -172,7 +173,7 @@ class GitFetcher extends Fetcher {
const noPrepare = !process.env._PACOTE_NO_PREPARE_ ? []
: process.env._PACOTE_NO_PREPARE_.split('\n')
if (noPrepare.includes(this.resolved)) {
this.log.info('prepare', 'skip prepare, already seen', this.resolved)
log.info('prepare', 'skip prepare, already seen', this.resolved)
return
}
noPrepare.push(this.resolved)
Expand Down
21 changes: 0 additions & 21 deletions lib/util/proc-log.js

This file was deleted.

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -54,6 +54,7 @@
"npm-packlist": "^3.0.0",
"npm-pick-manifest": "^7.0.0",
"npm-registry-fetch": "^12.0.2",
"proc-log": "^2.0.0",
"promise-retry": "^2.0.1",
"read-package-json": "^4.1.1",
"read-package-json-fast": "^2.0.3",
Expand Down
8 changes: 4 additions & 4 deletions tap-snapshots/test/dir.js.test.cjs
Expand Up @@ -247,23 +247,23 @@ Object {
}
`

exports[`test/dir.js TAP responds to foregroundScripts: true and log:{level: silent} > extract 1`] = `
exports[`test/dir.js TAP responds to foregroundScripts: true and silent: true > extract 1`] = `
Object {
"from": "file:test/fixtures/prepare-script",
"integrity": "sha512-shf/7QYgFII06kJbyyqj4u86uLuyJnD0xVGLm0XDkC6nuVU+GBHwQ9uogbLUQnBu0gSvcWYVnO1TyPxj+YQDdw==",
"resolved": "\${CWD}/test/fixtures/prepare-script",
}
`

exports[`test/dir.js TAP responds to foregroundScripts: true and log:{level: silent} > file list 1`] = `
exports[`test/dir.js TAP responds to foregroundScripts: true and silent: true > file list 1`] = `
Array [
"index.js",
"package.json",
"prepare.js",
]
`

exports[`test/dir.js TAP responds to foregroundScripts: true and log:{level: silent} > manifest 1`] = `
exports[`test/dir.js TAP responds to foregroundScripts: true and silent: true > manifest 1`] = `
Object {
"_from": "file:test/fixtures/prepare-script",
"_id": "git-prepare-script@1.0.0",
Expand All @@ -282,7 +282,7 @@ Object {
}
`

exports[`test/dir.js TAP responds to foregroundScripts: true and log:{level: silent} > packument 1`] = `
exports[`test/dir.js TAP responds to foregroundScripts: true and silent: true > packument 1`] = `
Object {
"dist-tags": Object {
"latest": "1.0.0",
Expand Down
4 changes: 2 additions & 2 deletions test/dir.js
Expand Up @@ -75,9 +75,9 @@ t.test('responds to foregroundScripts: true', t => {
}, 'should run in foreground'))
})

t.test('responds to foregroundScripts: true and log:{level: silent}', t => {
t.test('responds to foregroundScripts: true and silent: true', t => {
RUNS.length = 0
const opt = { foregroundScripts: true, log: { level: 'silent' } }
const opt = { foregroundScripts: true, silent: true }
const f = new DirFetcher(preparespec, opt)
t.resolveMatchSnapshot(f.packument(), 'packument')
t.resolveMatchSnapshot(f.manifest(), 'manifest')
Expand Down
11 changes: 0 additions & 11 deletions test/util/proc-log.js

This file was deleted.

0 comments on commit 26e01b0

Please sign in to comment.