feat!: remove requiresBuild from the lockfile (#7710)

close #7707

.changeset/happy-bags-reply.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/exec.pkg-requires-build": major
+---
+
+Initial release.

deps/graph-builder/src/lockfileToDepGraph.ts

@@ -41,8 +41,7 @@ export interface DependenciesGraphNode {
   optional: boolean
   depPath: string // this option is only needed for saving pendingBuild when running with --ignore-scripts flag
   isBuilt?: boolean
-  requiresBuild: boolean
-  prepare: boolean
+  requiresBuild?: boolean
   hasBin: boolean
   filesIndexFile?: string
   patchFile?: PatchFile
@@ -195,8 +194,6 @@ export async function lockfileToDepGraph (
           name: pkgName,
           optional: !!pkgSnapshot.optional,
           optionalDependencies: new Set(Object.keys(pkgSnapshot.optionalDependencies ?? {})),
-          prepare: pkgSnapshot.prepare === true,
-          requiresBuild: pkgSnapshot.requiresBuild === true,
           patchFile: opts.patchedDependencies?.[`${pkgName}@${pkgVersion}`],
         }
         pkgSnapshotByLocation[dir] = pkgSnapshot

env/node.fetcher/src/index.ts

@@ -50,6 +50,7 @@ export async function fetchNode (fetch: FetchFromRegistry, version: string, targ
     filesResponse: {
       filesIndex: filesIndex as Record<string, string>,
       resolvedFrom: 'remote',
+      requiresBuild: false,
     },
     force: true,
   })

exec/pkg-requires-build/README.md

@@ -0,0 +1,15 @@
+# @pnpm/exec.pkg-requires-build
+
+> Checks if a package requires to be built
+
+[![npm version](https://img.shields.io/npm/v/@pnpm/exec.pkg-requires-build.svg)](https://www.npmjs.com/package/@pnpm/exec.pkg-requires-build)
+
+## Installation
+
+```sh
+pnpm add @pnpm/exec.pkg-requires-build
+```
+
+## License
+
+MIT

exec/files-include-install-scripts/package.json → exec/pkg-requires-build/package.json

@@ -1,7 +1,7 @@
 {
-  "name": "@pnpm/exec.files-include-install-scripts",
-  "version": "1.0.0",
-  "description": "Checks if a package has files indicating that it needs to be built during installation",
+  "name": "@pnpm/exec.pkg-requires-build",
+  "version": "0.0.0",
+  "description": "Checks if a package requires to be built",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",
   "files": [
@@ -18,7 +18,7 @@
     "fix": "tslint -c tslint.json src/**/*.ts test/**/*.ts --fix",
     "compile": "tsc --build && pnpm run lint --fix"
   },
-  "repository": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts",
+  "repository": "https://github.com/pnpm/pnpm/blob/main/exec/pkg-requires-build",
   "keywords": [
     "pnpm9",
     "pnpm"
@@ -27,9 +27,12 @@
   "bugs": {
     "url": "https://github.com/pnpm/pnpm/issues"
   },
-  "homepage": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts#readme",
+  "homepage": "https://github.com/pnpm/pnpm/blob/main/exec/pkg-requires-build#readme",
+  "dependencies": {
+    "@pnpm/types": "workspace:*"
+  },
   "devDependencies": {
-    "@pnpm/exec.files-include-install-scripts": "workspace:*"
+    "@pnpm/exec.pkg-requires-build": "workspace:*"
   },
   "funding": "https://opencollective.com/pnpm",
   "exports": {

exec/pkg-requires-build/src/index.ts

@@ -0,0 +1,17 @@
+import { type DependencyManifest } from '@pnpm/types'
+
+export function pkgRequiresBuild (manifest: Partial<DependencyManifest> | undefined, filesIndex: Record<string, unknown>) {
+  return Boolean(
+    manifest?.scripts != null && (
+      Boolean(manifest.scripts.preinstall) ||
+      Boolean(manifest.scripts.install) ||
+      Boolean(manifest.scripts.postinstall)
+    ) ||
+    filesIncludeInstallScripts(filesIndex)
+  )
+}
+
+function filesIncludeInstallScripts (filesIndex: Record<string, unknown>): boolean {
+  return filesIndex['binding.gyp'] != null ||
+    Object.keys(filesIndex).some((filename) => !(filename.match(/^[.]hooks[\\/]/) == null)) // TODO: optimize this
+}

exec/files-include-install-scripts/tsconfig.json → exec/pkg-requires-build/tsconfig.json

@@ -8,6 +8,10 @@
     "src/**/*.ts",
     "../../__typings__/**/*.d.ts"
   ],
-  "references": [],
+  "references": [
+    {
+      "path": "../../packages/types"
+    }
+  ],
   "composite": true
 }

fetching/directory-fetcher/package.json

@@ -34,6 +34,7 @@
     "@pnpm/logger": "^5.0.0"
   },
   "dependencies": {
+    "@pnpm/exec.pkg-requires-build": "workspace:*",
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/fs.packlist": "workspace:*",
     "@pnpm/read-project-manifest": "workspace:*",

fetching/directory-fetcher/src/index.ts

@@ -1,5 +1,6 @@
 import { promises as fs, type Stats } from 'fs'
 import path from 'path'
+import { pkgRequiresBuild } from '@pnpm/exec.pkg-requires-build'
 import type { DirectoryFetcher, DirectoryFetcherOptions } from '@pnpm/fetcher-base'
 import { logger } from '@pnpm/logger'
 import { packlist } from '@pnpm/fs.packlist'
@@ -21,7 +22,7 @@ export function createDirectoryFetcher (
 
   const directoryFetcher: DirectoryFetcher = (cafs, resolution, opts) => {
     const dir = path.join(opts.lockfileDir, resolution.directory)
-    return fetchFromDir(dir, opts)
+    return fetchFromDir(dir)
   }
 
   return {
@@ -36,30 +37,28 @@ export async function fetchFromDir (
   opts: FetchFromDirOpts & CreateDirectoryFetcherOptions
 ) {
   if (opts.includeOnlyPackageFiles) {
-    return fetchPackageFilesFromDir(dir, opts)
+    return fetchPackageFilesFromDir(dir)
   }
   const readFileStat: ReadFileStat = opts?.resolveSymlinks === true ? realFileStat : fileStat
-  return fetchAllFilesFromDir(readFileStat, dir, opts)
+  return fetchAllFilesFromDir(readFileStat, dir)
 }
 
 async function fetchAllFilesFromDir (
   readFileStat: ReadFileStat,
-  dir: string,
-  opts: FetchFromDirOpts
+  dir: string
 ) {
   const filesIndex = await _fetchAllFilesFromDir(readFileStat, dir)
-  let manifest: DependencyManifest | undefined
-  if (opts.readManifest) {
-    // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
-    // Safe read was added to support the Bit workspace in which the components have no package.json files.
-    // Related PR in Bit: https://github.com/teambit/bit/pull/5251
-    manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
-  }
+  // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
+  // Safe read was added to support the Bit workspace in which the components have no package.json files.
+  // Related PR in Bit: https://github.com/teambit/bit/pull/5251
+  const manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
+  const requiresBuild = pkgRequiresBuild(manifest, filesIndex)
   return {
     local: true as const,
     filesIndex,
     packageImportMethod: 'hardlink' as const,
     manifest,
+    requiresBuild,
   }
 }
 
@@ -124,23 +123,19 @@ async function fileStat (filePath: string): Promise<{ filePath: string, stat: St
   }
 }
 
-async function fetchPackageFilesFromDir (
-  dir: string,
-  opts: FetchFromDirOpts
-) {
+async function fetchPackageFilesFromDir (dir: string) {
   const files = await packlist(dir)
   const filesIndex: Record<string, string> = Object.fromEntries(files.map((file) => [file, path.join(dir, file)]))
-  let manifest: DependencyManifest | undefined
-  if (opts.readManifest) {
-    // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
-    // Safe read was added to support the Bit workspace in which the components have no package.json files.
-    // Related PR in Bit: https://github.com/teambit/bit/pull/5251
-    manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
-  }
+  // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
+  // Safe read was added to support the Bit workspace in which the components have no package.json files.
+  // Related PR in Bit: https://github.com/teambit/bit/pull/5251
+  const manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
+  const requiresBuild = pkgRequiresBuild(manifest, filesIndex)
   return {
     local: true as const,
     filesIndex,
     packageImportMethod: 'hardlink' as const,
     manifest,
+    requiresBuild,
   }
 }

fetching/directory-fetcher/tsconfig.json

@@ -12,6 +12,9 @@
     {
       "path": "../../__utils__/test-fixtures"
     },
+    {
+      "path": "../../exec/pkg-requires-build"
+    },
     {
       "path": "../../fs/packlist"
     },

fetching/fetcher-base/src/index.ts

@@ -26,6 +26,7 @@ export interface FetchResult {
   local?: boolean
   manifest?: DependencyManifest
   filesIndex: Record<string, string>
+  requiresBuild: boolean
 }
 
 export interface GitFetcherOptions {
@@ -34,7 +35,13 @@ export interface GitFetcherOptions {
   pkg?: PkgNameVersion
 }
 
-export type GitFetcher = FetchFunction<GitResolution, GitFetcherOptions, { filesIndex: Record<string, string>, manifest?: DependencyManifest }>
+export interface GitFetcherResult {
+  filesIndex: Record<string, string>
+  manifest?: DependencyManifest
+  requiresBuild: boolean
+}
+
+export type GitFetcher = FetchFunction<GitResolution, GitFetcherOptions, GitFetcherResult>
 
 export interface DirectoryFetcherOptions {
   lockfileDir: string
@@ -46,6 +53,7 @@ export interface DirectoryFetcherResult {
   filesIndex: Record<string, string>
   packageImportMethod: 'hardlink'
   manifest?: DependencyManifest
+  requiresBuild: boolean
 }
 
 export type DirectoryFetcher = FetchFunction<DirectoryResolution, DirectoryFetcherOptions, DirectoryFetcherResult>

fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts

@@ -25,7 +25,7 @@ export interface CreateGitHostedTarballFetcher {
 export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction, fetcherOpts: CreateGitHostedTarballFetcher): FetchFunction {
   const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
     const tempIndexFile = pathTemp(opts.filesIndexFile)
-    const { filesIndex, manifest } = await fetchRemoteTarball(cafs, resolution, {
+    const { filesIndex, manifest, requiresBuild } = await fetchRemoteTarball(cafs, resolution, {
       ...opts,
       filesIndexFile: tempIndexFile,
     })
@@ -37,6 +37,7 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction
       return {
         filesIndex: prepareResult.filesIndex,
         manifest: prepareResult.manifest ?? manifest,
+        requiresBuild,
       }
     } catch (err: any) { // eslint-disable-line
       err.message = `Failed to prepare git-hosted package fetched from "${resolution.tarball}": ${err.message}`
@@ -67,6 +68,7 @@ async function prepareGitHostedPkg (
     filesResponse: {
       filesIndex,
       resolvedFrom: 'remote',
+      requiresBuild: false,
     },
     force: true,
   })

lockfile/lockfile-file/src/lockfileFormatConverters.ts

@@ -74,16 +74,6 @@ function normalizeLockfile (lockfile: InlineSpecifiersLockfile, opts: NormalizeL
   if ((lockfileToSave.patchedDependencies != null) && isEmpty(lockfileToSave.patchedDependencies)) {
     delete lockfileToSave.patchedDependencies
   }
-  if (lockfileToSave.neverBuiltDependencies != null) {
-    if (isEmpty(lockfileToSave.neverBuiltDependencies)) {
-      delete lockfileToSave.neverBuiltDependencies
-    } else {
-      lockfileToSave.neverBuiltDependencies = lockfileToSave.neverBuiltDependencies.sort()
-    }
-  }
-  if (lockfileToSave.onlyBuiltDependencies != null) {
-    lockfileToSave.onlyBuiltDependencies = lockfileToSave.onlyBuiltDependencies.sort()
-  }
   if (!lockfileToSave.packageExtensionsChecksum) {
     delete lockfileToSave.packageExtensionsChecksum
   }

lockfile/lockfile-file/src/sortLockfileKeys.ts

@@ -35,8 +35,6 @@ type RootKey = keyof LockfileFile
 const ROOT_KEYS: readonly RootKey[] = [
   'lockfileVersion',
   'settings',
-  'neverBuiltDependencies',
-  'onlyBuiltDependencies',
   'overrides',
   'packageExtensionsChecksum',
   'pnpmfileChecksum',

lockfile/lockfile-file/test/normalizeLockfile.test.ts

@@ -4,10 +4,7 @@ import { convertToLockfileFile } from '../lib/lockfileFormatConverters'
 test('empty overrides and neverBuiltDependencies are removed during lockfile normalization', () => {
   expect(convertToLockfileFile({
     lockfileVersion: LOCKFILE_VERSION,
-    // but this should be preserved.
-    onlyBuiltDependencies: [],
     overrides: {},
-    neverBuiltDependencies: [],
     patchedDependencies: {},
     packages: {},
     importers: {
@@ -24,7 +21,6 @@ test('empty overrides and neverBuiltDependencies are removed during lockfile nor
     forceSharedFormat: false,
   })).toStrictEqual({
     lockfileVersion: LOCKFILE_VERSION,
-    onlyBuiltDependencies: [],
     importers: {
       foo: {
         dependencies: {

lockfile/lockfile-types/src/index.ts

@@ -14,8 +14,6 @@ export interface Lockfile {
   lockfileVersion: number | string
   time?: Record<string, string>
   packages?: PackageSnapshots
-  neverBuiltDependencies?: string[]
-  onlyBuiltDependencies?: string[]
   overrides?: Record<string, string>
   packageExtensionsChecksum?: string
   patchedDependencies?: Record<string, PatchFile>
@@ -77,9 +75,7 @@ export interface PackageSnapshot {
   id?: string
   dev?: true | false
   optional?: true
-  requiresBuild?: true
   patched?: true
-  prepare?: true
   hasBin?: true
   // name and version are only needed
   // for packages that are hosted not in the npm registry