fix: don't build git-hosted dependencies when scripts are ignored (#5897

)

close #5876

.changeset/odd-ligers-fold.md

@@ -0,0 +1,14 @@
+---
+"@pnpm/resolve-dependencies": minor
+"@pnpm/store-connection-manager": minor
+"@pnpm/package-requester": minor
+"@pnpm/store-controller-types": minor
+"@pnpm/tarball-fetcher": minor
+"@pnpm/prepare-package": minor
+"@pnpm/git-fetcher": minor
+"@pnpm/headless": patch
+"@pnpm/client": minor
+"@pnpm/core": patch
+---
+
+When ignoreScripts=true is passed to the fetcher, do not build git-hosted dependencies.

.changeset/seven-seahorses-hug.md

@@ -0,0 +1,5 @@
+---
+"pnpm": patch
+---
+
+Git-hosted dependencies should not be built, when `ignore-scripts` is set to `true` [#5876](https://github.com/pnpm/pnpm/issues/5876).

.changeset/stupid-lizards-stare.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/git-fetcher": major
+---
+
+Added `@pnpm/logger` to the peer dependencies.

exec/plugin-commands-rebuild/test/index.ts

@@ -68,11 +68,6 @@ test('rebuilds dependencies', async () => {
       'preinstall',
       'install',
       'postinstall',
-      'prepare',
-      'prepublishOnly',
-      'preinstall',
-      'install',
-      'postinstall',
     ])
   }
 })

exec/prepare-package/src/index.ts

@@ -16,13 +16,15 @@ const PREPUBLISH_SCRIPTS = [
 ]
 
 export interface PreparePackageOptions {
+  ignoreScripts?: boolean
   rawConfig: object
   unsafePerm?: boolean
 }
 
