fix(directory-fetcher): resolve the injected files to their real locations (#5637)

.changeset/good-seals-reflect.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/client": minor
+"@pnpm/directory-fetcher": minor
+"@pnpm/store-connection-manager": minor
+---
+
+New option added to resolve symlinks to their real locations, when injecting directories.

packages/client/src/index.ts

@@ -21,6 +21,7 @@ export type ClientOptions = {
   userAgent?: string
   userConfig?: Record<string, string>
   gitShallowHosts?: string[]
+  resolveSymlinksInInjectedDirs?: boolean
 } & ResolverFactoryOptions & AgentOptions
 
 export interface Client {
@@ -51,13 +52,13 @@ type Fetchers = {
 function createFetchers (
   fetchFromRegistry: FetchFromRegistry,
   getAuthHeader: GetAuthHeader,
-  opts: Pick<ClientOptions, 'retry' | 'gitShallowHosts'>,
+  opts: Pick<ClientOptions, 'retry' | 'gitShallowHosts' | 'resolveSymlinksInInjectedDirs'>,
   customFetchers?: CustomFetchers
 ): Fetchers {
   const defaultFetchers = {
     ...createTarballFetcher(fetchFromRegistry, getAuthHeader, opts),
     ...createGitFetcher(opts),
-    ...createDirectoryFetcher(),
+    ...createDirectoryFetcher({ resolveSymlinks: opts.resolveSymlinksInInjectedDirs }),
   }
 
   const overwrites = Object.entries(customFetchers ?? {})

packages/directory-fetcher/package.json

@@ -44,7 +44,8 @@
     "@pnpm/directory-fetcher": "workspace:*",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/npm-packlist": "^3.0.0",
-    "@types/ramda": "0.28.15"
+    "@types/ramda": "0.28.15",
+    "@zkochan/rimraf": "^2.1.2"
   },
   "exports": {
     ".": "./lib/index.js"

packages/directory-fetcher/src/index.ts

@@ -10,12 +10,14 @@ const directoryFetcherLogger = logger('directory-fetcher')
 
 export interface CreateDirectoryFetcherOptions {
   includeOnlyPackageFiles?: boolean
+  resolveSymlinks?: boolean
 }
 
 export function createDirectoryFetcher (
   opts?: CreateDirectoryFetcherOptions
 ) {
-  const fetchFromDir = opts?.includeOnlyPackageFiles ? fetchPackageFilesFromDir : fetchAllFilesFromDir
+  const readFileStat: ReadFileStat = opts?.resolveSymlinks === true ? realFileStat : fileStat
+  const fetchFromDir = opts?.includeOnlyPackageFiles ? fetchPackageFilesFromDir : fetchAllFilesFromDir.bind(null, readFileStat)
 
   const directoryFetcher: DirectoryFetcher = (cafs, resolution, opts) => {
     const dir = path.join(opts.lockfileDir, resolution.directory)
@@ -36,14 +38,16 @@ export async function fetchFromDir (
   if (opts.includeOnlyPackageFiles) {
     return fetchPackageFilesFromDir(dir, opts)
   }
-  return fetchAllFilesFromDir(dir, opts)
+  const readFileStat: ReadFileStat = opts?.resolveSymlinks === true ? realFileStat : fileStat
+  return fetchAllFilesFromDir(readFileStat, dir, opts)
 }
 
 async function fetchAllFilesFromDir (
+  readFileStat: ReadFileStat,
   dir: string,
   opts: FetchFromDirOpts
 ) {
-  const filesIndex = await _fetchAllFilesFromDir(dir)
+  const filesIndex = await _fetchAllFilesFromDir(readFileStat, dir)
   if (opts.manifest) {
     // 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.
@@ -59,6 +63,7 @@ async function fetchAllFilesFromDir (
 }
 
 async function _fetchAllFilesFromDir (
+  readFileStat: ReadFileStat,
   dir: string,
   relativeDir = ''
 ): Promise<Record<string, string>> {
@@ -67,21 +72,11 @@ async function _fetchAllFilesFromDir (
   await Promise.all(files
     .filter((file) => file !== 'node_modules')
     .map(async (file) => {
-      const filePath = path.join(dir, file)
-      let stat: Stats
-      try {
-        stat = await fs.stat(filePath)
-      } catch (err: any) { // eslint-disable-line @typescript-eslint/no-explicit-any
-        // Broken symlinks are skipped
-        if (err.code === 'ENOENT') {
-          directoryFetcherLogger.debug({ brokenSymlink: filePath })
-          return
-        }
-        throw err
-      }
+      const { filePath, stat } = await readFileStat(path.join(dir, file))
+      if (!filePath) return
       const relativeSubdir = `${relativeDir}${relativeDir ? '/' : ''}${file}`
       if (stat.isDirectory()) {
-        const subFilesIndex = await _fetchAllFilesFromDir(filePath, relativeSubdir)
+        const subFilesIndex = await _fetchAllFilesFromDir(readFileStat, filePath, relativeSubdir)
         Object.assign(filesIndex, subFilesIndex)
       } else {
         filesIndex[relativeSubdir] = filePath
@@ -91,6 +86,43 @@ async function _fetchAllFilesFromDir (
   return filesIndex
 }
 
+type ReadFileStat = (filePath: string) => Promise<{ filePath: string, stat: Stats } | { filePath: null, stat: null }>
+
+async function realFileStat (filePath: string): Promise<{ filePath: string, stat: Stats } | { filePath: null, stat: null }> {
+  let stat = await fs.lstat(filePath)
+  if (!stat.isSymbolicLink()) {
+    return { filePath, stat }
+  }
+  try {
+    filePath = await fs.realpath(filePath)
+    stat = await fs.stat(filePath)
+    return { filePath, stat }
+  } catch (err: any) { // eslint-disable-line @typescript-eslint/no-explicit-any
+    // Broken symlinks are skipped
+    if (err.code === 'ENOENT') {
+      directoryFetcherLogger.debug({ brokenSymlink: filePath })
+      return { filePath: null, stat: null }
+    }
+    throw err
+  }
+}
+
+async function fileStat (filePath: string): Promise<{ filePath: string, stat: Stats } | { filePath: null, stat: null }> {
+  try {
+    return {
+      filePath,
+      stat: await fs.stat(filePath),
+    }
+  } catch (err: any) { // eslint-disable-line @typescript-eslint/no-explicit-any
+    // Broken symlinks are skipped
+    if (err.code === 'ENOENT') {
+      directoryFetcherLogger.debug({ brokenSymlink: filePath })
+      return { filePath: null, stat: null }
+    }
+    throw err
+  }
+}
+
 async function fetchPackageFilesFromDir (
   dir: string,
   opts: FetchFromDirOpts

packages/directory-fetcher/test/fixtures/pkg-with-symlinked-dir-and-files/.gitignore

@@ -0,0 +1,2 @@
+src
+index.js

packages/directory-fetcher/test/fixtures/pkg-with-symlinked-dir-and-files/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "pkg-with-symlinked-dir-and-files",
+  "version": "0.0.0"
+}

packages/directory-fetcher/test/index.ts

@@ -1,9 +1,11 @@
 /// <reference path="../../../typings/index.d.ts"/>
+import fs from 'fs'
 import path from 'path'
 import { createDirectoryFetcher } from '@pnpm/directory-fetcher'
 // @ts-expect-error
 import { debug } from '@pnpm/logger'
 import { fixtures } from '@pnpm/test-fixtures'
+import rimraf from '@zkochan/rimraf'
 
 const f = fixtures(__dirname)
 jest.mock('@pnpm/logger', () => {
@@ -110,3 +112,48 @@ test('fetch does not fail on package with broken symlink', async () => {
   ])
   expect(debug).toHaveBeenCalledWith({ brokenSymlink: path.resolve('not-exists') })
 })
+
+describe('fetch resolves symlinked files to their real locations', () => {
+  const indexJsPath = path.join(f.find('no-manifest'), 'index.js')
+  const srcPath = f.find('simple-pkg')
+  beforeAll(async () => {
+    process.chdir(f.find('pkg-with-symlinked-dir-and-files'))
+    await rimraf('index.js')
+    fs.symlinkSync(indexJsPath, path.resolve('index.js'), 'file')
+    await rimraf('src')
+    fs.symlinkSync(srcPath, path.resolve('src'), 'dir')
+  })
+  test('fetch resolves symlinked files to their real locations', async () => {
+    const fetcher = createDirectoryFetcher({ resolveSymlinks: true })
+    // eslint-disable-next-line
+    const fetchResult = await fetcher.directory({} as any, {
+      directory: '.',
+      type: 'directory',
+    }, {
+      lockfileDir: process.cwd(),
+    })
+
+    expect(fetchResult.local).toBe(true)
+    expect(fetchResult.packageImportMethod).toBe('hardlink')
+    expect(fetchResult.filesIndex['package.json']).toBe(path.resolve('package.json'))
+    expect(fetchResult.filesIndex['index.js']).toBe(indexJsPath)
+    expect(fetchResult.filesIndex['src/index.js']).toBe(path.join(srcPath, 'index.js'))
+  })
+  test('fetch does not resolve symlinked files to their real locations by default', async () => {
+    const fetcher = createDirectoryFetcher()
+
+    // eslint-disable-next-line
+    const fetchResult = await fetcher.directory({} as any, {
+      directory: '.',
+      type: 'directory',
+    }, {
+      lockfileDir: process.cwd(),
+    })
+
+    expect(fetchResult.local).toBe(true)
+    expect(fetchResult.packageImportMethod).toBe('hardlink')
+    expect(fetchResult.filesIndex['package.json']).toBe(path.resolve('package.json'))
+    expect(fetchResult.filesIndex['index.js']).toBe(path.resolve('index.js'))
+    expect(fetchResult.filesIndex['src/index.js']).toBe(path.resolve('src/index.js'))
+  })
+})

packages/store-connection-manager/src/createNewStoreController.ts

@@ -1,5 +1,5 @@
 import { promises as fs } from 'fs'
-import { createClient } from '@pnpm/client'
+import { createClient, ClientOptions } from '@pnpm/client'
 import { Config } from '@pnpm/config'
 import { createPackageStore } from '@pnpm/package-store'
 import { packageManager } from '@pnpm/cli-meta'
@@ -41,7 +41,7 @@ export type CreateNewStoreControllerOptions = CreateResolverOptions & Pick<Confi
 | 'verifyStoreIntegrity'
 > & {
   ignoreFile?: (filename: string) => boolean
-} & Partial<Pick<Config, 'userConfig'>>
+} & Partial<Pick<Config, 'userConfig'>> & Pick<ClientOptions, 'resolveSymlinksInInjectedDirs'>
 
 export async function createNewStoreController (
   opts: CreateNewStoreControllerOptions
@@ -78,6 +78,7 @@ export async function createNewStoreController (
         : undefined
     ),
     gitShallowHosts: opts.gitShallowHosts,
+    resolveSymlinksInInjectedDirs: opts.resolveSymlinksInInjectedDirs,
   })
   await fs.mkdir(opts.storeDir, { recursive: true })
   return {

pnpm-lock.yaml

@@ -1084,6 +1084,9 @@ importers:
       '@types/ramda':
         specifier: 0.28.15
         version: 0.28.15
+      '@zkochan/rimraf':
+        specifier: ^2.1.2
+        version: 2.1.2
 
   packages/error:
     dependencies: