Skip to content

Commit

Permalink
feat: pacote now optionally takes a tree when preparing directories
Browse files Browse the repository at this point in the history
If a tree is not passed in, then pacote will create one with
`loadActual` and the resolved path to the directory it is preparing.

BREAKING CHANGE: `pacote` now has a peer dependency on
`@npmcli/arborist`.
  • Loading branch information
lukekarrys committed Sep 27, 2022
1 parent c3289af commit d3517fd
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 66 deletions.
9 changes: 8 additions & 1 deletion lib/dir.js
Expand Up @@ -3,6 +3,7 @@ const FileFetcher = require('./file.js')
const Minipass = require('minipass')
const tarCreateOptions = require('./util/tar-create-options.js')
const packlist = require('npm-packlist')
const Arborist = require('@npmcli/arborist')
const tar = require('tar')
const _prepareDir = Symbol('_prepareDir')
const { resolve } = require('path')
Expand Down Expand Up @@ -68,7 +69,13 @@ 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]()
.then(() => packlist({ path: this.resolved, prefix, workspaces }))
.then(async () => {
if (!this.tree) {
const arb = new Arborist({ path: this.resolved })
this.tree = await arb.loadActual()
}
return packlist(this.tree, { path: this.resolved, prefix, workspaces })
})
.then(files => tar.c(tarCreateOptions(this.package), files)
.on('error', er => stream.emit('error', er)).pipe(stream))
.catch(er => stream.emit('error', er))
Expand Down
1 change: 1 addition & 0 deletions lib/fetcher.js
Expand Up @@ -72,6 +72,7 @@ class FetcherBase {

this.cache = opts.cache || cacheDir()
this.resolved = opts.resolved || null
this.tree = opts.tree || null

// default to caching/verifying with sha512, that's what we usually have
// need to change this default, or start overriding it, when sha512
Expand Down
6 changes: 5 additions & 1 deletion package.json
Expand Up @@ -25,6 +25,7 @@
]
},
"devDependencies": {
"@npmcli/arborist": "^6.0.0 || ^6.0.0-pre.0",
"@npmcli/eslint-config": "^3.1.0",
"@npmcli/template-oss": "4.4.2",
"hosted-git-info": "^5.0.0",
Expand Down Expand Up @@ -54,7 +55,7 @@
"minipass": "^3.1.6",
"mkdirp": "^1.0.4",
"npm-package-arg": "^9.0.0",
"npm-packlist": "^6.0.0",
"npm-packlist": "^7.0.0 || ^7.0.0-pre.0",
"npm-pick-manifest": "^7.0.0",
"npm-registry-fetch": "^13.0.1",
"proc-log": "^2.0.0",
Expand All @@ -65,6 +66,9 @@
"ssri": "^9.0.0",
"tar": "^6.1.11"
},
"peerDependencies": {
"@npmcli/arborist": "^6.0.0 || ^6.0.0-pre.0"
},
"engines": {
"node": "^14.17.0 || ^16.13.0 || >=18.0.0"
},
Expand Down
39 changes: 23 additions & 16 deletions test/dir.js
@@ -1,6 +1,13 @@
const runScript = require('@npmcli/run-script')
const RUNS = []
const t = require('tap')
const Arborist = require('@npmcli/arborist')

const loadActual = async (path) => {
const arb = new Arborist({ path })
const tree = await arb.loadActual()
return tree
}

