From 1b6950b993f07f87cf66a0e4d76e646939174d88 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 6 May 2024 09:02:02 -0700 Subject: [PATCH] fix(refactor): symbol cleanup (#365) fix: move bin to its own directory --- lib/bin.js => bin/index.js | 1 - lib/dir.js | 13 ++---- lib/fetcher.js | 69 +++++++++++---------------- lib/file.js | 15 +++--- lib/git.js | 96 +++++++++++++++++--------------------- lib/index.js | 10 ++-- lib/registry.js | 25 ++++------ lib/remote.js | 12 ++--- lib/util/protected.js | 11 +++++ package.json | 2 +- test/bin.js | 13 +++--- test/dir.js | 5 +- test/fetcher.js | 26 +++++------ test/file.js | 5 +- test/git.js | 36 +++++++------- test/index.js | 3 +- test/registry.js | 10 ++-- test/remote.js | 10 ++-- 18 files changed, 165 insertions(+), 197 deletions(-) rename lib/bin.js => bin/index.js (99%) create mode 100644 lib/util/protected.js diff --git a/lib/bin.js b/bin/index.js similarity index 99% rename from lib/bin.js rename to bin/index.js index d5df6362..f35b62ca 100755 --- a/lib/bin.js +++ b/bin/index.js @@ -1,5 +1,4 @@ #!/usr/bin/env node -/* eslint-disable no-console */ const run = conf => { const pacote = require('../') diff --git a/lib/dir.js b/lib/dir.js index 135be8e6..6674bb0b 100644 --- a/lib/dir.js +++ b/lib/dir.js @@ -4,13 +4,10 @@ const { Minipass } = require('minipass') const tarCreateOptions = require('./util/tar-create-options.js') const packlist = require('npm-packlist') const tar = require('tar') -const _prepareDir = Symbol('_prepareDir') const { resolve } = require('path') -const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson') - const runScript = require('@npmcli/run-script') +const _ = require('./util/protected.js') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') class DirFetcher extends Fetcher { constructor (spec, opts) { super(spec, opts) @@ -30,7 +27,7 @@ class DirFetcher extends Fetcher { return ['directory'] } - [_prepareDir] () { + [_.prepareDir] () { return this.manifest().then(mani => { if (!mani.scripts || !mani.scripts.prepare) { return @@ -55,7 +52,7 @@ class DirFetcher extends Fetcher { }) } - [_tarballFromResolved] () { + [_.tarballFromResolved] () { if (!this.tree && !this.Arborist) { throw new Error('DirFetcher requires either a tree or an Arborist constructor to pack') } @@ -68,7 +65,7 @@ class DirFetcher extends Fetcher { // run the prepare script, get the list of files, and tar it up // pipe to the stream, and proxy errors the chain. - this[_prepareDir]() + this[_.prepareDir]() .then(async () => { if (!this.tree) { const arb = new this.Arborist({ path: this.resolved }) @@ -87,7 +84,7 @@ class DirFetcher extends Fetcher { return Promise.resolve(this.package) } - return this[_readPackageJson](this.resolved) + return this[_.readPackageJson](this.resolved) .then(mani => this.package = { ...mani, _integrity: this.integrity && String(this.integrity), diff --git a/lib/fetcher.js b/lib/fetcher.js index c4a707e7..ec5a807b 100644 --- a/lib/fetcher.js +++ b/lib/fetcher.js @@ -18,27 +18,12 @@ const getContents = require('@npmcli/installed-package-contents') const PackageJson = require('@npmcli/package-json') const { Minipass } = require('minipass') const cacheDir = require('./util/cache-dir.js') +const _ = require('./util/protected.js') // Pacote is only concerned with the package.json contents const packageJsonPrepare = (p) => PackageJson.prepare(p).then(pkg => pkg.content) const packageJsonNormalize = (p) => PackageJson.normalize(p).then(pkg => pkg.content) -// Private methods. -// Child classes should not have to override these. -// Users should never call them. -const _extract = Symbol('_extract') -const _mkdir = Symbol('_mkdir') -const _empty = Symbol('_empty') -const _toFile = Symbol('_toFile') -const _tarxOptions = Symbol('_tarxOptions') -const _entryMode = Symbol('_entryMode') -const _istream = Symbol('_istream') -const _assertType = Symbol('_assertType') -const _tarballFromCache = Symbol('_tarballFromCache') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') -const _cacheFetches = Symbol.for('pacote.Fetcher._cacheFetches') -const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson') - class FetcherBase { constructor (spec, opts) { if (!opts || typeof opts !== 'object') { @@ -57,7 +42,7 @@ class FetcherBase { this.from = this.spec.registry ? `${this.spec.name}@${this.spec.rawSpec}` : this.spec.saveSpec - this[_assertType]() + this.#assertType() // clone the opts object so that others aren't upset when we mutate it // by adding/modifying the integrity value. this.opts = { ...opts } @@ -93,11 +78,9 @@ class FetcherBase { this.before = opts.before this.fullMetadata = this.before ? true : !!opts.fullMetadata this.fullReadJson = !!opts.fullReadJson - if (this.fullReadJson) { - this[_readPackageJson] = packageJsonPrepare - } else { - this[_readPackageJson] = packageJsonNormalize - } + this[_.readPackageJson] = this.fullReadJson + ? packageJsonPrepare + : packageJsonNormalize // rrh is a registry hostname or 'never' or 'always' // defaults to registry.npmjs.org @@ -188,7 +171,7 @@ class FetcherBase { // private, should be overridden. // Note that they should *not* calculate or check integrity or cache, // but *just* return the raw tarball data stream. - [_tarballFromResolved] () { + [_.tarballFromResolved] () { throw this.notImplementedError } @@ -204,17 +187,17 @@ class FetcherBase { // private // Note: cacache will raise a EINTEGRITY error if the integrity doesn't match - [_tarballFromCache] () { + #tarballFromCache () { return cacache.get.stream.byDigest(this.cache, this.integrity, this.opts) } - get [_cacheFetches] () { + get [_.cacheFetches] () { return true } - [_istream] (stream) { + #istream (stream) { // if not caching this, just return it - if (!this.opts.cache || !this[_cacheFetches]) { + if (!this.opts.cache || !this[_.cacheFetches]) { // instead of creating a new integrity stream, we only piggyback on the // provided stream's events if (stream.hasIntegrityEmitter) { @@ -267,7 +250,7 @@ class FetcherBase { return false } - [_assertType] () { + #assertType () { if (this.types && !this.types.includes(this.spec.type)) { throw new TypeError(`Wrong spec type (${ this.spec.type @@ -306,7 +289,7 @@ class FetcherBase { !this.preferOnline && this.integrity && this.resolved - ) ? streamHandler(this[_tarballFromCache]()).catch(er => { + ) ? streamHandler(this.#tarballFromCache()).catch(er => { if (this.isDataCorruptionError(er)) { log.warn('tarball', `cached data for ${ this.spec @@ -329,7 +312,7 @@ class FetcherBase { }. Extracting by manifest.`) } return this.resolve().then(() => retry(tryAgain => - streamHandler(this[_istream](this[_tarballFromResolved]())) + streamHandler(this.#istream(this[_.tarballFromResolved]())) .catch(streamErr => { // Most likely data integrity. A cache ENOENT error is unlikely // here, since we're definitely not reading from the cache, but it @@ -352,24 +335,24 @@ class FetcherBase { return cacache.rm.content(this.cache, this.integrity, this.opts) } - [_empty] (path) { + #empty (path) { return getContents({ path, depth: 1 }).then(contents => Promise.all( contents.map(entry => fs.rm(entry, { recursive: true, force: true })))) } - async [_mkdir] (dest) { - await this[_empty](dest) + async #mkdir (dest) { + await this.#empty(dest) return await fs.mkdir(dest, { recursive: true }) } // extraction is always the same. the only difference is where // the tarball comes from. async extract (dest) { - await this[_mkdir](dest) - return this.tarballStream((tarball) => this[_extract](dest, tarball)) + await this.#mkdir(dest) + return this.tarballStream((tarball) => this.#extract(dest, tarball)) } - [_toFile] (dest) { + #toFile (dest) { return this.tarballStream(str => new Promise((res, rej) => { const writer = new fsm.WriteStream(dest) str.on('error', er => writer.emit('error', er)) @@ -383,15 +366,15 @@ class FetcherBase { })) } - // don't use this[_mkdir] because we don't want to rimraf anything + // don't use this.#mkdir because we don't want to rimraf anything async tarballFile (dest) { const dir = dirname(dest) await fs.mkdir(dir, { recursive: true }) - return this[_toFile](dest) + return this.#toFile(dest) } - [_extract] (dest, tarball) { - const extractor = tar.x(this[_tarxOptions]({ cwd: dest })) + #extract (dest, tarball) { + const extractor = tar.x(this.#tarxOptions({ cwd: dest })) const p = new Promise((resolve, reject) => { extractor.on('end', () => { resolve({ @@ -416,7 +399,7 @@ class FetcherBase { // always ensure that entries are at least as permissive as our configured // dmode/fmode, but never more permissive than the umask allows. - [_entryMode] (path, mode, type) { + #entryMode (path, mode, type) { const m = /Directory|GNUDumpDir/.test(type) ? this.dmode : /File$/.test(type) ? this.fmode : /* istanbul ignore next - should never happen in a pkg */ 0 @@ -427,7 +410,7 @@ class FetcherBase { return ((mode | m) & ~this.umask) | exe | 0o600 } - [_tarxOptions] ({ cwd }) { + #tarxOptions ({ cwd }) { const sawIgnores = new Set() return { cwd, @@ -437,7 +420,7 @@ class FetcherBase { if (/Link$/.test(entry.type)) { return false } - entry.mode = this[_entryMode](entry.path, entry.mode, entry.type) + entry.mode = this.#entryMode(entry.path, entry.mode, entry.type) // this replicates the npm pack behavior where .gitignore files // are treated like .npmignore files, but only if a .npmignore // file is not present. diff --git a/lib/file.js b/lib/file.js index 95769de1..307efedb 100644 --- a/lib/file.js +++ b/lib/file.js @@ -3,10 +3,7 @@ const cacache = require('cacache') const { resolve } = require('path') const { stat, chmod } = require('fs/promises') const Fetcher = require('./fetcher.js') - -const _exeBins = Symbol('_exeBins') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') -const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson') +const _ = require('./util/protected.js') class FileFetcher extends Fetcher { constructor (spec, opts) { @@ -27,7 +24,7 @@ class FileFetcher extends Fetcher { // have to unpack the tarball for this. return cacache.tmp.withTmp(this.cache, this.opts, dir => this.extract(dir) - .then(() => this[_readPackageJson](dir)) + .then(() => this[_.readPackageJson](dir)) .then(mani => this.package = { ...mani, _integrity: this.integrity && String(this.integrity), @@ -36,7 +33,7 @@ class FileFetcher extends Fetcher { })) } - [_exeBins] (pkg, dest) { + #exeBins (pkg, dest) { if (!pkg.bin) { return Promise.resolve() } @@ -65,11 +62,11 @@ class FileFetcher extends Fetcher { // but if not, read the unpacked manifest and chmod properly. return super.extract(dest) .then(result => this.package ? result - : this[_readPackageJson](dest).then(pkg => - this[_exeBins](pkg, dest)).then(() => result)) + : this[_.readPackageJson](dest).then(pkg => + this.#exeBins(pkg, dest)).then(() => result)) } - [_tarballFromResolved] () { + [_.tarballFromResolved] () { // create a read stream and return it return new fsm.ReadStream(this.resolved) } diff --git a/lib/git.js b/lib/git.js index 2cac44ae..23f4b1d2 100644 --- a/lib/git.js +++ b/lib/git.js @@ -2,7 +2,6 @@ const Fetcher = require('./fetcher.js') const FileFetcher = require('./file.js') const RemoteFetcher = require('./remote.js') const DirFetcher = require('./dir.js') -const hashre = /^[a-f0-9]{40}$/ const git = require('@npmcli/git') const pickManifest = require('npm-pick-manifest') const npa = require('npm-package-arg') @@ -10,19 +9,10 @@ const { Minipass } = require('minipass') const cacache = require('cacache') const { log } = require('proc-log') const npm = require('./util/npm.js') - -const _resolvedFromRepo = Symbol('_resolvedFromRepo') -const _resolvedFromHosted = Symbol('_resolvedFromHosted') -const _resolvedFromClone = Symbol('_resolvedFromClone') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') -const _addGitSha = Symbol('_addGitSha') const addGitSha = require('./util/add-git-sha.js') -const _clone = Symbol('_clone') -const _cloneHosted = Symbol('_cloneHosted') -const _cloneRepo = Symbol('_cloneRepo') -const _setResolvedWithSha = Symbol('_setResolvedWithSha') -const _prepareDir = Symbol('_prepareDir') -const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson') +const _ = require('./util/protected.js') + +const hashre = /^[a-f0-9]{40}$/ // get the repository url. // prefer https if there's auth, since ssh will drop that. @@ -84,8 +74,9 @@ class GitFetcher extends Fetcher { // fetch the git repo and then look at the current hash const h = this.spec.hosted // try to use ssh, fall back to git. - return h ? this[_resolvedFromHosted](h) - : this[_resolvedFromRepo](this.spec.fetchSpec) + return h + ? this.#resolvedFromHosted(h) + : this.#resolvedFromRepo(this.spec.fetchSpec) } // first try https, since that's faster and passphrase-less for @@ -93,23 +84,22 @@ class GitFetcher extends Fetcher { // Fall back to SSH to support private repos // NB: we always store the https url in resolved field if auth // is present, otherwise ssh if the hosted type provides it - [_resolvedFromHosted] (hosted) { - return this[_resolvedFromRepo](hosted.https && hosted.https()) - .catch(er => { - // Throw early since we know pathspec errors will fail again if retried - if (er instanceof git.errors.GitPathspecError) { - throw er - } - const ssh = hosted.sshurl && hosted.sshurl() - // no fallthrough if we can't fall through or have https auth - if (!ssh || hosted.auth) { - throw er - } - return this[_resolvedFromRepo](ssh) - }) + #resolvedFromHosted (hosted) { + return this.#resolvedFromRepo(hosted.https && hosted.https()).catch(er => { + // Throw early since we know pathspec errors will fail again if retried + if (er instanceof git.errors.GitPathspecError) { + throw er + } + const ssh = hosted.sshurl && hosted.sshurl() + // no fallthrough if we can't fall through or have https auth + if (!ssh || hosted.auth) { + throw er + } + return this.#resolvedFromRepo(ssh) + }) } - [_resolvedFromRepo] (gitRemote) { + #resolvedFromRepo (gitRemote) { // XXX make this a custom error class if (!gitRemote) { return Promise.reject(new Error(`No git url for ${this.spec}`)) @@ -130,17 +120,17 @@ class GitFetcher extends Fetcher { // the committish provided isn't in the rev list // things like HEAD~3 or @yesterday can land here. if (!revDoc || !revDoc.sha) { - return this[_resolvedFromClone]() + return this.#resolvedFromClone() } this.resolvedRef = revDoc this.resolvedSha = revDoc.sha - this[_addGitSha](revDoc.sha) + this.#addGitSha(revDoc.sha) return this.resolved }) } - [_setResolvedWithSha] (withSha) { + #setResolvedWithSha (withSha) { // we haven't cloned, so a tgz download is still faster // of course, if it's not a known host, we can't do that. this.resolved = !this.spec.hosted ? withSha @@ -149,18 +139,18 @@ class GitFetcher extends Fetcher { // when we get the git sha, we affix it to our spec to build up // either a git url with a hash, or a tarball download URL - [_addGitSha] (sha) { - this[_setResolvedWithSha](addGitSha(this.spec, sha)) + #addGitSha (sha) { + this.#setResolvedWithSha(addGitSha(this.spec, sha)) } - [_resolvedFromClone] () { + #resolvedFromClone () { // do a full or shallow clone, then look at the HEAD // kind of wasteful, but no other option, really - return this[_clone](() => this.resolved) + return this.#clone(() => this.resolved) } - [_prepareDir] (dir) { - return this[_readPackageJson](dir).then(mani => { + #prepareDir (dir) { + return this[_.readPackageJson](dir).then(mani => { // no need if we aren't going to do any preparation. const scripts = mani.scripts if (!mani.workspaces && (!scripts || !( @@ -200,13 +190,13 @@ class GitFetcher extends Fetcher { }) } - [_tarballFromResolved] () { + [_.tarballFromResolved] () { const stream = new Minipass() stream.resolved = this.resolved stream.from = this.from // check it out and then shell out to the DirFetcher tarball packer - this[_clone](dir => this[_prepareDir](dir) + this.#clone(dir => this.#prepareDir(dir) .then(() => new Promise((res, rej) => { if (!this.Arborist) { throw new Error('GitFetcher requires an Arborist constructor to pack a tarball') @@ -217,7 +207,7 @@ class GitFetcher extends Fetcher { resolved: null, integrity: null, }) - const dirStream = df[_tarballFromResolved]() + const dirStream = df[_.tarballFromResolved]() dirStream.on('error', rej) dirStream.on('end', res) dirStream.pipe(stream) @@ -235,7 +225,7 @@ class GitFetcher extends Fetcher { // TODO: after cloning, create a tarball of the folder, and add to the cache // with cacache.put.stream(), using a key that's deterministic based on the // spec and repo, so that we don't ever clone the same thing multiple times. - [_clone] (handler, tarballOk = true) { + #clone (handler, tarballOk = true) { const o = { tmpPrefix: 'git-clone' } const ref = this.resolvedSha || this.spec.gitCommittish const h = this.spec.hosted @@ -258,7 +248,7 @@ class GitFetcher extends Fetcher { }).extract(tmp).then(() => handler(tmp), er => { // fall back to ssh download if tarball fails if (er.constructor.name.match(/^Http/)) { - return this[_clone](handler, false) + return this.#clone(handler, false) } else { throw er } @@ -266,12 +256,12 @@ class GitFetcher extends Fetcher { } const sha = await ( - h ? this[_cloneHosted](ref, tmp) - : this[_cloneRepo](this.spec.fetchSpec, ref, tmp) + h ? this.#cloneHosted(ref, tmp) + : this.#cloneRepo(this.spec.fetchSpec, ref, tmp) ) this.resolvedSha = sha if (!this.resolved) { - await this[_addGitSha](sha) + await this.#addGitSha(sha) } return handler(tmp) }) @@ -282,9 +272,9 @@ class GitFetcher extends Fetcher { // Fall back to SSH to support private repos // NB: we always store the https url in resolved field if auth // is present, otherwise ssh if the hosted type provides it - [_cloneHosted] (ref, tmp) { + #cloneHosted (ref, tmp) { const hosted = this.spec.hosted - return this[_cloneRepo](hosted.https({ noCommittish: true }), ref, tmp) + return this.#cloneRepo(hosted.https({ noCommittish: true }), ref, tmp) .catch(er => { // Throw early since we know pathspec errors will fail again if retried if (er instanceof git.errors.GitPathspecError) { @@ -295,11 +285,11 @@ class GitFetcher extends Fetcher { if (!ssh || hosted.auth) { throw er } - return this[_cloneRepo](ssh, ref, tmp) + return this.#cloneRepo(ssh, ref, tmp) }) } - [_cloneRepo] (repo, ref, tmp) { + #cloneRepo (repo, ref, tmp) { const { opts, spec } = this return git.clone(repo, ref, tmp, { ...opts, spec }) } @@ -311,8 +301,8 @@ class GitFetcher extends Fetcher { return this.spec.hosted && this.resolved ? FileFetcher.prototype.manifest.apply(this) - : this[_clone](dir => - this[_readPackageJson](dir) + : this.#clone(dir => + this[_.readPackageJson](dir) .then(mani => this.package = { ...mani, _resolved: this.resolved, diff --git a/lib/index.js b/lib/index.js index cbcbd7c9..f35314d2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -5,6 +5,10 @@ const FileFetcher = require('./file.js') const DirFetcher = require('./dir.js') const RemoteFetcher = require('./remote.js') +const tarball = (spec, opts) => get(spec, opts).tarball() +tarball.stream = (spec, handler, opts) => get(spec, opts).tarballStream(handler) +tarball.file = (spec, dest, opts) => get(spec, opts).tarballFile(dest) + module.exports = { GitFetcher, RegistryFetcher, @@ -14,10 +18,6 @@ module.exports = { resolve: (spec, opts) => get(spec, opts).resolve(), extract: (spec, dest, opts) => get(spec, opts).extract(dest), manifest: (spec, opts) => get(spec, opts).manifest(), - tarball: (spec, opts) => get(spec, opts).tarball(), packument: (spec, opts) => get(spec, opts).packument(), + tarball, } -module.exports.tarball.stream = (spec, handler, opts) => - get(spec, opts).tarballStream(handler) -module.exports.tarball.file = (spec, dest, opts) => - get(spec, opts).tarballFile(dest) diff --git a/lib/registry.js b/lib/registry.js index b6a8d49b..3dc2ccc0 100644 --- a/lib/registry.js +++ b/lib/registry.js @@ -1,6 +1,5 @@ const Fetcher = require('./fetcher.js') const RemoteFetcher = require('./remote.js') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') const pacoteVersion = require('../package.json').version const removeTrailingSlashes = require('./util/trailing-slashes.js') const PackageJson = require('@npmcli/package-json') @@ -9,6 +8,8 @@ const ssri = require('ssri') const crypto = require('crypto') const npa = require('npm-package-arg') const sigstore = require('sigstore') +const fetch = require('npm-registry-fetch') +const _ = require('./util/protected.js') // Corgis are cute. 🐕🐶 const corgiDoc = 'application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*' @@ -18,9 +19,6 @@ const fullDoc = 'application/json' // cutoff date. const MISSING_TIME_CUTOFF = '2015-01-01T00:00:00.000Z' -const fetch = require('npm-registry-fetch') - -const _headers = Symbol('_headers') class RegistryFetcher extends Fetcher { constructor (spec, opts) { super(spec, opts) @@ -63,7 +61,7 @@ class RegistryFetcher extends Fetcher { return this.resolved } - [_headers] () { + #headers () { return { // npm will override UA, but ensure that we always send *something* 'user-agent': this.opts.userAgent || @@ -80,7 +78,7 @@ class RegistryFetcher extends Fetcher { // note this might be either an in-flight promise for a request, // or the actual packument, but we never want to make more than // one request at a time for the same thing regardless. - if (this.packumentCache && this.packumentCache.has(this.packumentUrl)) { + if (this.packumentCache?.has(this.packumentUrl)) { return this.packumentCache.get(this.packumentUrl) } @@ -90,21 +88,18 @@ class RegistryFetcher extends Fetcher { try { const res = await fetch(this.packumentUrl, { ...this.opts, - headers: this[_headers](), + headers: this.#headers(), spec: this.spec, + // never check integrity for packuments themselves integrity: null, }) const packument = await res.json() packument._contentLength = +res.headers.get('content-length') - if (this.packumentCache) { - this.packumentCache.set(this.packumentUrl, packument) - } + this.packumentCache?.set(this.packumentUrl, packument) return packument } catch (err) { - if (this.packumentCache) { - this.packumentCache.delete(this.packumentUrl) - } + this.packumentCache?.delete(this.packumentUrl) if (err.code !== 'E404' || this.fullMetadata) { throw err } @@ -350,13 +345,13 @@ class RegistryFetcher extends Fetcher { return this.package } - [_tarballFromResolved] () { + [_.tarballFromResolved] () { // we use a RemoteFetcher to get the actual tarball stream return new RemoteFetcher(this.resolved, { ...this.opts, resolved: this.resolved, pkgid: `registry:${this.spec.name}@${this.resolved}`, - })[_tarballFromResolved]() + })[_.tarballFromResolved]() } get types () { diff --git a/lib/remote.js b/lib/remote.js index fd617459..9a743322 100644 --- a/lib/remote.js +++ b/lib/remote.js @@ -1,12 +1,10 @@ const Fetcher = require('./fetcher.js') const FileFetcher = require('./file.js') -const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') const pacoteVersion = require('../package.json').version const fetch = require('npm-registry-fetch') const { Minipass } = require('minipass') +const _ = require('./util/protected.js') -const _cacheFetches = Symbol.for('pacote.Fetcher._cacheFetches') -const _headers = Symbol('_headers') class RemoteFetcher extends Fetcher { constructor (spec, opts) { super(spec, opts) @@ -25,17 +23,17 @@ class RemoteFetcher extends Fetcher { // Don't need to cache tarball fetches in pacote, because make-fetch-happen // will write into cacache anyway. - get [_cacheFetches] () { + get [_.cacheFetches] () { return false } - [_tarballFromResolved] () { + [_.tarballFromResolved] () { const stream = new Minipass() stream.hasIntegrityEmitter = true const fetchOpts = { ...this.opts, - headers: this[_headers](), + headers: this.#headers(), spec: this.spec, integrity: this.integrity, algorithms: [this.pickIntegrityAlgorithm()], @@ -59,7 +57,7 @@ class RemoteFetcher extends Fetcher { return stream } - [_headers] () { + #headers () { return { // npm will override this, but ensure that we always send *something* 'user-agent': this.opts.userAgent || diff --git a/lib/util/protected.js b/lib/util/protected.js new file mode 100644 index 00000000..33345501 --- /dev/null +++ b/lib/util/protected.js @@ -0,0 +1,11 @@ +const readPackageJson = Symbol.for('package.Fetcher._readPackageJson') +const prepareDir = Symbol('_prepareDir') +const tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved') +const cacheFetches = Symbol.for('pacote.Fetcher._cacheFetches') + +module.exports = { + readPackageJson, + prepareDir, + tarballFromResolved, + cacheFetches, +} diff --git a/package.json b/package.json index 3e66c41c..44d5afdf 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "JavaScript package downloader", "author": "GitHub Inc.", "bin": { - "pacote": "lib/bin.js" + "pacote": "bin/index.js" }, "license": "ISC", "main": "lib/index.js", diff --git a/test/bin.js b/test/bin.js index e91bf289..3fe42448 100644 --- a/test/bin.js +++ b/test/bin.js @@ -1,13 +1,15 @@ -const bin = require.resolve('../lib/bin.js') -const { main, run, parseArg, parse } = require('../lib/bin.js') +const pkg = require('../package.json') +const bin = require.resolve(`../${pkg.bin.pacote}`) +const { main, run, parseArg, parse } = require(bin) const { spawn } = require('child_process') const t = require('tap') -const version = require('../package.json').version +const pacote = require('../') +const { Minipass } = require('minipass') + t.cleanSnapshot = str => - str.split(version).join('{VERSION}') + str.split(pkg.version).join('{VERSION}') .split(process.env.HOME).join('{HOME}') -const pacote = require('../') pacote.resolve = (spec, conf) => spec === 'fail' ? Promise.reject(new Error('fail')) : spec === 'string' ? Promise.resolve('just a string') @@ -22,7 +24,6 @@ pacote.manifest = (spec, conf) => Promise.resolve({ }) pacote.packument = (spec, conf) => Promise.resolve({ method: 'packument', spec, conf }) pacote.tarball.file = (spec, file, conf) => Promise.resolve({ method: 'tarball', spec, file, conf }) -const { Minipass } = require('minipass') pacote.tarball.stream = (spec, handler) => handler(new Minipass().end('tarball data')) pacote.extract = (spec, dest, conf) => Promise.resolve({ method: 'extract', spec, dest, conf }) diff --git a/test/dir.js b/test/dir.js index c61c1aaa..bfc5beb2 100644 --- a/test/dir.js +++ b/test/dir.js @@ -2,6 +2,8 @@ const runScript = require('@npmcli/run-script') const RUNS = [] const t = require('tap') const Arborist = require('@npmcli/arborist') +const fs = require('fs') +const { relative, resolve, basename } = require('path') const loadActual = async (path) => { const arb = new Arborist({ path }) @@ -17,9 +19,6 @@ const DirFetcher = t.mock('../lib/dir.js', { }, }) -const fs = require('fs') -const { relative, resolve, basename } = require('path') - const me = t.testdir() t.cleanSnapshot = str => str diff --git a/test/fetcher.js b/test/fetcher.js index 6bff73eb..f475cad0 100644 --- a/test/fetcher.js +++ b/test/fetcher.js @@ -1,4 +1,15 @@ const fs = require('fs') +const { relative, resolve, basename } = require('path') +const t = require('tap') +const Fetcher = require('../lib/fetcher.js') +const abbrevMani = require('./fixtures/abbrev-manifest-min.json') +const cacache = require('cacache') +const { Minipass } = require('minipass') +// we actually use a file fetcher for this, because we need implementations +const FileFetcher = require('../lib/file.js') +const npa = require('npm-package-arg') + +const byDigest = cacache.get.stream.byDigest fs.utimes = () => { throw new Error('do not call utimes') @@ -13,26 +24,13 @@ fs.futimesSync = () => { throw new Error('do not call futimesSync') } -const { relative, resolve, basename } = require('path') -const t = require('tap') -const me = t.testdir() -const Fetcher = require('../lib/fetcher.js') t.cleanSnapshot = s => s.split(process.cwd()).join('{CWD}') -const npa = require('npm-package-arg') - +const me = t.testdir() const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz') const abbrevspec = `file:${relative(process.cwd(), abbrev)}` -const abbrevMani = require('./fixtures/abbrev-manifest-min.json') const weird = resolve(__dirname, 'fixtures/weird-pkg.tgz') const weirdspec = `file:${relative(process.cwd(), weird)}` - -const cacache = require('cacache') -const byDigest = cacache.get.stream.byDigest -const { Minipass } = require('minipass') - -// we actually use a file fetcher for this, because we need implementations -const FileFetcher = require('../lib/file.js') const cache = resolve(me, 'cache') t.test('do not mutate opts object passed in', t => { diff --git a/test/file.js b/test/file.js index f8b7f2c0..08b9e308 100644 --- a/test/file.js +++ b/test/file.js @@ -2,10 +2,11 @@ const FileFetcher = require('../lib/file.js') const t = require('tap') const { relative, resolve, basename } = require('path') const fs = require('fs') -const me = t.testdir({ cache: {} }) -const cache = resolve(me, 'cache') + t.cleanSnapshot = str => str.split(process.cwd()).join('${CWD}') +const me = t.testdir({ cache: {} }) +const cache = resolve(me, 'cache') const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz') const abbrevspec = `file:${relative(process.cwd(), abbrev)}` diff --git a/test/git.js b/test/git.js index 94ff5c6b..317b8664 100644 --- a/test/git.js +++ b/test/git.js @@ -1,7 +1,22 @@ +const GitFetcher = require('../lib/git.js') +const t = require('tap') +const fs = require('fs') +const http = require('http') +const { dirname, basename, resolve } = require('path') +const HostedGit = require('hosted-git-info') +const npa = require('npm-package-arg') +const Arborist = require('@npmcli/arborist') +const spawnGit = require('@npmcli/git').spawn +const { spawn } = require('child_process') +const spawnNpm = require('../lib/util/npm.js') +const { mkdir } = require('fs/promises') +const { rmSync } = require('fs') + +const tar = require('tar') + // set this up first, so we can use 127.0.0.1 as our "hosted git" service const httpPort = 18000 + (+process.env.TAP_CHILD_ID || 0) const hostedUrl = `http://localhost:${httpPort}` -const HostedGit = require('hosted-git-info') const gitPort = 12345 + (+process.env.TAP_CHILD_ID || 0) HostedGit.addHost('localhost', { @@ -52,16 +67,9 @@ const submodsRemote = `git://localhost:${gitPort}/submodule-repo` const workspacesRemote = `git://localhost:${gitPort}/workspaces-repo` const prepackRemote = `git://localhost:${gitPort}/prepack-repo` -const GitFetcher = require('../lib/git.js') -const t = require('tap') -const fs = require('fs') -const http = require('http') - -const { dirname, basename, resolve } = require('path') const fixtures = resolve(__dirname, 'fixtures') const abbrev = resolve(fixtures, 'abbrev-1.1.1.tgz') const prepIgnore = resolve(fixtures, 'prepare-requires-gitignore-1.2.3.tgz') -const npa = require('npm-package-arg') const me = t.testdir({ repo: {}, @@ -71,20 +79,10 @@ const repo = resolve(me, 'repo') const cache = resolve(me, 'cache') const cycleA = resolve(me, 'cycle-a') const cycleB = resolve(me, 'cycle-b') +const opts = { cache, Arborist } const abbrevSpec = `file:${abbrev}` -const opts = { cache, Arborist: require('@npmcli/arborist') } - -const spawnGit = require('@npmcli/git').spawn -const { spawn } = require('child_process') -const spawnNpm = require('../lib/util/npm.js') - -const { mkdir } = require('fs/promises') -const { rmSync } = require('fs') - -const tar = require('tar') - let REPO_HEAD = '' t.test('setup', { bail: true }, t => { t.test('create repo', () => { diff --git a/test/index.js b/test/index.js index 1a9309d3..830e93f3 100644 --- a/test/index.js +++ b/test/index.js @@ -1,14 +1,13 @@ const t = require('tap') const pacote = require('../lib/index.js') const fs = require('fs') - const GitFetcher = require('../lib/git.js') const RegistryFetcher = require('../lib/registry.js') const FileFetcher = require('../lib/file.js') const DirFetcher = require('../lib/dir.js') const RemoteFetcher = require('../lib/remote.js') - const { resolve, relative } = require('path') + const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz') const abbrevspec = `file:${relative(process.cwd(), abbrev)}` diff --git a/test/registry.js b/test/registry.js index 95ea5bc7..7037f802 100644 --- a/test/registry.js +++ b/test/registry.js @@ -1,6 +1,11 @@ const t = require('tap') +const path = require('path') +const fs = require('fs') +const mr = require('npm-registry-mock') +const tnock = require('./fixtures/tnock') // Stub out sigstore verification for testing to avoid needing to refresh the tuf cache const RegistryFetcher = require('../lib/registry.js') + const MockedRegistryFetcher = t.mock('../lib/registry.js', { sigstore: { verify: async (bundle, options) => { @@ -14,10 +19,7 @@ const MockedRegistryFetcher = t.mock('../lib/registry.js', { }, }, }) -const path = require('path') -const fs = require('fs') -const mr = require('npm-registry-mock') -const tnock = require('./fixtures/tnock') + const port = 18000 + (+process.env.TAP_CHILD_ID || 0) const me = t.testdir() diff --git a/test/remote.js b/test/remote.js index 4281406f..87d2c865 100644 --- a/test/remote.js +++ b/test/remote.js @@ -2,17 +2,17 @@ const RemoteFetcher = require('../lib/remote.js') const http = require('http') const ssri = require('ssri') const t = require('tap') - const { resolve } = require('path') +const fs = require('fs') + const me = t.testdir() const cache = resolve(me, 'cache') - -t.cleanSnapshot = str => str.split('' + port).join('{PORT}') - -const fs = require('fs') const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz') const port = 12345 + (+process.env.TAP_CHILD_ID || 0) const server = `http://localhost:${port}` + +t.cleanSnapshot = str => str.split('' + port).join('{PORT}') + let abbrevIntegrity const requestLog = [] t.test('start server', t => {