fix: building git-hosted dependencies (#7407)

.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).

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:*",

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<string, string>, cafs, opts.filesIndexFile, fetcherOpts, opts)
+      const prepareResult = await prepareGitHostedPkg(filesIndex as Record<string, string>, 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<string, string>,
   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,
   }
 }

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()
+})

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:*