const DirFetcher = t.mock('../lib/dir.js', {
'@npmcli/run-script': (opts) => {
Expand All @@ -20,8 +27,8 @@ t.cleanSnapshot = str => str.split(process.cwd()).join('${CWD}')
const abbrev = resolve(__dirname, 'fixtures/abbrev')
const abbrevspec = `file:${relative(process.cwd(), abbrev)}`

t.test('basic', t => {
const f = new DirFetcher(abbrevspec, {})
t.test('basic', async t => {
const f = new DirFetcher(abbrevspec, { tree: await loadActual(abbrev) })
t.same(f.types, ['directory'])
t.resolveMatchSnapshot(f.packument(), 'packument')
t.resolveMatchSnapshot(f.manifest(), 'manifest')
Expand All @@ -32,21 +39,21 @@ t.test('basic', t => {
.then(() => f.manifest().then(mani => t.equal(mani, f.package)))
})

t.test('dir with integrity', t => {
t.test('dir with integrity', async t => {
const f = new DirFetcher(abbrevspec, {
integrity: 'sha512-whatever-this-is-only-checked-if-we-extract-it',
tree: await loadActual(abbrev),
})
t.same(f.types, ['directory'])
t.resolveMatchSnapshot(f.packument(), 'packument')
return t.end()
return t.resolveMatchSnapshot(f.packument(), 'packument')
})

const prepare = resolve(__dirname, 'fixtures/prepare-script')
const preparespec = `file:${relative(process.cwd(), prepare)}`

t.test('with prepare script', t => {
t.test('with prepare script', async t => {
RUNS.length = 0
const f = new DirFetcher(preparespec, {})
const f = new DirFetcher(preparespec, { tree: await loadActual(prepare) })
t.resolveMatchSnapshot(f.packument(), 'packument')
t.resolveMatchSnapshot(f.manifest(), 'manifest')
const index = me + '/prepare/index.js'
Expand All @@ -59,9 +66,9 @@ t.test('with prepare script', t => {
}, 'should run in background'))
})

t.test('responds to foregroundScripts: true', t => {
t.test('responds to foregroundScripts: true', async t => {
RUNS.length = 0
const opt = { foregroundScripts: true }
const opt = { foregroundScripts: true, tree: await loadActual(prepare) }
const f = new DirFetcher(preparespec, opt)
t.resolveMatchSnapshot(f.packument(), 'packument')
t.resolveMatchSnapshot(f.manifest(), 'manifest')
Expand All @@ -75,9 +82,9 @@ t.test('responds to foregroundScripts: true', t => {
}, 'should run in foreground'))
})

t.test('responds to foregroundScripts: true and silent: true', t => {
t.test('responds to foregroundScripts: true and silent: true', async t => {
RUNS.length = 0
const opt = { foregroundScripts: true, silent: true }
const opt = { foregroundScripts: true, silent: true, tree: await loadActual(prepare) }
const f = new DirFetcher(preparespec, opt)
t.resolveMatchSnapshot(f.packument(), 'packument')
t.resolveMatchSnapshot(f.manifest(), 'manifest')
Expand All @@ -91,8 +98,8 @@ t.test('responds to foregroundScripts: true and silent: true', t => {
}, 'should run in foreground, but without banner'))
})

t.test('missing dir cannot be packed', t => {
const f = new DirFetcher('file:/this/dir/doesnt/exist', {})
t.test('missing dir cannot be packed', async t => {
const f = new DirFetcher('file:/this/dir/doesnt/exist', { tree: await loadActual() })
return t.rejects(f.extract(me + '/nope'), {
message: `no such file or directory, open '/this/dir/doesnt/exist/package.json`,
errno: Number,
Expand All @@ -102,19 +109,19 @@ t.test('missing dir cannot be packed', t => {
})
})

t.test('when read fails', t => {
t.test('when read fails', async t => {
const read = fs.read
t.teardown(() => fs.read = read)
const poop = new Error('poop')
fs.read = (...args) => setTimeout(() => args[args.length - 1](poop))
const f = new DirFetcher(preparespec, {})
const f = new DirFetcher(preparespec, { tree: await loadActual() })
return t.rejects(f.extract(me + '/nope'), poop)
})

t.test('make bins executable', async t => {
const file = resolve(__dirname, 'fixtures/bin-object')
const spec = `file:${relative(process.cwd(), file)}`
const f = new DirFetcher(spec, {})
const f = new DirFetcher(spec, { tree: await loadActual(file) })
const target = resolve(me, basename(file))
const res = await f.extract(target)
// node v13.8 swapped out their zlib implementation with chromium's
Expand Down
28 changes: 28 additions & 0 deletions test/fixtures/npm-mock.js
@@ -0,0 +1,28 @@
#!/usr/bin/env node
const { argv, env } = process

const pnp = env._PACOTE_NO_PREPARE_ || ''
const pacotePath = env._PACOTE_TEST_PATH_
const pacoteOpts = env._PACOTE_TEST_OPTS_

const data = {
argv,
noPrepare: pnp ? pnp.split('\\n') : [],
cwd: process.cwd(),
}

if (data.noPrepare.length > 5) {
throw new Error('infinite regress detected!')
}

// just an incredibly rudimentary package manager
const pkg = require(process.cwd() + '/package.json')
const pacote = require(pacotePath)
for (const [name, spec] of Object.entries(pkg.dependencies)) {
pacote.extract(spec, process.cwd() + '/' + name, {
npmBin: __filename,
...JSON.parse(pacoteOpts),
})
}

require('fs').writeFileSync('log', JSON.stringify(data, 0, 2))

0 comments on commit d3517fd

Please sign in to comment.