-export async function preparePackage (opts: PreparePackageOptions, pkgDir: string) {
+export async function preparePackage (opts: PreparePackageOptions, pkgDir: string): Promise<boolean> {
   const manifest = await safeReadPackageJsonFromDir(pkgDir)
-  if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return
+  if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return false
+  if (opts.ignoreScripts) return true
   const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
   const execOpts: RunLifecycleHookOptions = {
     depPath: `${manifest.name}@${manifest.version}`,
@@ -46,6 +48,7 @@ export async function preparePackage (opts: PreparePackageOptions, pkgDir: strin
     throw err
   }
   await rimraf(path.join(pkgDir, 'node_modules'))
+  return true
 }
 
 function packageShouldBeBuilt (manifest: PackageManifest, pkgDir: string): boolean {

fetching/git-fetcher/package.json

@@ -29,6 +29,9 @@
     "url": "https://github.com/pnpm/pnpm/issues"
   },
   "homepage": "https://github.com/pnpm/pnpm/blob/main/fetching/git-fetcher#readme",
+  "peerDependencies": {
+    "@pnpm/logger": "^5.0.0"
+  },
   "dependencies": {
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/prepare-package": "workspace:*",

fetching/git-fetcher/src/index.ts

@@ -1,5 +1,6 @@
 import path from 'path'
 import type { GitFetcher } from '@pnpm/fetcher-base'
+import { globalWarn } from '@pnpm/logger'
 import { preparePackage } from '@pnpm/prepare-package'
 import rimraf from '@zkochan/rimraf'
 import execa from 'execa'
@@ -9,11 +10,17 @@ export interface CreateGitFetcherOptions {
   gitShallowHosts?: string[]
   rawConfig: object
   unsafePerm?: boolean
+  ignoreScripts?: boolean
 }
 
 export function createGitFetcher (createOpts: CreateGitFetcherOptions) {
   const allowedHosts = new Set(createOpts?.gitShallowHosts ?? [])
-  const preparePkg = preparePackage.bind(null, { rawConfig: createOpts.rawConfig, unsafePerm: createOpts.unsafePerm })
+  const ignoreScripts = createOpts.ignoreScripts ?? false
+  const preparePkg = preparePackage.bind(null, {
+    ignoreScripts: createOpts.ignoreScripts,
+    rawConfig: createOpts.rawConfig,
+    unsafePerm: createOpts.unsafePerm,
+  })
 
   const gitFetcher: GitFetcher = async (cafs, resolution, opts) => {
     const tempLocation = await cafs.tempDir()
@@ -26,7 +33,10 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions) {
     }
     await execGit(['checkout', resolution.commit], { cwd: tempLocation })
     try {
-      await preparePkg(tempLocation)
+      const shouldBeBuilt = await preparePkg(tempLocation)
+      if (ignoreScripts && shouldBeBuilt) {
+        globalWarn(`The git-hosted package fetched from "${resolution.repo}" has to be built but the build scripts were ignored.`)
+      }
     } catch (err: any) { // eslint-disable-line
       err.message = `Failed to prepare git-hosted package fetched from "${resolution.repo}": ${err.message}` // eslint-disable-line
       throw err

fetching/git-fetcher/test/index.ts

@@ -2,6 +2,7 @@
 import path from 'path'
 import { createCafsStore } from '@pnpm/create-cafs-store'
 import { createGitFetcher } from '@pnpm/git-fetcher'
+import { globalWarn } from '@pnpm/logger'
 import { DependencyManifest } from '@pnpm/types'
 import pDefer from 'p-defer'
 import tempy from 'tempy'
@@ -16,8 +17,17 @@ jest.mock('execa', () => {
   }
 })
 
+jest.mock('@pnpm/logger', () => {
+  const originalModule = jest.requireActual('@pnpm/logger')
+  return {
+    ...originalModule,
+    globalWarn: jest.fn(),
+  }
+})
+
 beforeEach(() => {
-  (execa as jest.Mock).mockClear()
+  ;(execa as jest.Mock).mockClear()
+  ;(globalWarn as jest.Mock).mockClear()
 })
 
 test('fetch', async () => {
@@ -138,6 +148,21 @@ test('fail when preparing a git-hosted package', async () => {
   ).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`')
 })
 
+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<DependencyManifest>()
+  const { filesIndex } = await fetch(createCafsStore(cafsDir),
+    {
+      commit: '55416a9c468806a935636c0ad0371a14a64df8c9',
+      repo: 'https://github.com/pnpm-e2e/prepare-script-works.git',
+      type: 'git',
+    }, { manifest })
+  expect(filesIndex['package.json']).toBeTruthy()
+  expect(filesIndex['prepare.txt']).toBeFalsy()
+  expect(globalWarn).toHaveBeenCalledWith('The git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-works.git" has to be built but the build scripts were ignored.')
+})
+
 function prefixGitArgs (): string[] {
   return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : []
 }

fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts

@@ -1,5 +1,6 @@
 import { FetchFunction, FetchOptions } from '@pnpm/fetcher-base'
 import type { Cafs, FilesIndex, PackageFileInfo } from '@pnpm/cafs-types'
+import { globalWarn } from '@pnpm/logger'
 import { preparePackage } from '@pnpm/prepare-package'
 import pMapValues from 'p-map-values'
 import omit from 'ramda/src/omit'
@@ -11,6 +12,7 @@ interface Resolution {
 }
 
 export interface CreateGitHostedTarballFetcher {
+  ignoreScripts?: boolean
   rawConfig: object
   unsafePerm?: boolean
 }
@@ -19,7 +21,11 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction
   const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
     const { filesIndex } = await fetchRemoteTarball(cafs, resolution, opts)
     try {
-      return { filesIndex: await prepareGitHostedPkg(filesIndex as FilesIndex, cafs, fetcherOpts) }
+      const prepareResult = await prepareGitHostedPkg(filesIndex as FilesIndex, cafs, fetcherOpts)
+      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 }
     } catch (err: any) { // eslint-disable-line
       err.message = `Failed to prepare git-hosted package fetched from "${resolution.tarball}": ${err.message}` // eslint-disable-line
       throw err
@@ -38,12 +44,15 @@ async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs, opts: Cr
     },
     force: true,
   })
-  await preparePackage(opts, tempLocation)
+  const shouldBeBuilt = await preparePackage(opts, 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
+  return {
+    filesIndex: newFilesIndex,
+    ignoredBuild: opts.ignoreScripts && shouldBeBuilt,
+  }
 }
 
 export async function waitForFilesIndex (filesIndex: FilesIndex): Promise<Record<string, PackageFileInfo>> {

fetching/tarball-fetcher/src/index.ts

@@ -33,6 +33,7 @@ export function createTarballFetcher (
   opts: {
     rawConfig: object
     unsafePerm?: boolean
+    ignoreScripts?: boolean
     timeout?: number
     retry?: RetryTimeoutOptions
     offline?: boolean

fetching/tarball-fetcher/test/fetch.ts

@@ -4,6 +4,7 @@ import path from 'path'
 import { FetchError, PnpmError } from '@pnpm/error'
 import { createFetchFromRegistry } from '@pnpm/fetch'
 import { createCafsStore } from '@pnpm/create-cafs-store'
+import { globalWarn } from '@pnpm/logger'
 import { fixtures } from '@pnpm/test-fixtures'
 import {
   createTarballFetcher,
@@ -14,6 +15,18 @@ import nock from 'nock'
 import ssri from 'ssri'
 import tempy from 'tempy'
 
+jest.mock('@pnpm/logger', () => {
+  const originalModule = jest.requireActual('@pnpm/logger')
+  return {
+    ...originalModule,
+    globalWarn: jest.fn(),
+  }
+})
+
+beforeEach(() => {
+  ;(globalWarn as jest.Mock).mockClear()
+})
+
 const cafsDir = tempy.directory()
 const cafs = createCafsStore(cafsDir)
 
@@ -394,3 +407,25 @@ test('fail when extracting a broken tarball', async () => {
   )
   expect(scope.isDone()).toBeTruthy()
 })
+
+test('do not build the package when scripts are ignored', async () => {
+  process.chdir(tempy.directory())
+
+  const tarball = 'https://codeload.github.com/pnpm-e2e/prepare-script-works/tar.gz/55416a9c468806a935636c0ad0371a14a64df8c9'
+  const resolution = { tarball }
+
+  const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
+    ignoreScripts: true,
+    rawConfig: {},
+    retry: {
+      maxTimeout: 100,
+      minTimeout: 0,
+      retries: 1,
+    },
+  })
+  const { filesIndex } = await fetch.gitHostedTarball(cafs, resolution, { lockfileDir: process.cwd() })
+
+  expect(filesIndex).toHaveProperty(['package.json'])
+  expect(filesIndex).not.toHaveProperty(['prepare.txt'])
+  expect(globalWarn).toHaveBeenCalledWith(`The git-hosted package fetched from "${tarball}" has to be built but the build scripts were ignored.`)
+})

pkg-manager/client/src/index.ts

@@ -17,6 +17,7 @@ export { ResolveFunction }
 export type ClientOptions = {
   authConfig: Record<string, string>
   customFetchers?: CustomFetchers
+  ignoreScripts?: boolean
   rawConfig: object
   retry?: RetryTimeoutOptions
   timeout?: number

pkg-manager/core/src/install/index.ts

@@ -813,6 +813,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
       engineStrict: opts.engineStrict,
       force: opts.force,
       forceFullResolution,
+      ignoreScripts: opts.ignoreScripts,
       hooks: {
         readPackage: opts.readPackageHook,
       },

pkg-manager/core/test/install/fromRepo.ts

@@ -5,6 +5,7 @@ import {
   addDependenciesToPackage,
   install,
 } from '@pnpm/core'
+import rimraf from '@zkochan/rimraf'
 import { isCI } from 'ci-info'
 import exists from 'path-exists'
 import sinon from 'sinon'
@@ -173,3 +174,47 @@ test('from a github repo the has no package.json file', async () => {
     'for-testing.no-package-json': 'github:pnpm/for-testing.no-package-json',
   })
 })
+
+test('from a github repo that needs to be built. isolated node linker is used', async () => {
+  const project = prepareEmpty()
+
+  const manifest = await addDependenciesToPackage({}, ['pnpm-e2e/prepare-script-works'], await testDefaults({ ignoreScripts: true }, { ignoreScripts: true }))
+
+  await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ preferFrozenLockfile: false }))
+  await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ frozenLockfile: true }))
+  await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ frozenLockfile: true, ignoreScripts: true }, { ignoreScripts: true }))
+  await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
+})
+
+test('from a github repo that needs to be built. hoisted node linker is  used', async () => {
+  const project = prepareEmpty()
+
+  const manifest = await addDependenciesToPackage(
+    {},
+    ['pnpm-e2e/prepare-script-works'],
+    await testDefaults({ ignoreScripts: true, nodeLinker: 'hoisted' }, { ignoreScripts: true })
+  )
+
+  await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ preferFrozenLockfile: false, nodeLinker: 'hoisted' }))
+  await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ frozenLockfile: true, nodeLinker: 'hoisted' }))
+  await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ frozenLockfile: true, ignoreScripts: true, nodeLinker: 'hoisted' }, { ignoreScripts: true }))
+  await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
+})

pkg-manager/headless/src/lockfileToDepGraph.ts

@@ -56,6 +56,7 @@ export interface LockfileToDepGraphOptions {
   force: boolean
   importerIds: string[]
   include: IncludedDependencies
+  ignoreScripts: boolean
   lockfileDir: string
   nodeVersion: string
   pnpmVersion: string
@@ -148,6 +149,7 @@ export async function lockfileToDepGraph (
           fetchResponse = opts.storeController.fetchPackage({
             force: false,
             lockfileDir: opts.lockfileDir,
+            ignoreScripts: opts.ignoreScripts,
             pkg: {
               id: packageId,
               resolution,

pkg-manager/headless/src/lockfileToHoistedDepGraph.ts

@@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions {
   externalDependencies?: Set<string>
   importerIds: string[]
   include: IncludedDependencies
+  ignoreScripts: boolean
   currentHoistedLocations?: Record<string, string[]>
   lockfileDir: string
   nodeVersion: string
@@ -188,6 +189,7 @@ async function fetchDeps (
         fetchResponse = opts.storeController.fetchPackage({
           force: false,
           lockfileDir: opts.lockfileDir,
+          ignoreScripts: opts.ignoreScripts,
           pkg: {
             id: packageId,
             resolution,

pkg-manager/package-requester/src/packageRequester.ts

@@ -257,6 +257,7 @@ async function resolveAndFetch (
   const fetchResult = ctx.fetchPackageToStore({
     fetchRawManifest: true,
     force: forceFetch,
+    ignoreScripts: options.ignoreScripts,
     lockfileDir: options.lockfileDir,
     pkg: {
       ...pkg,
@@ -331,7 +332,7 @@ function fetchToStore (
     const finishing = pDefer<undefined>()
     const filesIndexFile = opts.pkg.resolution['integrity']
       ? ctx.getFilePathInCafs(opts.pkg.resolution['integrity'], 'index')
-      : path.join(target, 'integrity.json')
+      : path.join(target, opts.ignoreScripts ? 'integrity-not-built.json' : 'integrity.json')
 
     doFetchToStore(filesIndexFile, bundledManifest, files, finishing) // eslint-disable-line