diff --git a/.changeset/sharp-months-compare.md b/.changeset/sharp-months-compare.md new file mode 100644 index 0000000000..f63ece775e --- /dev/null +++ b/.changeset/sharp-months-compare.md @@ -0,0 +1,6 @@ +--- +"@pnpm/tarball-fetcher": patch +"pnpm": patch +--- + +A git-hosted dependency should not be added to the store if it failed to be built [#7407](https://github.com/pnpm/pnpm/pull/7407). diff --git a/fetching/tarball-fetcher/package.json b/fetching/tarball-fetcher/package.json index 01c859c6bf..75959fe99e 100644 --- a/fetching/tarball-fetcher/package.json +++ b/fetching/tarball-fetcher/package.json @@ -44,7 +44,9 @@ "@zkochan/retry": "^0.2.0", "lodash.throttle": "4.1.1", "p-map-values": "^1.0.0", - "ramda": "npm:@pnpm/ramda@0.28.1" + "path-temp": "^2.1.0", + "ramda": "npm:@pnpm/ramda@0.28.1", + "rename-overwrite": "^5.0.0" }, "devDependencies": { "@pnpm/cafs-types": "workspace:*", diff --git a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts index 55d4983e18..a0b0226a4d 100644 --- a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts @@ -1,8 +1,11 @@ +import fs from 'node:fs/promises' import { type FetchFunction, type FetchOptions } from '@pnpm/fetcher-base' import type { Cafs } from '@pnpm/cafs-types' import { globalWarn } from '@pnpm/logger' import { preparePackage } from '@pnpm/prepare-package' import { addFilesFromDir } from '@pnpm/worker' +import renameOverwrite from 'rename-overwrite' +import { fastPathTemp as pathTemp } from 'path-temp' interface Resolution { integrity?: string @@ -18,9 +21,15 @@ export interface CreateGitHostedTarballFetcher { export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction, fetcherOpts: CreateGitHostedTarballFetcher): FetchFunction { const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => { - const { filesIndex, manifest } = await fetchRemoteTarball(cafs, resolution, opts) + // This solution is not perfect but inside the fetcher we don't currently know the location + // of the built and non-built index files. + const nonBuiltIndexFile = fetcherOpts.ignoreScripts ? opts.filesIndexFile : pathTemp(opts.filesIndexFile) + const { filesIndex, manifest } = await fetchRemoteTarball(cafs, resolution, { + ...opts, + filesIndexFile: nonBuiltIndexFile, + }) try { - const prepareResult = await prepareGitHostedPkg(filesIndex as Record, cafs, opts.filesIndexFile, fetcherOpts, opts) + const prepareResult = await prepareGitHostedPkg(filesIndex as Record, cafs, nonBuiltIndexFile, 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.`) } @@ -37,6 +46,7 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction async function prepareGitHostedPkg ( filesIndex: Record, cafs: Cafs, + filesIndexFileNonBuilt: string, filesIndexFile: string, opts: CreateGitHostedTarballFetcher, fetcherOpts: FetchOptions @@ -50,6 +60,25 @@ async function prepareGitHostedPkg ( force: true, }) const shouldBeBuilt = await preparePackage(opts, tempLocation) + if (!shouldBeBuilt) { + if (filesIndexFileNonBuilt !== filesIndexFile) { + await renameOverwrite(filesIndexFileNonBuilt, filesIndexFile) + } + return { + filesIndex, + ignoredBuild: false, + } + } + if (opts.ignoreScripts) { + return { + filesIndex, + ignoredBuild: true, + } + } + try { + // The temporary index file may be deleted + await fs.unlink(filesIndexFileNonBuilt) + } catch {} // 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. @@ -60,6 +89,6 @@ async function prepareGitHostedPkg ( filesIndexFile, pkg: fetcherOpts.pkg, }), - ignoredBuild: opts.ignoreScripts && shouldBeBuilt, + ignoredBuild: false, } } diff --git a/pkg-manager/core/test/install/fromRepo.ts b/pkg-manager/core/test/install/fromRepo.ts index 4504dccff2..085fe023c9 100644 --- a/pkg-manager/core/test/install/fromRepo.ts +++ b/pkg-manager/core/test/install/fromRepo.ts @@ -314,3 +314,15 @@ test('should not update when adding unrelated dependency', async () => { }, }) }) + +test('git-hosted repository is not added to the store if it fails to be built', async () => { + prepareEmpty() + + await expect( + addDependenciesToPackage({}, ['pnpm-e2e/prepare-script-fails'], await testDefaults()) + ).rejects.toThrow() + + await expect( + addDependenciesToPackage({}, ['pnpm-e2e/prepare-script-fails'], await testDefaults()) + ).rejects.toThrow() +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6bbf1897ad..7d3e8abeca 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1642,9 +1642,15 @@ importers: p-map-values: specifier: ^1.0.0 version: 1.0.0 + path-temp: + specifier: ^2.1.0 + version: 2.1.0 ramda: specifier: npm:@pnpm/ramda@0.28.1 version: /@pnpm/ramda@0.28.1 + rename-overwrite: + specifier: ^5.0.0 + version: 5.0.0 devDependencies: '@pnpm/cafs-types': specifier: workspace:*