refactor: split package fetcher (#5175)

This commit is contained in:
Dominic Elm
2022-08-08 20:44:50 +02:00
committed by GitHub
parent 8cb47ac9d2
commit 7a17f99aba
20 changed files with 327 additions and 128 deletions

View File

@@ -0,0 +1,8 @@
---
"@pnpm/package-requester": major
"@pnpm/pick-fetcher": major
"@pnpm/tarball-fetcher": major
"@pnpm/node.fetcher": patch
---
Refactor `tarball-fetcher` and separate it into more specific fetchers, such as `localTarball`, `remoteTarball` and `gitHostedTarball`.

View File

@@ -39,6 +39,7 @@
"@pnpm/error": "workspace:*",
"@pnpm/fetcher-base": "workspace:*",
"@pnpm/fetching-types": "workspace:*",
"@pnpm/pick-fetcher": "workspace:*",
"@pnpm/tarball-fetcher": "workspace:*",
"adm-zip": "^0.5.9",
"detect-libc": "^2.0.1",

View File

@@ -6,6 +6,7 @@ import {
RetryTimeoutOptions,
} from '@pnpm/fetching-types'
import { FilesIndex } from '@pnpm/fetcher-base'
import { pickFetcher } from '@pnpm/pick-fetcher'
import createCafsStore from '@pnpm/create-cafs-store'
import createFetcher, { waitForFilesIndex } from '@pnpm/tarball-fetcher'
import AdmZip from 'adm-zip'
@@ -32,11 +33,12 @@ export async function fetchNode (fetch: FetchFromRegistry, version: string, targ
return
}
const getCredentials = () => ({ authHeaderValue: undefined, alwaysAuth: undefined })
const { tarball: fetchTarball } = createFetcher(fetch, getCredentials, {
const fetchers = createFetcher(fetch, getCredentials, {
retry: opts.retry,
timeout: opts.fetchTimeout,
})
const cafs = createCafsStore(opts.cafsDir)
const fetchTarball = pickFetcher(fetchers, { tarball })
const { filesIndex } = await fetchTarball(cafs, { tarball }, {
lockfileDir: process.cwd(),
})

View File

@@ -24,6 +24,9 @@
{
"path": "../fetching-types"
},
{
"path": "../pick-fetcher"
},
{
"path": "../tarball-fetcher"
}

View File

@@ -44,6 +44,7 @@
"@pnpm/fetcher-base": "workspace:*",
"@pnpm/graceful-fs": "workspace:*",
"@pnpm/package-is-installable": "workspace:*",
"@pnpm/pick-fetcher": "workspace:*",
"@pnpm/read-package-json": "workspace:*",
"@pnpm/resolver-base": "workspace:*",
"@pnpm/store-controller-types": "workspace:*",

View File

@@ -10,6 +10,7 @@ import {
PackageFilesIndex,
} from '@pnpm/cafs'
import { fetchingProgressLogger, progressLogger } from '@pnpm/core-loggers'
import { pickFetcher } from '@pnpm/pick-fetcher'
import PnpmError from '@pnpm/error'
import {
Cafs,
@@ -662,10 +663,7 @@ async function fetcher (
resolution: Resolution,
opts: FetchOptions
): Promise<FetchResult> {
const fetch = fetcherByHostingType[resolution.type ?? 'tarball']
if (!fetch) {
throw new Error(`Fetching for dependency type "${resolution.type ?? 'undefined'}" is not supported`)
}
const fetch = pickFetcher(fetcherByHostingType, resolution)
try {
return await fetch(cafs, resolution, opts)
} catch (err: any) { // eslint-disable-line

View File

@@ -39,6 +39,9 @@
{
"path": "../package-is-installable"
},
{
"path": "../pick-fetcher"
},
{
"path": "../read-package-json"
},

View File

@@ -0,0 +1,15 @@
# @pnpm/pick-fetcher
> Pick a package fetcher by type
[![npm version](https://img.shields.io/npm/v/@pnpm/pick-fetcher.svg)](https://www.npmjs.com/package/@pnpm/pick-fetcher)
## Deploy
```sh
pnpm add @pnpm/pick-fetcher
```
## License
MIT

View File

@@ -0,0 +1 @@
module.exports = require('../../jest.config.js')

View File

@@ -0,0 +1,39 @@
{
"name": "@pnpm/pick-fetcher",
"version": "0.0.0",
"description": "Pick a package fetcher by type",
"main": "lib/index.js",
"types": "lib/index.d.ts",
"files": [
"lib",
"!*.map"
],
"scripts": {
"lint": "eslint src/**/*.ts test/**/*.ts",
"_test": "jest",
"test": "pnpm run compile && pnpm run _test",
"prepublishOnly": "pnpm run compile",
"compile": "tsc --build && pnpm run lint --fix"
},
"repository": "https://github.com/pnpm/pnpm/blob/main/packages/pick-fetcher",
"license": "MIT",
"engines": {
"node": ">=14.6"
},
"bugs": {
"url": "https://github.com/pnpm/pnpm/issues"
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/packages/pick-fetcher#readme",
"funding": "https://opencollective.com/pnpm",
"keywords": [
"pnpm7"
],
"devDependencies": {
"@pnpm/fetcher-base": "workspace:*",
"@pnpm/pick-fetcher": "workspace:*",
"@pnpm/resolver-base": "workspace:*"
},
"exports": {
".": "./lib/index.js"
}
}

View File

@@ -0,0 +1,32 @@
import type { Resolution } from '@pnpm/resolver-base'
import type { FetchFunction } from '@pnpm/fetcher-base'
export function pickFetcher (fetcherByHostingType: {[hostingType: string]: FetchFunction}, resolution: Resolution) {
let fetcherType = resolution.type
if (resolution.type == null) {
if (resolution.tarball.startsWith('file:')) {
fetcherType = 'localTarball'
} else if (isGitHostedPkgUrl(resolution.tarball)) {
fetcherType = 'gitHostedTarball'
} else {
fetcherType = 'remoteTarball'
}
}
const fetch = fetcherByHostingType[fetcherType!]
if (!fetch) {
throw new Error(`Fetching for dependency type "${resolution.type ?? 'undefined'}" is not supported`)
}
return fetch
}
function isGitHostedPkgUrl (url: string) {
return (
url.startsWith('https://codeload.github.com/') ||
url.startsWith('https://bitbucket.org/') ||
url.startsWith('https://gitlab.com/')
) && url.includes('tar.gz')
}

View File

@@ -0,0 +1,29 @@
import { pickFetcher } from '@pnpm/pick-fetcher'
test('should pick localTarball fetcher', () => {
const localTarball = jest.fn()
const fetcher = pickFetcher({ localTarball }, { tarball: 'file:is-positive-1.0.0.tgz' })
expect(fetcher).toBe(localTarball)
})
test('should pick remoteTarball fetcher', () => {
const remoteTarball = jest.fn()
const fetcher = pickFetcher({ remoteTarball }, { tarball: 'is-positive-1.0.0.tgz' })
expect(fetcher).toBe(remoteTarball)
})
test.each([
'https://codeload.github.com/zkochan/is-negative/tar.gz/2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5',
'https://bitbucket.org/pnpmjs/git-resolver/get/87cf6a67064d2ce56e8cd20624769a5512b83ff9.tar.gz',
'https://gitlab.com/api/v4/projects/pnpm%2Fgit-resolver/repository/archive.tar.gz',
])('should pick gitHostedTarball fetcher', (tarball) => {
const gitHostedTarball = jest.fn()
const fetcher = pickFetcher({ gitHostedTarball }, { tarball })
expect(fetcher).toBe(gitHostedTarball)
})
test('should fail to pick fetcher if the type is not defined', () => {
expect(() => {
pickFetcher({}, { type: 'directory' })
}).toThrow('Fetching for dependency type "directory" is not supported')
})

View File

@@ -0,0 +1,19 @@
{
"extends": "@pnpm/tsconfig",
"compilerOptions": {
"outDir": "lib",
"rootDir": "src"
},
"include": [
"src/**/*.ts",
"../../typings/**/*.d.ts"
],
"references": [
{
"path": "../fetcher-base"
},
{
"path": "../resolver-base"
}
]
}

View File

@@ -0,0 +1,8 @@
{
"extends": "./tsconfig.json",
"include": [
"src/**/*.ts",
"test/**/*.ts",
"../../typings/**/*.d.ts"
]
}

View File

@@ -0,0 +1,55 @@
import { Cafs, FetchFunction, FetchOptions, FilesIndex, PackageFileInfo } from '@pnpm/fetcher-base'
import preparePackage from '@pnpm/prepare-package'
import fromPairs from 'ramda/src/fromPairs'
import omit from 'ramda/src/omit'
interface Resolution {
integrity?: string
registry?: string
tarball: string
}
export default function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction): FetchFunction {
const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
const { filesIndex } = await fetchRemoteTarball(cafs, resolution, opts)
return { filesIndex: await prepareGitHostedPkg(filesIndex as FilesIndex, cafs) }
}
return fetch as FetchFunction
}
async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs) {
const tempLocation = await cafs.tempDir()
await cafs.importPackage(tempLocation, {
filesResponse: {
filesIndex: await waitForFilesIndex(filesIndex),
fromStore: false,
},
force: true,
})
await preparePackage(tempLocation)
const newFilesIndex = await cafs.addFilesFromDir(tempLocation)
// Important! We cannot remove the temp location at this stage.
// Even though we have the index of the package,
// the linking of files to the store is in progress.
return newFilesIndex
}
export async function waitForFilesIndex (filesIndex: FilesIndex): Promise<Record<string, PackageFileInfo>> {
return fromPairs(
await Promise.all(
Object.entries(filesIndex).map(async ([fileName, fileInfo]): Promise<[string, PackageFileInfo]> => {
const { integrity, checkedAt } = await fileInfo.writeResult
return [
fileName,
{
...omit(['writeResult'], fileInfo),
checkedAt,
integrity: integrity.toString(),
},
]
})
)
)
}

View File

@@ -1,24 +1,20 @@
import path from 'path'
import PnpmError from '@pnpm/error'
import {
Cafs,
DeferredManifestPromise,
FetchFunction,
FetchOptions,
FetchResult,
} from '@pnpm/fetcher-base'
import {
FetchFromRegistry,
GetCredentials,
RetryTimeoutOptions,
} from '@pnpm/fetching-types'
import gfs from '@pnpm/graceful-fs'
import ssri from 'ssri'
import createDownloader, {
DownloadFunction,
TarballIntegrityError,
waitForFilesIndex,
} from './createDownloader'
} from './remoteTarballFetcher'
import createLocalTarballFetcher from './localTarballFetcher'
import createGitHostedTarballFetcher, { waitForFilesIndex } from './gitHostedTarballFetcher'
export { BadTarballError } from './errorTypes'
@@ -32,17 +28,22 @@ export default function (
retry?: RetryTimeoutOptions
offline?: boolean
}
): { tarball: FetchFunction } {
): { localTarball: FetchFunction, remoteTarball: FetchFunction, gitHostedTarball: FetchFunction } {
const download = createDownloader(fetchFromRegistry, {
retry: opts.retry,
timeout: opts.timeout,
})
const remoteTarballFetcher = fetchFromTarball.bind(null, {
download,
getCredentialsByURI: getCredentials,
offline: opts.offline,
}) as FetchFunction
return {
tarball: fetchFromTarball.bind(null, {
download,
getCredentialsByURI: getCredentials,
offline: opts.offline,
}) as FetchFunction,
localTarball: createLocalTarballFetcher(),
remoteTarball: remoteTarballFetcher,
gitHostedTarball: createGitHostedTarballFetcher(remoteTarballFetcher),
}
}
@@ -63,13 +64,6 @@ async function fetchFromTarball (
},
opts: FetchOptions
) {
if (resolution.tarball.startsWith('file:')) {
const tarball = resolvePath(opts.lockfileDir, resolution.tarball.slice(5))
return fetchFromLocalTarball(cafs, tarball, {
integrity: resolution.integrity,
manifest: opts.manifest,
})
}
if (ctx.offline) {
throw new PnpmError('NO_OFFLINE_TARBALL',
`A package is missing from the store but cannot download it in offline mode. The missing package may be downloaded from ${resolution.tarball}.`)
@@ -85,41 +79,3 @@ async function fetchFromTarball (
registry: resolution.registry,
})
}
const isAbsolutePath = /^[/]|^[A-Za-z]:/
function resolvePath (where: string, spec: string) {
if (isAbsolutePath.test(spec)) return spec
return path.resolve(where, spec)
}
async function fetchFromLocalTarball (
cafs: Cafs,
tarball: string,
opts: {
integrity?: string
manifest?: DeferredManifestPromise
}
): Promise<FetchResult> {
try {
const tarballStream = gfs.createReadStream(tarball)
const [fetchResult] = (
await Promise.all([
cafs.addFilesFromTarball(tarballStream, opts.manifest),
opts.integrity && (ssri.checkStream(tarballStream, opts.integrity) as any), // eslint-disable-line
])
)
return { filesIndex: fetchResult }
} catch (err: any) { // eslint-disable-line
const error = new TarballIntegrityError({
attempts: 1,
algorithm: err['algorithm'],
expected: err['expected'],
found: err['found'],
sri: err['sri'],
url: tarball,
})
error['resource'] = tarball
throw error
}
}

View File

@@ -0,0 +1,62 @@
import path from 'path'
import { Cafs, DeferredManifestPromise, FetchFunction, FetchOptions, FetchResult } from '@pnpm/fetcher-base'
import gfs from '@pnpm/graceful-fs'
import ssri from 'ssri'
import { TarballIntegrityError } from './remoteTarballFetcher'
const isAbsolutePath = /^[/]|^[A-Za-z]:/
interface Resolution {
integrity?: string
registry?: string
tarball: string
}
export default function createLocalTarballFetcher (): FetchFunction {
const fetch = (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
const tarball = resolvePath(opts.lockfileDir, resolution.tarball.slice(5))
return fetchFromLocalTarball(cafs, tarball, {
integrity: resolution.integrity,
manifest: opts.manifest,
})
}
return fetch as FetchFunction
}
function resolvePath (where: string, spec: string) {
if (isAbsolutePath.test(spec)) return spec
return path.resolve(where, spec)
}
async function fetchFromLocalTarball (
cafs: Cafs,
tarball: string,
opts: {
integrity?: string
manifest?: DeferredManifestPromise
}
): Promise<FetchResult> {
try {
const tarballStream = gfs.createReadStream(tarball)
const [fetchResult] = (
await Promise.all([
cafs.addFilesFromTarball(tarballStream, opts.manifest),
opts.integrity && (ssri.checkStream(tarballStream, opts.integrity) as any), // eslint-disable-line
])
)
return { filesIndex: fetchResult }
} catch (err: any) { // eslint-disable-line
const error = new TarballIntegrityError({
attempts: 1,
algorithm: err['algorithm'],
expected: err['expected'],
found: err['found'],
sri: err['sri'],
url: tarball,
})
error['resource'] = tarball
throw error
}
}

View File

@@ -6,14 +6,9 @@ import {
Cafs,
DeferredManifestPromise,
FetchResult,
FilesIndex,
PackageFileInfo,
} from '@pnpm/fetcher-base'
import { FetchFromRegistry } from '@pnpm/fetching-types'
import preparePackage from '@pnpm/prepare-package'
import * as retry from '@zkochan/retry'
import fromPairs from 'ramda/src/fromPairs'
import omit from 'ramda/src/omit'
import ssri from 'ssri'
import { BadTarballError } from './errorTypes'
@@ -192,11 +187,8 @@ export default (
// eslint-disable-next-line
throw integrityCheckResult
}
if (!isGitHostedPkgUrl(url)) {
resolve({ filesIndex })
return
}
resolve({ filesIndex: await prepareGitHostedPkg(filesIndex, opts.cafs) })
resolve({ filesIndex })
} catch (err: any) { // eslint-disable-line
reject(err)
}
@@ -210,49 +202,6 @@ export default (
}
}
function isGitHostedPkgUrl (url: string) {
return (
url.startsWith('https://codeload.github.com/') ||
url.startsWith('https://bitbucket.org/') ||
url.startsWith('https://gitlab.com/')
) && url.includes('tar.gz')
}
export async function waitForFilesIndex (filesIndex: FilesIndex): Promise<Record<string, PackageFileInfo>> {
return fromPairs(
await Promise.all(
Object.entries(filesIndex).map(async ([fileName, fileInfo]): Promise<[string, PackageFileInfo]> => {
const { integrity, checkedAt } = await fileInfo.writeResult
return [
fileName,
{
...omit(['writeResult'], fileInfo),
checkedAt,
integrity: integrity.toString(),
},
]
})
)
)
}
async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs) {
const tempLocation = await cafs.tempDir()
await cafs.importPackage(tempLocation, {
filesResponse: {
filesIndex: await waitForFilesIndex(filesIndex),
fromStore: false,
},
force: true,
})
await preparePackage(tempLocation)
const newFilesIndex = await cafs.addFilesFromDir(tempLocation)
// Important! We cannot remove the temp location at this stage.
// Even though we have the index of the package,
// the linking of files to the store is in progress.
return newFilesIndex
}
async function safeCheckStream (stream: any, integrity: string, url: string): Promise<true | Error> { // eslint-disable-line @typescript-eslint/no-explicit-any
try {
await ssri.checkStream(stream, integrity)

View File

@@ -51,7 +51,7 @@ test('fail when tarball size does not match content-length', async () => {
}
await expect(
fetch.tarball(cafs, resolution, {
fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
).rejects.toThrow(
@@ -81,7 +81,7 @@ test('retry when tarball size does not match content-length', async () => {
const resolution = { tarball: 'http://example.com/foo.tgz' }
const result = await fetch.tarball(cafs, resolution, {
const result = await fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
@@ -105,7 +105,7 @@ test('fail when integrity check fails two times in a row', async () => {
}
await expect(
fetch.tarball(cafs, resolution, {
fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
).rejects.toThrow(
@@ -139,7 +139,7 @@ test('retry when integrity check fails', async () => {
}
const params: Array<[number | null, number]> = []
await fetch.tarball(cafs, resolution, {
await fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
onStart (size, attempts) {
params.push([size, attempts])
@@ -163,7 +163,7 @@ test('fail when integrity check of local file fails', async () => {
}
await expect(
fetch.tarball(cafs, resolution, {
fetch.localTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
).rejects.toThrow(
@@ -187,7 +187,7 @@ test("don't fail when integrity check of local file succeeds", async () => {
tarball: 'file:tar.tgz',
}
const { filesIndex } = await fetch.tarball(cafs, resolution, {
const { filesIndex } = await fetch.localTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
@@ -211,7 +211,7 @@ test("don't fail when fetching a local tarball in offline mode", async () => {
retries: 1,
},
})
const { filesIndex } = await fetch.tarball(cafs, resolution, {
const { filesIndex } = await fetch.localTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
@@ -236,7 +236,7 @@ test('fail when trying to fetch a non-local tarball in offline mode', async () =
},
})
await expect(
fetch.tarball(cafs, resolution, {
fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
).rejects.toThrow(
@@ -262,7 +262,7 @@ test('retry on server error', async () => {
tarball: 'http://example.com/foo.tgz',
}
const index = await fetch.tarball(cafs, resolution, {
const index = await fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
@@ -284,7 +284,7 @@ test('throw error when accessing private package w/o authorization', async () =>
}
await expect(
fetch.tarball(cafs, resolution, {
fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
).rejects.toThrow(
@@ -336,7 +336,7 @@ test('accessing private packages', async () => {
tarball: 'http://example.com/foo.tgz',
}
const index = await fetch.tarball(cafs, resolution, {
const index = await fetch.remoteTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})
@@ -355,7 +355,7 @@ test('fetch a big repository', async () => {
const resolution = { tarball: 'https://codeload.github.com/sveltejs/action-deploy-docs/tar.gz/a65fbf5a90f53c9d72fed4daaca59da50f074355' }
const result = await fetch.tarball(cafs, resolution, {
const result = await fetch.gitHostedTarball(cafs, resolution, {
lockfileDir: process.cwd(),
})

18
pnpm-lock.yaml generated
View File

@@ -2350,6 +2350,9 @@ importers:
'@pnpm/fetching-types':
specifier: workspace:*
version: link:../fetching-types
'@pnpm/pick-fetcher':
specifier: workspace:*
version: link:../pick-fetcher
'@pnpm/tarball-fetcher':
specifier: workspace:*
version: link:../tarball-fetcher
@@ -2642,6 +2645,9 @@ importers:
'@pnpm/package-is-installable':
specifier: workspace:*
version: link:../package-is-installable
'@pnpm/pick-fetcher':
specifier: workspace:*
version: link:../pick-fetcher
'@pnpm/read-package-json':
specifier: workspace:*
version: link:../read-package-json
@@ -2843,6 +2849,18 @@ importers:
specifier: ^4.0.0
version: 4.0.0
packages/pick-fetcher:
devDependencies:
'@pnpm/fetcher-base':
specifier: workspace:*
version: link:../fetcher-base
'@pnpm/pick-fetcher':
specifier: workspace:*
version: 'link:'
'@pnpm/resolver-base':
specifier: workspace:*
version: link:../resolver-base
packages/pick-registry-for-package:
dependencies:
'@pnpm/types':