Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clean up requires #371

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions lib/dir.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const { Minipass } = require('minipass')
const tarCreateOptions = require('./util/tar-create-options.js')
const { resolve } = require('node:path')
const packlist = require('npm-packlist')
const tar = require('tar')
const { resolve } = require('path')
const runScript = require('@npmcli/run-script')
const tar = require('tar')
const { Minipass } = require('minipass')
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const _ = require('./util/protected.js')
const tarCreateOptions = require('./util/tar-create-options.js')

class DirFetcher extends Fetcher {
constructor (spec, opts) {
Expand All @@ -27,7 +27,7 @@ class DirFetcher extends Fetcher {
return ['directory']
}

[_.prepareDir] () {
#prepareDir () {
return this.manifest().then(mani => {
if (!mani.scripts || !mani.scripts.prepare) {
return
Expand Down Expand Up @@ -65,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 })
Expand Down
26 changes: 13 additions & 13 deletions lib/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
// It handles the unpacking and retry logic that is shared among
// all of the other Fetcher types.

const { basename, dirname } = require('node:path')
const { rm, mkdir } = require('node:fs/promises')
const PackageJson = require('@npmcli/package-json')
const cacache = require('cacache')
const fsm = require('fs-minipass')
const getContents = require('@npmcli/installed-package-contents')
const npa = require('npm-package-arg')
const retry = require('promise-retry')
const ssri = require('ssri')
const { basename, dirname } = require('path')
const tar = require('tar')
const { Minipass } = require('minipass')
const { log } = require('proc-log')
const retry = require('promise-retry')
const fs = require('fs/promises')
const fsm = require('fs-minipass')
const cacache = require('cacache')
const _ = require('./util/protected.js')
const cacheDir = require('./util/cache-dir.js')
const isPackageBin = require('./util/is-package-bin.js')
const removeTrailingSlashes = require('./util/trailing-slashes.js')
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)
Expand Down Expand Up @@ -337,12 +337,12 @@ class FetcherBase {

#empty (path) {
return getContents({ path, depth: 1 }).then(contents => Promise.all(
contents.map(entry => fs.rm(entry, { recursive: true, force: true }))))
contents.map(entry => rm(entry, { recursive: true, force: true }))))
}

async #mkdir (dest) {
await this.#empty(dest)
return await fs.mkdir(dest, { recursive: true })
return await mkdir(dest, { recursive: true })
}

// extraction is always the same. the only difference is where
Expand All @@ -369,7 +369,7 @@ class FetcherBase {
// 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 })
await mkdir(dir, { recursive: true })
return this.#toFile(dest)
}

Expand Down
6 changes: 3 additions & 3 deletions lib/file.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const fsm = require('fs-minipass')
const { resolve } = require('node:path')
const { stat, chmod } = require('node:fs/promises')
const cacache = require('cacache')
const { resolve } = require('path')
const { stat, chmod } = require('fs/promises')
const fsm = require('fs-minipass')
const Fetcher = require('./fetcher.js')
const _ = require('./util/protected.js')

Expand Down
16 changes: 8 additions & 8 deletions lib/git.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const RemoteFetcher = require('./remote.js')
const DirFetcher = require('./dir.js')
const cacache = require('cacache')
const git = require('@npmcli/git')
const pickManifest = require('npm-pick-manifest')
const npa = require('npm-package-arg')
const pickManifest = require('npm-pick-manifest')
const { Minipass } = require('minipass')
const cacache = require('cacache')
const { log } = require('proc-log')
const npm = require('./util/npm.js')
const addGitSha = require('./util/add-git-sha.js')
const DirFetcher = require('./dir.js')
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const RemoteFetcher = require('./remote.js')
const _ = require('./util/protected.js')
const addGitSha = require('./util/add-git-sha.js')
const npm = require('./util/npm.js')

const hashre = /^[a-f0-9]{40}$/

