From 70b2830accacdef130112af258200ad705b57bae Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 3 Sep 2023 02:30:08 +0300 Subject: [PATCH] refactor: worker (#7033) --- .changeset/plenty-deers-talk.md | 7 ++++ env/node.fetcher/src/index.ts | 1 + fetching/fetcher-base/src/index.ts | 17 ++++++-- fetching/git-fetcher/package.json | 1 - fetching/git-fetcher/src/index.ts | 14 +++---- fetching/git-fetcher/test/index.ts | 28 +++---------- fetching/tarball-fetcher/package.json | 1 - .../src/gitHostedTarballFetcher.ts | 26 ++++++++----- fetching/tarball-fetcher/src/index.ts | 3 +- .../src/localTarballFetcher.ts | 22 +++++------ .../src/remoteTarballFetcher.ts | 31 +++++++-------- fetching/tarball-fetcher/test/fetch.ts | 25 +++++++++--- pkg-manager/package-requester/package.json | 1 - .../package-requester/src/packageRequester.ts | 39 +++++++------------ pnpm-lock.yaml | 12 ++---- store/cafs/src/addFilesFromDir.ts | 10 +++-- store/cafs/src/addFilesFromTarball.ts | 5 ++- .../src/storeController/index.ts | 1 + worker/src/index.ts | 35 +++++++---------- worker/src/types.ts | 9 +++++ worker/src/worker.ts | 12 +++--- 21 files changed, 153 insertions(+), 147 deletions(-) create mode 100644 .changeset/plenty-deers-talk.md diff --git a/.changeset/plenty-deers-talk.md b/.changeset/plenty-deers-talk.md new file mode 100644 index 0000000000..97d4802fe9 --- /dev/null +++ b/.changeset/plenty-deers-talk.md @@ -0,0 +1,7 @@ +--- +"@pnpm/tarball-fetcher": major +"@pnpm/fetcher-base": major +"@pnpm/git-fetcher": major +--- + +Breaking changes to the API. diff --git a/env/node.fetcher/src/index.ts b/env/node.fetcher/src/index.ts index c6a61f3063..43cd938744 100644 --- a/env/node.fetcher/src/index.ts +++ b/env/node.fetcher/src/index.ts @@ -44,6 +44,7 @@ export async function fetchNode (fetch: FetchFromRegistry, version: string, targ const { filesIndex } = await fetchTarball(cafs, { tarball } as any, { // eslint-disable-line @typescript-eslint/no-explicit-any filesIndexFile: path.join(opts.cafsDir, encodeURIComponent(tarball)), // TODO: change the name or don't save an index file for node.js tarballs lockfileDir: process.cwd(), + pkg: {}, }) cafs.importPackage(targetDir, { filesResponse: { diff --git a/fetching/fetcher-base/src/index.ts b/fetching/fetcher-base/src/index.ts index 2c5c888c0e..f816811e7a 100644 --- a/fetching/fetcher-base/src/index.ts +++ b/fetching/fetcher-base/src/index.ts @@ -1,12 +1,19 @@ import { type Resolution, type GitResolution, type DirectoryResolution } from '@pnpm/resolver-base' -import type { DeferredManifestPromise, Cafs } from '@pnpm/cafs-types' +import { type DeferredManifestPromise, type Cafs } from '@pnpm/cafs-types' +import { type DependencyManifest } from '@pnpm/types' + +export interface PkgNameVersion { + name?: string + version?: string +} export interface FetchOptions { filesIndexFile: string - manifest?: DeferredManifestPromise lockfileDir: string onStart?: (totalSize: number | null, attempt: number) => void onProgress?: (downloaded: number) => void + readManifest?: boolean + pkg: PkgNameVersion } export type FetchFunction = ( @@ -17,15 +24,17 @@ export type FetchFunction } export interface GitFetcherOptions { - manifest?: DeferredManifestPromise + readManifest?: boolean filesIndexFile: string + pkg?: PkgNameVersion } -export type GitFetcher = FetchFunction }> +export type GitFetcher = FetchFunction, manifest?: DependencyManifest }> export interface DirectoryFetcherOptions { lockfileDir: string diff --git a/fetching/git-fetcher/package.json b/fetching/git-fetcher/package.json index 90bec955e1..59586e1836 100644 --- a/fetching/git-fetcher/package.json +++ b/fetching/git-fetcher/package.json @@ -44,7 +44,6 @@ "@pnpm/git-fetcher": "workspace:*", "@pnpm/store.cafs": "workspace:*", "@pnpm/types": "workspace:*", - "p-defer": "^3.0.0", "tempy": "^1.0.1" }, "funding": "https://opencollective.com/pnpm", diff --git a/fetching/git-fetcher/src/index.ts b/fetching/git-fetcher/src/index.ts index 9917320816..74d24dfd5e 100644 --- a/fetching/git-fetcher/src/index.ts +++ b/fetching/git-fetcher/src/index.ts @@ -44,16 +44,16 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions) { } // removing /.git to make directory integrity calculation faster await rimraf(path.join(tempLocation, '.git')) - const filesIndex = await addFilesFromDir({ - cafsDir: cafs.cafsDir, - dir: tempLocation, - filesIndexFile: opts.filesIndexFile, - manifest: opts.manifest, - }) // 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 { filesIndex } + return addFilesFromDir({ + cafsDir: cafs.cafsDir, + dir: tempLocation, + filesIndexFile: opts.filesIndexFile, + readManifest: opts.readManifest, + pkg: opts.pkg, + }) } return { diff --git a/fetching/git-fetcher/test/index.ts b/fetching/git-fetcher/test/index.ts index edd5a18927..91ba54a40d 100644 --- a/fetching/git-fetcher/test/index.ts +++ b/fetching/git-fetcher/test/index.ts @@ -3,8 +3,6 @@ import path from 'path' import { createCafsStore } from '@pnpm/create-cafs-store' import { createGitFetcher } from '@pnpm/git-fetcher' import { globalWarn } from '@pnpm/logger' -import { type DependencyManifest } from '@pnpm/types' -import pDefer from 'p-defer' import tempy from 'tempy' import execa from 'execa' @@ -33,8 +31,7 @@ beforeEach(() => { test('fetch', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ rawConfig: {} }).git - const manifest = pDefer() - const { filesIndex } = await fetch( + const { filesIndex, manifest } = await fetch( createCafsStore(cafsDir), { commit: 'c9b30e71d704cd30fa71f2edd1ecc7dcc4985493', @@ -42,19 +39,17 @@ test('fetch', async () => { type: 'git', }, { - manifest, + readManifest: true, filesIndexFile: path.join(cafsDir, 'index.json'), } ) expect(filesIndex['package.json']).toBeTruthy() - const name = (await manifest.promise).name - expect(name).toEqual('is-positive') + expect(manifest?.name).toEqual('is-positive') }) test('fetch a package from Git that has a prepare script', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ rawConfig: {} }).git - const manifest = pDefer() const { filesIndex } = await fetch( createCafsStore(cafsDir), { @@ -63,7 +58,6 @@ test('fetch a package from Git that has a prepare script', async () => { type: 'git', }, { - manifest, filesIndexFile: path.join(cafsDir, 'index.json'), } ) @@ -74,7 +68,6 @@ test('fetch a package from Git that has a prepare script', async () => { test('fetch a package without a package.json', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ rawConfig: {} }).git - const manifest = pDefer() const { filesIndex } = await fetch( createCafsStore(cafsDir), { @@ -84,7 +77,6 @@ test('fetch a package without a package.json', async () => { type: 'git', }, { - manifest, filesIndexFile: path.join(cafsDir, 'index.json'), } ) @@ -95,14 +87,12 @@ test('fetch a package without a package.json', async () => { test('fetch a big repository', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ rawConfig: {} }).git - const manifest = pDefer() const { filesIndex } = await fetch(createCafsStore(cafsDir), { commit: 'a65fbf5a90f53c9d72fed4daaca59da50f074355', repo: 'https://github.com/sveltejs/action-deploy-docs.git', type: 'git', }, { - manifest, filesIndexFile: path.join(cafsDir, 'index.json'), }) expect(filesIndex).toBeTruthy() @@ -111,14 +101,13 @@ test('fetch a big repository', async () => { test('still able to shallow fetch for allowed hosts', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ gitShallowHosts: ['github.com'], rawConfig: {} }).git - const manifest = pDefer() const resolution = { commit: 'c9b30e71d704cd30fa71f2edd1ecc7dcc4985493', repo: 'https://github.com/kevva/is-positive.git', type: 'git' as const, } - const { filesIndex } = await fetch(createCafsStore(cafsDir), resolution, { - manifest, + const { filesIndex, manifest } = await fetch(createCafsStore(cafsDir), resolution, { + readManifest: true, filesIndexFile: path.join(cafsDir, 'index.json'), }) const calls = (execa as jest.Mock).mock.calls @@ -135,14 +124,12 @@ test('still able to shallow fetch for allowed hosts', async () => { expect(calls[i].slice(0, -1)).toEqual(expectedCalls[i]) } expect(filesIndex['package.json']).toBeTruthy() - const name = (await manifest.promise).name - expect(name).toEqual('is-positive') + expect(manifest?.name).toEqual('is-positive') }) test('fail when preparing a git-hosted package', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ rawConfig: {} }).git - const manifest = pDefer() await expect( fetch(createCafsStore(cafsDir), { @@ -150,7 +137,6 @@ test('fail when preparing a git-hosted package', async () => { repo: 'https://github.com/pnpm-e2e/prepare-script-fails.git', type: 'git', }, { - manifest, filesIndexFile: path.join(cafsDir, 'index.json'), }) ).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-fails.git": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`') @@ -159,14 +145,12 @@ test('fail when preparing a git-hosted package', async () => { test('do not build the package when scripts are ignored', async () => { const cafsDir = tempy.directory() const fetch = createGitFetcher({ ignoreScripts: true, rawConfig: {} }).git - const manifest = pDefer() const { filesIndex } = await fetch(createCafsStore(cafsDir), { commit: '55416a9c468806a935636c0ad0371a14a64df8c9', repo: 'https://github.com/pnpm-e2e/prepare-script-works.git', type: 'git', }, { - manifest, filesIndexFile: path.join(cafsDir, 'index.json'), }) expect(filesIndex['package.json']).toBeTruthy() diff --git a/fetching/tarball-fetcher/package.json b/fetching/tarball-fetcher/package.json index 49529e4b00..7267846e4f 100644 --- a/fetching/tarball-fetcher/package.json +++ b/fetching/tarball-fetcher/package.json @@ -58,7 +58,6 @@ "@types/retry": "^0.12.2", "@types/ssri": "^7.1.1", "nock": "13.3.2", - "safe-promise-defer": "^1.0.1", "ssri": "10.0.4", "tempy": "^1.0.1" }, diff --git a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts index 9dae3a09cd..555d350181 100644 --- a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts @@ -18,13 +18,13 @@ export interface CreateGitHostedTarballFetcher { export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction, fetcherOpts: CreateGitHostedTarballFetcher): FetchFunction { const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => { - const { filesIndex } = await fetchRemoteTarball(cafs, resolution, opts) + const { filesIndex, manifest } = await fetchRemoteTarball(cafs, resolution, opts) try { - const prepareResult = await prepareGitHostedPkg(filesIndex as Record, cafs, opts.filesIndexFile, fetcherOpts) + const prepareResult = await prepareGitHostedPkg(filesIndex as Record, cafs, opts.filesIndexFile, fetcherOpts, opts) if (prepareResult.ignoredBuild) { globalWarn(`The git-hosted package fetched from "${resolution.tarball}" has to be built but the build scripts were ignored.`) } - return { filesIndex: prepareResult.filesIndex } + return { filesIndex: prepareResult.filesIndex, manifest } } catch (err: any) { // eslint-disable-line err.message = `Failed to prepare git-hosted package fetched from "${resolution.tarball}": ${err.message}` throw err @@ -34,7 +34,13 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction return fetch as FetchFunction } -async function prepareGitHostedPkg (filesIndex: Record, cafs: Cafs, filesIndexFile: string, opts: CreateGitHostedTarballFetcher) { +async function prepareGitHostedPkg ( + filesIndex: Record, + cafs: Cafs, + filesIndexFile: string, + opts: CreateGitHostedTarballFetcher, + fetcherOpts: FetchOptions +) { const tempLocation = await cafs.tempDir() cafs.importPackage(tempLocation, { filesResponse: { @@ -44,16 +50,16 @@ async function prepareGitHostedPkg (filesIndex: Record, cafs: Ca force: true, }) const shouldBeBuilt = await preparePackage(opts, tempLocation) - const newFilesIndex = await addFilesFromDir({ - cafsDir: cafs.cafsDir, - dir: tempLocation, - filesIndexFile, - }) // 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 { - filesIndex: newFilesIndex, + ...await addFilesFromDir({ + cafsDir: cafs.cafsDir, + dir: tempLocation, + filesIndexFile, + pkg: fetcherOpts.pkg, + }), ignoredBuild: opts.ignoreScripts && shouldBeBuilt, } } diff --git a/fetching/tarball-fetcher/src/index.ts b/fetching/tarball-fetcher/src/index.ts index e9bd5da23b..7e141b87bc 100644 --- a/fetching/tarball-fetcher/src/index.ts +++ b/fetching/tarball-fetcher/src/index.ts @@ -79,10 +79,11 @@ async function fetchFromTarball ( getAuthHeaderByURI: ctx.getAuthHeaderByURI, cafs, integrity: resolution.integrity, - manifest: opts.manifest, + readManifest: opts.readManifest, onProgress: opts.onProgress, onStart: opts.onStart, registry: resolution.registry, filesIndexFile: opts.filesIndexFile, + pkg: opts.pkg, }) } diff --git a/fetching/tarball-fetcher/src/localTarballFetcher.ts b/fetching/tarball-fetcher/src/localTarballFetcher.ts index e0605d6b07..798ffd2c2f 100644 --- a/fetching/tarball-fetcher/src/localTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/localTarballFetcher.ts @@ -13,20 +13,18 @@ interface Resolution { } export function createLocalTarballFetcher (): FetchFunction { - const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => { + const fetch = (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => { const tarball = resolvePath(opts.lockfileDir, resolution.tarball.slice(5)) - const buffer = gfs.readFileSync(tarball) - return { - filesIndex: await addFilesFromTarball({ - cafsDir: cafs.cafsDir, - buffer, - filesIndexFile: opts.filesIndexFile, - integrity: resolution.integrity, - manifest: opts.manifest, - url: tarball, - }), - } + return addFilesFromTarball({ + cafsDir: cafs.cafsDir, + buffer, + filesIndexFile: opts.filesIndexFile, + integrity: resolution.integrity, + readManifest: opts.readManifest, + url: tarball, + pkg: opts.pkg, + }) } return fetch as FetchFunction diff --git a/fetching/tarball-fetcher/src/remoteTarballFetcher.ts b/fetching/tarball-fetcher/src/remoteTarballFetcher.ts index 0f1458cfc6..95659381f7 100644 --- a/fetching/tarball-fetcher/src/remoteTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/remoteTarballFetcher.ts @@ -1,8 +1,8 @@ import { type IncomingMessage } from 'http' import { requestRetryLogger } from '@pnpm/core-loggers' import { FetchError } from '@pnpm/error' -import { type FetchResult } from '@pnpm/fetcher-base' -import type { Cafs, DeferredManifestPromise } from '@pnpm/cafs-types' +import { type FetchResult, type FetchOptions } from '@pnpm/fetcher-base' +import { type Cafs } from '@pnpm/cafs-types' import { type FetchFromRegistry } from '@pnpm/fetching-types' import { addFilesFromTarball } from '@pnpm/worker' import * as retry from '@zkochan/retry' @@ -18,13 +18,13 @@ export interface HttpResponse { export type DownloadFunction = (url: string, opts: { getAuthHeaderByURI: (registry: string) => string | undefined cafs: Cafs - manifest?: DeferredManifestPromise + readManifest?: boolean registry?: string onStart?: (totalSize: number | null, attempt: number) => void onProgress?: (downloaded: number) => void integrity?: string filesIndexFile: string -}) => Promise +} & Pick) => Promise export interface NpmRegistryClient { get: (url: string, getOpts: object, cb: (err: Error, data: object, raw: object, res: HttpResponse) => void) => void @@ -56,13 +56,13 @@ export function createDownloader ( return async function download (url: string, opts: { getAuthHeaderByURI: (registry: string) => string | undefined cafs: Cafs - manifest?: DeferredManifestPromise + readManifest?: boolean registry?: string onStart?: (totalSize: number | null, attempt: number) => void onProgress?: (downloaded: number) => void integrity?: string filesIndexFile: string - }): Promise { + } & Pick): Promise { const authHeaderValue = opts.getAuthHeaderByURI(url) const op = retry.operation(retryOpts) @@ -147,16 +147,15 @@ export function createDownloader ( chunk.copy(data, offset) offset += chunk.length } - return { - filesIndex: await addFilesFromTarball({ - buffer: data, - cafsDir: opts.cafs.cafsDir, - manifest: opts.manifest, - integrity: opts.integrity, - filesIndexFile: opts.filesIndexFile, - url, - }), - } + return await addFilesFromTarball({ + buffer: data, + cafsDir: opts.cafs.cafsDir, + readManifest: opts.readManifest, + integrity: opts.integrity, + filesIndexFile: opts.filesIndexFile, + url, + pkg: opts.pkg, + }) } catch (err: any) { // eslint-disable-line err.attempts = currentAttempt err.resource = url diff --git a/fetching/tarball-fetcher/test/fetch.ts b/fetching/tarball-fetcher/test/fetch.ts index a7921835fc..b49a641d45 100644 --- a/fetching/tarball-fetcher/test/fetch.ts +++ b/fetching/tarball-fetcher/test/fetch.ts @@ -11,9 +11,7 @@ import { BadTarballError, TarballIntegrityError, } from '@pnpm/tarball-fetcher' -import { type DependencyManifest } from '@pnpm/types' import nock from 'nock' -import safePromiseDefer from 'safe-promise-defer' import ssri from 'ssri' import tempy from 'tempy' @@ -72,6 +70,7 @@ test('fail when tarball size does not match content-length', async () => { fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow( new BadTarballError({ @@ -103,6 +102,7 @@ test('retry when tarball size does not match content-length', async () => { const result = await fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(result.filesIndex).toBeTruthy() @@ -128,6 +128,7 @@ test('fail when integrity check fails two times in a row', async () => { fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow( new TarballIntegrityError({ @@ -166,6 +167,7 @@ test('retry when integrity check fails', async () => { onStart (size, attempts) { params.push([size, attempts]) }, + pkg: {}, }) expect(params[0]).toStrictEqual([1194, 1]) @@ -188,6 +190,7 @@ test('fail when integrity check of local file fails', async () => { fetch.localTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow( new TarballIntegrityError({ @@ -213,6 +216,7 @@ test("don't fail when integrity check of local file succeeds", async () => { const { filesIndex } = await fetch.localTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(typeof filesIndex['package.json']).toBe('string') @@ -239,6 +243,7 @@ test("don't fail when fetching a local tarball in offline mode", async () => { const { filesIndex } = await fetch.localTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(typeof filesIndex['package.json']).toBe('string') @@ -266,6 +271,7 @@ test('fail when trying to fetch a non-local tarball in offline mode', async () = fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow( new PnpmError('NO_OFFLINE_TARBALL', @@ -293,6 +299,7 @@ test('retry on server error', async () => { const index = await fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(index).toBeTruthy() @@ -316,6 +323,7 @@ test('throw error when accessing private package w/o authorization', async () => fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow( new FetchError( @@ -367,6 +375,7 @@ test('accessing private packages', async () => { const index = await fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(index).toBeTruthy() @@ -387,6 +396,7 @@ test('fetch a big repository', async () => { const result = await fetch.gitHostedTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(result.filesIndex).toBeTruthy() @@ -401,6 +411,7 @@ test('fail when preparing a git-hosted package', async () => { fetch.gitHostedTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://codeload.github.com/pnpm-e2e/prepare-script-fails/tar.gz/ba58874aae1210a777eb309dd01a9fdacc7e54e7": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`') }) @@ -421,6 +432,7 @@ test('fail when extracting a broken tarball', async () => { fetch.remoteTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) ).rejects.toThrow(`Failed to unpack the tarball from "${registry}foo.tgz": Error: Invalid checksum for TAR header at offset 0. Expected 0, got NaN` ) @@ -445,6 +457,7 @@ test('do not build the package when scripts are ignored', async () => { const { filesIndex } = await fetch.gitHostedTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), + pkg: {}, }) expect(filesIndex).toHaveProperty(['package.json']) @@ -458,13 +471,13 @@ test('when extracting files with the same name, pick the last ones', async () => tarball: `file:${tar}`, } - const manifest = safePromiseDefer() - const { filesIndex } = await fetch.localTarball(cafs, resolution, { + const { filesIndex, manifest } = await fetch.localTarball(cafs, resolution, { filesIndexFile, lockfileDir: process.cwd(), - manifest, + readManifest: true, + pkg: {}, }) const pkgJson = JSON.parse(fs.readFileSync(filesIndex['package.json'], 'utf8')) expect(pkgJson.name).toBe('pkg2') - expect((await manifest())?.name).toBe('pkg2') + expect(manifest?.name).toBe('pkg2') }) diff --git a/pkg-manager/package-requester/package.json b/pkg-manager/package-requester/package.json index 61bd7ac6f6..e5f07cad55 100644 --- a/pkg-manager/package-requester/package.json +++ b/pkg-manager/package-requester/package.json @@ -53,7 +53,6 @@ "p-queue": "^6.6.2", "promise-share": "^1.0.0", "ramda": "npm:@pnpm/ramda@0.28.1", - "safe-promise-defer": "^1.0.1", "semver": "^7.5.4", "ssri": "10.0.4" }, diff --git a/pkg-manager/package-requester/src/packageRequester.ts b/pkg-manager/package-requester/src/packageRequester.ts index b11cc558d6..e4a2bf2a50 100644 --- a/pkg-manager/package-requester/src/packageRequester.ts +++ b/pkg-manager/package-requester/src/packageRequester.ts @@ -15,7 +15,7 @@ import { type FetchOptions, type FetchResult, } from '@pnpm/fetcher-base' -import type { Cafs, DeferredManifestPromise, PackageFilesResponse } from '@pnpm/cafs-types' +import { type Cafs, type PackageFilesResponse } from '@pnpm/cafs-types' import gfs from '@pnpm/graceful-fs' import { logger } from '@pnpm/logger' import { packageIsInstallable } from '@pnpm/package-is-installable' @@ -49,7 +49,6 @@ import pick from 'ramda/src/pick' import semver from 'semver' import ssri from 'ssri' import { equalOrSemverEqual } from './equalOrSemverEqual' -import safePromiseDefer from 'safe-promise-defer' const TARBALL_INTEGRITY_FILENAME = 'tarball-integrity' const packageRequestLogger = logger('package-requester') @@ -318,8 +317,8 @@ function fetchToStore ( ctx: { readPkgFromCafs: ( filesIndexFile: string, - manifest?: DeferredManifestPromise - ) => Promise<{ verified: boolean, pkgFilesIndex: PackageFilesIndex }> + readManifest?: boolean + ) => Promise<{ verified: boolean, pkgFilesIndex: PackageFilesIndex, manifest?: DependencyManifest }> fetch: ( packageId: string, resolution: Resolution, @@ -460,10 +459,7 @@ function fetchToStore ( ) && !isLocalPkg ) { - const manifest = opts.fetchRawManifest - ? safePromiseDefer() - : undefined - const { verified, pkgFilesIndex } = await ctx.readPkgFromCafs(filesIndexFile, manifest) + const { verified, pkgFilesIndex, manifest } = await ctx.readPkgFromCafs(filesIndexFile, opts.fetchRawManifest) if (verified) { if ( ( @@ -492,12 +488,8 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF fromStore: true, sideEffects: pkgFilesIndex.sideEffects, }) - if (manifest != null) { - manifest() - .then((manifest) => { - bundledManifest.resolve(manifest == null ? manifest : normalizeBundledManifest(manifest)) - }) - .catch(bundledManifest.reject) + if (opts.fetchRawManifest) { + bundledManifest.resolve(manifest == null ? manifest : normalizeBundledManifest(manifest)) } finishing.resolve(undefined) return @@ -519,23 +511,13 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF // As many tarballs should be downloaded simultaneously as possible. const priority = (++ctx.requestsQueue.counter % ctx.requestsQueue.concurrency === 0 ? -1 : 1) * 1000 - const fetchManifest = opts.fetchRawManifest - ? safePromiseDefer() - : undefined - if (fetchManifest != null) { - fetchManifest() - .then((manifest) => { - bundledManifest.resolve(manifest == null ? manifest : normalizeBundledManifest(manifest)) - }) - .catch(bundledManifest.reject) - } const fetchedPackage = await ctx.requestsQueue.add(async () => ctx.fetch( opts.pkg.id, opts.pkg.resolution, { filesIndexFile, lockfileDir: opts.lockfileDir, - manifest: fetchManifest, + readManifest: opts.fetchRawManifest, onProgress: (downloaded) => { fetchingProgressLogger.debug({ downloaded, @@ -551,8 +533,15 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF status: 'started', }) }, + pkg: { + name: opts.pkg.name, + version: opts.pkg.version, + }, } ), { priority }) + if (opts.fetchRawManifest) { + bundledManifest.resolve(fetchedPackage.manifest == null ? fetchedPackage.manifest : normalizeBundledManifest(fetchedPackage.manifest)) + } const filesResult: PackageFilesResponse = { local: fetchedPackage.local, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f4ed1daf83..c5770c31c5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1555,9 +1555,6 @@ importers: '@pnpm/types': specifier: workspace:* version: link:../../packages/types - p-defer: - specifier: ^3.0.0 - version: 3.0.0 tempy: specifier: ^1.0.1 version: 1.0.1 @@ -1646,9 +1643,6 @@ importers: nock: specifier: 13.3.2 version: 13.3.2 - safe-promise-defer: - specifier: ^1.0.1 - version: 1.0.1 ssri: specifier: 10.0.4 version: 10.0.4 @@ -3669,9 +3663,6 @@ importers: ramda: specifier: npm:@pnpm/ramda@0.28.1 version: /@pnpm/ramda@0.28.1 - safe-promise-defer: - specifier: ^1.0.1 - version: 1.0.1 semver: specifier: ^7.5.4 version: 7.5.4 @@ -14441,6 +14432,7 @@ packages: /p-reflect@2.1.0: resolution: {integrity: sha512-paHV8NUz8zDHu5lhr/ngGWQiW067DK/+IbJ+RfZ4k+s8y4EKyYCz8pGYWjxCg35eHztpJAt+NUgvN4L+GCbPlg==} engines: {node: '>=8'} + dev: false /p-settle@4.1.1: resolution: {integrity: sha512-6THGh13mt3gypcNMm0ADqVNCcYa3BK6DWsuJWFCuEKP1rpY+OKGp7gaZwVmLspmic01+fsg/fN57MfvDzZ/PuQ==} @@ -14792,6 +14784,7 @@ packages: engines: {node: '>=8'} dependencies: p-reflect: 2.1.0 + dev: false /prompts@2.4.2: resolution: {integrity: sha512-NxNv/kLguCA7p3jE8oL2aEBsrJWgAakBpgmgK6lpPWV+WuOmY6r2/zbAVnP+T8bQlA0nzHXSJSJW0Hq7ylaD2Q==} @@ -15414,6 +15407,7 @@ packages: engines: {node: '>=12'} dependencies: promise-share: 1.0.0 + dev: false /safe-regex-test@1.0.0: resolution: {integrity: sha512-JBUUzyOgEwXQY1NuPtvcj/qcBDbDmEvWufhlnXZIm75DEHp+afM1r1ujJpJsV/gSM4t59tpDyPi1sd6ZaPFfsA==} diff --git a/store/cafs/src/addFilesFromDir.ts b/store/cafs/src/addFilesFromDir.ts index 85a7c227f1..2d40747e57 100644 --- a/store/cafs/src/addFilesFromDir.ts +++ b/store/cafs/src/addFilesFromDir.ts @@ -11,10 +11,11 @@ import { parseJsonBufferSync } from './parseJson' export function addFilesFromDir ( addBuffer: (buffer: Buffer, mode: number) => FileWriteResult, - dirname: string + dirname: string, + readManifest?: boolean ): AddToStoreResult { const filesIndex: FilesIndex = {} - const manifest = _retrieveFileIntegrities(addBuffer, dirname, dirname, filesIndex) + const manifest = _retrieveFileIntegrities(addBuffer, dirname, dirname, filesIndex, readManifest) return { filesIndex, manifest } } @@ -22,7 +23,8 @@ function _retrieveFileIntegrities ( addBuffer: (buffer: Buffer, mode: number) => FileWriteResult, rootDir: string, currDir: string, - index: FilesIndex + index: FilesIndex, + readManifest?: boolean ) { const files = fs.readdirSync(currDir, { withFileTypes: true }) let manifest: DependencyManifest | undefined @@ -44,7 +46,7 @@ function _retrieveFileIntegrities ( continue } const buffer = gfs.readFileSync(fullPath) - if (rootDir === currDir && file.name === 'package.json') { + if (rootDir === currDir && readManifest && file.name === 'package.json') { manifest = parseJsonBufferSync(buffer) } index[relativePath] = { diff --git a/store/cafs/src/addFilesFromTarball.ts b/store/cafs/src/addFilesFromTarball.ts index 6f5ec7730d..acb73f9822 100644 --- a/store/cafs/src/addFilesFromTarball.ts +++ b/store/cafs/src/addFilesFromTarball.ts @@ -11,7 +11,8 @@ import { parseTarball } from './parseTarball' export function addFilesFromTarball ( addBufferToCafs: (buffer: Buffer, mode: number) => FileWriteResult, _ignore: null | ((filename: string) => boolean), - tarballBuffer: Buffer + tarballBuffer: Buffer, + readManifest?: boolean ): AddToStoreResult { const ignore = _ignore ?? (() => false) const tarContent = isGzip(tarballBuffer) ? gunzipSync(tarballBuffer) : (Buffer.isBuffer(tarballBuffer) ? tarballBuffer : Buffer.from(tarballBuffer)) @@ -23,7 +24,7 @@ export function addFilesFromTarball ( if (ignore(relativePath)) continue const fileBuffer = tarContent.slice(offset, offset + size) - if (relativePath === 'package.json') { + if (readManifest && relativePath === 'package.json') { manifestBuffer = fileBuffer } filesIndex[relativePath] = { diff --git a/store/package-store/src/storeController/index.ts b/store/package-store/src/storeController/index.ts index 212b028aa6..e2c13f1252 100644 --- a/store/package-store/src/storeController/index.ts +++ b/store/package-store/src/storeController/index.ts @@ -93,6 +93,7 @@ export async function createPackageStore ( dir: builtPkgLocation, sideEffectsCacheKey: opts.sideEffectsCacheKey, filesIndexFile: opts.filesIndexFile, + pkg: {}, }) } } diff --git a/worker/src/index.ts b/worker/src/index.ts index 1386127302..714c23c7f6 100644 --- a/worker/src/index.ts +++ b/worker/src/index.ts @@ -1,9 +1,9 @@ import path from 'path' import os from 'os' import { WorkerPool } from '@rushstack/worker-pool/lib/WorkerPool' -import { type DeferredManifestPromise } from '@pnpm/cafs-types' import { PnpmError } from '@pnpm/error' import { type PackageFilesIndex } from '@pnpm/store.cafs' +import { type DependencyManifest } from '@pnpm/types' import { type TarballExtractMessage, type AddDirToStoreMessage } from './types' export { type WorkerPool } @@ -42,12 +42,10 @@ function createTarballWorkerPool () { } export async function addFilesFromDir ( - opts: Pick & { - manifest?: DeferredManifestPromise - } + opts: Pick ) { const localWorker = await workerPool.checkoutWorkerAsync(true) - return new Promise>((resolve, reject) => { + return new Promise<{ filesIndex: Record, manifest: DependencyManifest }>((resolve, reject) => { // eslint-disalbe-next-line localWorker.once('message', ({ status, error, value }) => { workerPool.checkinWorker(localWorker) @@ -55,8 +53,7 @@ export async function addFilesFromDir ( reject(new PnpmError('GIT_FETCH_FAILED', error as string)) return } - opts.manifest?.resolve(value.manifest) - resolve(value.filesIndex) + resolve(value) }) localWorker.postMessage({ type: 'add-dir', @@ -64,6 +61,8 @@ export async function addFilesFromDir ( dir: opts.dir, filesIndexFile: opts.filesIndexFile, sideEffectsCacheKey: opts.sideEffectsCacheKey, + readManifest: opts.readManifest, + pkg: opts.pkg, }) }) } @@ -103,13 +102,12 @@ If you think that this is the case, then run "pnpm store prune" and rerun the co } export async function addFilesFromTarball ( - opts: Pick & { + opts: Pick & { url: string - manifest?: DeferredManifestPromise } ) { const localWorker = await workerPool.checkoutWorkerAsync(true) - return new Promise>((resolve, reject) => { + return new Promise<{ filesIndex: Record, manifest: DependencyManifest }>((resolve, reject) => { localWorker.once('message', ({ status, error, value }) => { workerPool.checkinWorker(localWorker) if (status === 'error') { @@ -123,8 +121,7 @@ export async function addFilesFromTarball ( reject(new PnpmError('TARBALL_EXTRACT', `Failed to unpack the tarball from "${opts.url}": ${error as string}`)) return } - opts.manifest?.resolve(value.manifest) - resolve(value.filesIndex) + resolve(value) }) localWorker.postMessage({ type: 'extract', @@ -132,6 +129,8 @@ export async function addFilesFromTarball ( cafsDir: opts.cafsDir, integrity: opts.integrity, filesIndexFile: opts.filesIndexFile, + readManifest: opts.readManifest, + pkg: opts.pkg, }) }) } @@ -140,8 +139,8 @@ export async function readPkgFromCafs ( cafsDir: string, verifyStoreIntegrity: boolean, filesIndexFile: string, - manifest?: DeferredManifestPromise -): Promise<{ verified: boolean, pkgFilesIndex: PackageFilesIndex }> { + readManifest?: boolean +): Promise<{ verified: boolean, pkgFilesIndex: PackageFilesIndex, manifest?: DependencyManifest }> { const localWorker = await workerPool.checkoutWorkerAsync(true) return new Promise<{ verified: boolean, pkgFilesIndex: PackageFilesIndex }>((resolve, reject) => { localWorker.once('message', ({ status, error, value }) => { @@ -150,17 +149,13 @@ export async function readPkgFromCafs ( reject(new PnpmError('READ_FROM_STORE', error as string)) return } - manifest?.resolve(value.manifest) - resolve({ - verified: value.verified, - pkgFilesIndex: value.pkgFilesIndex, - }) + resolve(value) }) localWorker.postMessage({ type: 'readPkgFromCafs', cafsDir, filesIndexFile, - readManifest: manifest != null, + readManifest, verifyStoreIntegrity, }) }) diff --git a/worker/src/types.ts b/worker/src/types.ts index 9867f81119..f939d6977a 100644 --- a/worker/src/types.ts +++ b/worker/src/types.ts @@ -1,11 +1,18 @@ import { type PackageFilesResponse } from '@pnpm/cafs-types' +export interface PkgNameVersion { + name?: string + version?: string +} + export interface TarballExtractMessage { type: 'extract' buffer: Buffer cafsDir: string integrity?: string filesIndexFile: string + readManifest?: boolean + pkg?: PkgNameVersion } export interface LinkPkgMessage { @@ -26,6 +33,8 @@ export interface AddDirToStoreMessage { dir: string filesIndexFile: string sideEffectsCacheKey?: string + readManifest?: boolean + pkg?: PkgNameVersion } export interface ReadPkgFromCafsMessage { diff --git a/worker/src/worker.ts b/worker/src/worker.ts index 3301a53151..ac85df2354 100644 --- a/worker/src/worker.ts +++ b/worker/src/worker.ts @@ -93,7 +93,7 @@ async function handleMessage (message: TarballExtractMessage | LinkPkgMessage | } } -function addTarballToStore ({ buffer, cafsDir, integrity, filesIndexFile }: TarballExtractMessage) { +function addTarballToStore ({ buffer, cafsDir, integrity, filesIndexFile, pkg, readManifest }: TarballExtractMessage) { if (integrity) { const [, algo, integrityHash] = integrity.match(INTEGRITY_REGEX)! // Compensate for the possibility of non-uniform Base64 padding @@ -116,18 +116,18 @@ function addTarballToStore ({ buffer, cafsDir, integrity, filesIndexFile }: Tarb cafsCache.set(cafsDir, createCafs(cafsDir)) } const cafs = cafsCache.get(cafsDir)! - const { filesIndex, manifest } = cafs.addFilesFromTarball(buffer) + const { filesIndex, manifest } = cafs.addFilesFromTarball(buffer, readManifest) const { filesIntegrity, filesMap } = processFilesIndex(filesIndex) - writeFilesIndexFile(filesIndexFile, { pkg: manifest ?? {}, files: filesIntegrity }) + writeFilesIndexFile(filesIndexFile, { pkg: pkg ?? {}, files: filesIntegrity }) return { status: 'success', value: { filesIndex: filesMap, manifest } } } -function addFilesFromDir ({ dir, cafsDir, filesIndexFile, sideEffectsCacheKey }: AddDirToStoreMessage) { +function addFilesFromDir ({ dir, cafsDir, filesIndexFile, sideEffectsCacheKey, pkg, readManifest }: AddDirToStoreMessage) { if (!cafsCache.has(cafsDir)) { cafsCache.set(cafsDir, createCafs(cafsDir)) } const cafs = cafsCache.get(cafsDir)! - const { filesIndex, manifest } = cafs.addFilesFromDir(dir) + const { filesIndex, manifest } = cafs.addFilesFromDir(dir, readManifest) const { filesIntegrity, filesMap } = processFilesIndex(filesIndex) if (sideEffectsCacheKey) { let filesIndex!: PackageFilesIndex @@ -140,7 +140,7 @@ function addFilesFromDir ({ dir, cafsDir, filesIndexFile, sideEffectsCacheKey }: filesIndex.sideEffects[sideEffectsCacheKey] = filesIntegrity writeJsonFile(filesIndexFile, filesIndex) } else { - writeFilesIndexFile(filesIndexFile, { pkg: manifest ?? {}, files: filesIntegrity }) + writeFilesIndexFile(filesIndexFile, { pkg: pkg ?? {}, files: filesIntegrity }) } return { status: 'success', value: { filesIndex: filesMap, manifest } } }