fix!: only fetch the included files of a git-hosted dependency (#7638)

close #6512
close #7329

.changeset/great-parents-explode.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/store.cafs": major
+---
+
+Breaking change to addFileFromDir args.

.changeset/late-frogs-sit.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/tarball-fetcher": patch
+"@pnpm/git-fetcher": patch
+"@pnpm/worker": patch
+"pnpm": patch
+---
+
+When installing git-hosted dependencies, only pick the files that would be packed with the package [#7638](https://github.com/pnpm/pnpm/pull/7638).

fetching/git-fetcher/package.json

@@ -35,6 +35,7 @@
   },
   "dependencies": {
     "@pnpm/fetcher-base": "workspace:*",
+    "@pnpm/fs.packlist": "1.0.3",
     "@pnpm/prepare-package": "workspace:*",
     "@zkochan/rimraf": "^2.1.3",
     "execa": "npm:safe-execa@0.1.2"

fetching/git-fetcher/src/index.ts

@@ -1,5 +1,6 @@
 import path from 'path'
 import type { GitFetcher } from '@pnpm/fetcher-base'
+import { packlist } from '@pnpm/fs.packlist'
 import { globalWarn } from '@pnpm/logger'
 import { preparePackage } from '@pnpm/prepare-package'
 import { addFilesFromDir } from '@pnpm/worker'
@@ -46,12 +47,14 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions) {
     }
     // removing /.git to make directory integrity calculation faster
     await rimraf(path.join(tempLocation, '.git'))
+    const files = await packlist(pkgDir)
     // 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 addFilesFromDir({
       cafsDir: cafs.cafsDir,
       dir: pkgDir,
+      files,
       filesIndexFile: opts.filesIndexFile,
       readManifest: opts.readManifest,
       pkg: opts.pkg,

fetching/git-fetcher/test/index.ts

@@ -221,3 +221,24 @@ test('do not build the package when scripts are ignored', async () => {
 function prefixGitArgs (): string[] {
   return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : []
 }
+
+test('fetch only the included files', async () => {
+  const cafsDir = tempy.directory()
+  const fetch = createGitFetcher({ rawConfig: {} }).git
+  const { filesIndex } = await fetch(
+    createCafsStore(cafsDir),
+    {
+      commit: '958d6d487217512bb154d02836e9b5b922a600d8',
+      repo: 'https://github.com/pnpm-e2e/pkg-with-ignored-files',
+      type: 'git',
+    },
+    {
+      filesIndexFile: path.join(cafsDir, 'index.json'),
+    }
+  )
+  expect(Object.keys(filesIndex).sort()).toStrictEqual([
+    'README.md',
+    `dist${path.sep}index.js`,
+    'package.json',
+  ])
+})

fetching/tarball-fetcher/package.json

@@ -39,6 +39,7 @@
     "@pnpm/error": "workspace:*",
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/fetching-types": "workspace:*",
+    "@pnpm/fs.packlist": "1.0.3",
     "@pnpm/graceful-fs": "workspace:*",
     "@pnpm/prepare-package": "workspace:*",
     "@zkochan/retry": "^0.2.0",

fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs/promises'
 import { type FetchFunction, type FetchOptions } from '@pnpm/fetcher-base'
 import type { Cafs } from '@pnpm/cafs-types'
+import { packlist } from '@pnpm/fs.packlist'
 import { globalWarn } from '@pnpm/logger'
 import { preparePackage } from '@pnpm/prepare-package'
 import { type DependencyManifest } from '@pnpm/types'
@@ -70,7 +71,8 @@ async function prepareGitHostedPkg (
     force: true,
   })
   const { shouldBeBuilt, pkgDir } = await preparePackage(opts, tempLocation, resolution.path ?? '')
-  if (!resolution.path) {
+  const files = await packlist(pkgDir)
+  if (!resolution.path && files.length === Object.keys(filesIndex).length) {
     if (!shouldBeBuilt) {
       if (filesIndexFileNonBuilt !== filesIndexFile) {
         await renameOverwrite(filesIndexFileNonBuilt, filesIndexFile)
@@ -98,10 +100,11 @@ async function prepareGitHostedPkg (
     ...await addFilesFromDir({
       cafsDir: cafs.cafsDir,
       dir: pkgDir,
+      files,
       filesIndexFile,
       pkg: fetcherOpts.pkg,
       readManifest: fetcherOpts.readManifest,
     }),
-    ignoredBuild: false,
+    ignoredBuild: Boolean(opts.ignoreScripts),
   }
 }

fetching/tarball-fetcher/test/fetch.ts

@@ -448,6 +448,24 @@ test('fail when preparing a git-hosted package', async () => {
   ).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`')
 })
 
+test('take only the files included in the package, when fetching a git-hosted package', async () => {
+  process.chdir(tempy.directory())
+
+  const resolution = { tarball: 'https://codeload.github.com/pnpm-e2e/pkg-with-ignored-files/tar.gz/958d6d487217512bb154d02836e9b5b922a600d8' }
+
+  const result = await fetch.gitHostedTarball(cafs, resolution, {
+    filesIndexFile,
+    lockfileDir: process.cwd(),
+    pkg: {},
+  })
+
+  expect(Object.keys(result.filesIndex).sort()).toStrictEqual([
+    'README.md',
+    `dist${path.sep}index.js`,
+    'package.json',
+  ])
+})
+
 test('fail when extracting a broken tarball', async () => {
   const scope = nock(registry)
     .get('/foo.tgz')

pnpm-lock.yaml

@@ -1592,6 +1592,9 @@ importers:
       '@pnpm/fetcher-base':
         specifier: workspace:*
         version: link:../fetcher-base
+      '@pnpm/fs.packlist':
+        specifier: 1.0.3
+        version: 1.0.3
       '@pnpm/logger':
         specifier: ^5.0.0
         version: 5.0.0
@@ -1650,6 +1653,9 @@ importers:
       '@pnpm/fetching-types':
         specifier: workspace:*
         version: link:../../network/fetching-types
+      '@pnpm/fs.packlist':
+        specifier: 1.0.3
+        version: 1.0.3
       '@pnpm/graceful-fs':
         specifier: workspace:*
         version: link:../../fs/graceful-fs
@@ -8440,6 +8446,13 @@ packages:
       p-filter: 2.1.0
     dev: true
 
+  /@pnpm/fs.packlist@1.0.3:
+    resolution: {integrity: sha512-fIf4V48oB6+iTghJOzgtJmwOoSEcgMt4wl819zaoGTUZQFpWWuvAaqhNVgE5dUP3Ee5WHXQVyVaPMl+1jRrXrQ==}
+    engines: {node: '>=16.14'}
+    dependencies:
+      npm-packlist: 5.1.3
+    dev: false
+
   /@pnpm/git-utils@1.0.0:
     resolution: {integrity: sha512-lUI+XrzOJN4zdPGOGnFUrmtXAXpXi8wD8OI0nWOZmlh+raqbLzC3VkXu1zgaduOK6YonOcnQW88O+ojav1rAdA==}
     engines: {node: '>=16.14'}
@@ -18292,6 +18305,7 @@ time:
   /@pnpm/colorize-semver-diff@1.0.1: '2020-10-25T15:50:17.812Z'
   /@pnpm/config.env-replace@1.1.0: '2023-04-04T18:59:45.025Z'
   /@pnpm/exec@2.0.0: '2020-10-29T23:51:01.271Z'
+  /@pnpm/fs.packlist@1.0.3: '2024-01-22T09:17:00.735Z'
   /@pnpm/hosted-git-info@1.0.0: '2024-02-05T14:40:06.830Z'
   /@pnpm/log.group@1.0.1: '2023-10-21T01:56:15.610Z'
   /@pnpm/logger@5.0.0: '2022-10-14T13:56:04.285Z'

store/cafs/src/addFilesFromDir.ts

@@ -12,49 +12,57 @@ import { parseJsonBufferSync } from './parseJson'
 export function addFilesFromDir (
   addBuffer: (buffer: Buffer, mode: number) => FileWriteResult,
   dirname: string,
-  readManifest?: boolean
+  opts: {
+    files?: string[]
+    readManifest?: boolean
+  } = {}
 ): AddToStoreResult {
   const filesIndex: FilesIndex = {}
-  const manifest = _retrieveFileIntegrities(addBuffer, dirname, dirname, filesIndex, readManifest)
-  return { filesIndex, manifest }
-}
-
-function _retrieveFileIntegrities (
-  addBuffer: (buffer: Buffer, mode: number) => FileWriteResult,
-  rootDir: string,
-  currDir: string,
-  index: FilesIndex,
-  readManifest?: boolean
-) {
-  const files = fs.readdirSync(currDir, { withFileTypes: true })
   let manifest: DependencyManifest | undefined
+  const files = opts.files ?? findFilesInDir(dirname)
   for (const file of files) {
-    const fullPath = path.join(currDir, file.name)
-    if (file.isDirectory()) {
-      _retrieveFileIntegrities(addBuffer, rootDir, fullPath, index)
+    const fullPath = path.join(dirname, file)
+    const relativePath = path.relative(dirname, fullPath)
+    let stat: Stats
+    try {
+      stat = fs.statSync(fullPath)
+    } catch (err: any) { // eslint-disable-line
+      if (err.code !== 'ENOENT') {
+        throw err
+      }
       continue
     }
-    if (file.isFile()) {
-      const relativePath = path.relative(rootDir, fullPath)
-      let stat: Stats
-      try {
-        stat = fs.statSync(fullPath)
-      } catch (err: any) { // eslint-disable-line
-        if (err.code !== 'ENOENT') {
-          throw err
-        }
-        continue
-      }
-      const buffer = gfs.readFileSync(fullPath)
-      if (rootDir === currDir && readManifest && file.name === 'package.json') {
-        manifest = parseJsonBufferSync(buffer)
-      }
-      index[relativePath] = {
-        mode: stat.mode,
-        size: stat.size,
-        ...addBuffer(buffer, stat.mode),
-      }
+    const buffer = gfs.readFileSync(fullPath)
+    if (opts.readManifest && file === 'package.json') {
+      manifest = parseJsonBufferSync(buffer)
+    }
+    filesIndex[relativePath] = {
+      mode: stat.mode,
+      size: stat.size,
+      ...addBuffer(buffer, stat.mode),
+    }
+  }
+  return { manifest, filesIndex }
+}
+
+function findFilesInDir (dir: string): string[] {
+  const files: string[] = []
+  findFiles(files, dir)
+  return files
+}
+
+function findFiles (
+  filesList: string[],
+  dir: string,
+  relativeDir = ''
+) {
+  const files = fs.readdirSync(dir, { withFileTypes: true })
+  for (const file of files) {
+    const relativeSubdir = `${relativeDir}${relativeDir ? '/' : ''}${file.name}`
+    if (file.isDirectory()) {
+      findFiles(filesList, path.join(dir, file.name), relativeSubdir)
+    } else {
+      filesList.push(relativeSubdir)
     }
   }
-  return manifest
 }

worker/src/index.ts

@@ -49,7 +49,7 @@ function createTarballWorkerPool (): WorkerPool {
 }
 
 export async function addFilesFromDir (
-  opts: Pick<AddDirToStoreMessage, 'cafsDir' | 'dir' | 'filesIndexFile' | 'sideEffectsCacheKey' | 'readManifest' | 'pkg'>
+  opts: Pick<AddDirToStoreMessage, 'cafsDir' | 'dir' | 'filesIndexFile' | 'sideEffectsCacheKey' | 'readManifest' | 'pkg' | 'files'>
 ) {
   if (!workerPool) {
     workerPool = createTarballWorkerPool()
@@ -72,6 +72,7 @@ export async function addFilesFromDir (
       sideEffectsCacheKey: opts.sideEffectsCacheKey,
       readManifest: opts.readManifest,
       pkg: opts.pkg,
+      files: opts.files,
     })
   })
 }

worker/src/types.ts

@@ -45,6 +45,7 @@ export interface AddDirToStoreMessage {
   sideEffectsCacheKey?: string
   readManifest?: boolean
   pkg?: PkgNameVersion
+  files?: string[]
 }
 
 export interface ReadPkgFromCafsMessage {

worker/src/worker.ts

@@ -136,12 +136,12 @@ function addTarballToStore ({ buffer, cafsDir, integrity, filesIndexFile, pkg, r
   return { status: 'success', value: { filesIndex: filesMap, manifest } }
 }
 
-function addFilesFromDir ({ dir, cafsDir, filesIndexFile, sideEffectsCacheKey, pkg, readManifest }: AddDirToStoreMessage) {
+function addFilesFromDir ({ dir, cafsDir, filesIndexFile, sideEffectsCacheKey, pkg, readManifest, files }: AddDirToStoreMessage) {
   if (!cafsCache.has(cafsDir)) {
     cafsCache.set(cafsDir, createCafs(cafsDir))
   }
   const cafs = cafsCache.get(cafsDir)!
-  const { filesIndex, manifest } = cafs.addFilesFromDir(dir, readManifest)
+  const { filesIndex, manifest } = cafs.addFilesFromDir(dir, { files, readManifest })
   const { filesIntegrity, filesMap } = processFilesIndex(filesIndex)
   if (sideEffectsCacheKey) {
     let filesIndex!: PackageFilesIndex