Expand Down
10 changes: 5 additions & 5 deletions lib/registry.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
const Fetcher = require('./fetcher.js')
const RemoteFetcher = require('./remote.js')
const pacoteVersion = require('../package.json').version
const removeTrailingSlashes = require('./util/trailing-slashes.js')
const crypto = require('node:crypto')
const PackageJson = require('@npmcli/package-json')
const pickManifest = require('npm-pick-manifest')
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 Fetcher = require('./fetcher.js')
const RemoteFetcher = require('./remote.js')
const pacoteVersion = require('../package.json').version
const removeTrailingSlashes = require('./util/trailing-slashes.js')
const _ = require('./util/protected.js')

// Corgis are cute. 🐕🐶
Expand Down
6 changes: 3 additions & 3 deletions lib/remote.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const pacoteVersion = require('../package.json').version
const fetch = require('npm-registry-fetch')
const { Minipass } = require('minipass')
const Fetcher = require('./fetcher.js')
const FileFetcher = require('./file.js')
const _ = require('./util/protected.js')
const pacoteVersion = require('../package.json').version

class RemoteFetcher extends Fetcher {
constructor (spec, opts) {
Expand Down
8 changes: 4 additions & 4 deletions lib/util/cache-dir.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
const os = require('os')
const { resolve } = require('path')
const { resolve } = require('node:path')
const { tmpdir, homedir } = require('node:os')

module.exports = (fakePlatform = false) => {
const temp = os.tmpdir()
const temp = tmpdir()
const uidOrPid = process.getuid ? process.getuid() : process.pid
const home = os.homedir() || resolve(temp, 'npm-' + uidOrPid)
const home = homedir() || resolve(temp, 'npm-' + uidOrPid)
const platform = fakePlatform || process.platform
const cacheExtra = platform === 'win32' ? 'npm-cache' : '.npm'
const cacheRoot = (platform === 'win32' && process.env.LOCALAPPDATA) || home
Expand Down
12 changes: 3 additions & 9 deletions lib/util/protected.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
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,
cacheFetches: Symbol.for('pacote.Fetcher._cacheFetches'),
readPackageJson: Symbol.for('package.Fetcher._readPackageJson'),
tarballFromResolved: Symbol.for('pacote.Fetcher._tarballFromResolved'),
}
8 changes: 4 additions & 4 deletions test/bin.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
const t = require('tap')
const { spawn } = require('node:child_process')
const { Minipass } = require('minipass')
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 pacote = require('../')
const { Minipass } = require('minipass')

t.cleanSnapshot = str =>
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')
Expand Down
4 changes: 2 additions & 2 deletions test/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +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 fs = require('node:fs')
const { relative, resolve, basename } = require('node:path')

const loadActual = async (path) => {
const arb = new Arborist({ path })
Expand Down
11 changes: 6 additions & 5 deletions test/fetcher.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
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 fs = require('node:fs')
const { relative, resolve, basename } = require('node:path')
const cacache = require('cacache')
const { Minipass } = require('minipass')
const npa = require('npm-package-arg')
// FUN FACT if we require FileFetcher first the code can't run cause of a circular dependency
const Fetcher = require('../lib/fetcher.js')
// 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 abbrevMani = require('./fixtures/abbrev-manifest-min.json')

const byDigest = cacache.get.stream.byDigest

Expand Down
6 changes: 3 additions & 3 deletions test/file.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const FileFetcher = require('../lib/file.js')
const t = require('tap')
const { relative, resolve, basename } = require('path')
const fs = require('fs')
const { relative, resolve, basename } = require('node:path')
const fs = require('node:fs')
const FileFetcher = require('../lib/file.js')

t.cleanSnapshot = str => str.split(process.cwd()).join('${CWD}')

Expand Down
19 changes: 9 additions & 10 deletions test/git.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
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 fs = require('node:fs')
const http = require('node:http')
const { dirname, basename, resolve } = require('node:path')
const { mkdir } = require('node:fs/promises')
const { rmSync } = require('node:fs')
const { spawn } = require('node:child_process')
const Arborist = require('@npmcli/arborist')
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')
const spawnNpm = require('../lib/util/npm.js')
const GitFetcher = require('../lib/git.js')

// 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)
Expand Down
10 changes: 5 additions & 5 deletions test/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
const t = require('tap')
const pacote = require('../lib/index.js')
const fs = require('fs')
const fs = require('node:fs')
const { resolve, relative } = require('node:path')
const DirFetcher = require('../lib/dir.js')
const FileFetcher = require('../lib/file.js')
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 pacote = require('../lib/index.js')

const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz')
const abbrevspec = `file:${relative(process.cwd(), abbrev)}`
Expand Down
6 changes: 3 additions & 3 deletions test/registry.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const t = require('tap')
const path = require('path')
const fs = require('fs')
const path = require('node:path')
const fs = require('node: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
Expand Down Expand Up @@ -1236,7 +1236,7 @@ t.test('corgi packument is not cached as full packument', async t => {
cache,
fullMetadata: true,
})
t.notEqual(await f.packument(), packument, 'did not get cached packument')
t.not(await f.packument(), packument, 'did not get cached packument')
t.ok(packumentCache.has(`full:${packumentUrl}`), 'full packument is also now cached')
})

Expand Down
10 changes: 5 additions & 5 deletions test/remote.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
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 fs = require('node:fs')
const http = require('node:http')
const { resolve } = require('node:path')
const ssri = require('ssri')
const RemoteFetcher = require('../lib/remote.js')

const me = t.testdir()
const cache = resolve(me, 'cache')
Expand Down
2 changes: 1 addition & 1 deletion test/util/add-git-sha.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const t = require('tap')
const addGitSha = require('../../lib/util/add-git-sha.js')
const npa = require('npm-package-arg')
const addGitSha = require('../../lib/util/add-git-sha.js')

const cases = [
// unknown host
Expand Down
20 changes: 12 additions & 8 deletions test/util/cache-dir.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
const t = require('tap')
const os = require('os')

os.tmpdir = () => '/tmp'
os.homedir = () => '/home/isaacs'
const path = require('path')
path.resolve = path.posix.resolve
const path = require('node:path')
process.getuid = () => 69420
process.env.LOCALAPPDATA = ''
const isWindows = process.platform === 'win32'
const posix = isWindows ? 'posix' : null
const windows = isWindows ? null : 'win32'

const cacheDir = require('../../lib/util/cache-dir.js')
let homedir = '/home/isaacs'
const cacheDir = t.mock('../../lib/util/cache-dir.js', {
'node:path': {
resolve: path.posix.resolve,
},
'node:os': {
tmpdir: () => '/tmp',
homedir: () => homedir,
},
})

// call it once just to cover the default arg setting
// the tests all specify something, so they work predictably
Expand All @@ -23,7 +27,7 @@ t.equal(cacheDir(windows).cacache, '/home/isaacs/npm-cache/_cacache')
t.equal(cacheDir(posix).tufcache, '/home/isaacs/.npm/_tuf')
t.equal(cacheDir(windows).tufcache, '/home/isaacs/npm-cache/_tuf')

os.homedir = () => null
homedir = null
t.equal(cacheDir(posix).cacache, '/tmp/npm-69420/.npm/_cacache')
t.equal(cacheDir(windows).cacache, '/tmp/npm-69420/npm-cache/_cacache')
t.equal(cacheDir(posix).tufcache, '/tmp/npm-69420/.npm/_tuf')
Expand Down
2 changes: 1 addition & 1 deletion test/util/is-package-bin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const isPackageBin = require('../../lib/util/is-package-bin.js')
const t = require('tap')
const isPackageBin = require('../../lib/util/is-package-bin.js')

t.ok(isPackageBin({ bin: 'foo' }, 'package/foo'), 'finds string')
t.ok(isPackageBin({ bin: { bar: 'foo' } }, 'package/foo'), 'finds in obj')
Expand Down
Loading
Loading