fix(plugin-commands-patching): should fetch dependency from tarball url when patching dependency installed from git (#7230)

close #7196

.changeset/proud-trees-develop.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-patching": patch
+"pnpm": patch
+---
+
+Should fetch dependency from tarball url when patching dependency installed from git [#7196](https://github.com/pnpm/pnpm/issues/7196)

patching/plugin-commands-patching/package.json

@@ -53,6 +53,7 @@
     "@pnpm/parse-wanted-dependency": "workspace:*",
     "@pnpm/patching.apply-patch": "workspace:*",
     "@pnpm/pick-registry-for-package": "workspace:*",
+    "@pnpm/pick-fetcher": "workspace:*",
     "@pnpm/plugin-commands-installation": "workspace:*",
     "@pnpm/read-package-json": "workspace:*",
     "@pnpm/read-project-manifest": "workspace:*",

patching/plugin-commands-patching/src/getPatchedDependency.ts

@@ -1,16 +1,17 @@
 import path from 'path'
 import { parseWantedDependency, type ParseWantedDependencyResult } from '@pnpm/parse-wanted-dependency'
 import { prompt } from 'enquirer'
-import { readCurrentLockfile } from '@pnpm/lockfile-file'
+import { readCurrentLockfile, type TarballResolution } from '@pnpm/lockfile-file'
 import { nameVerFromPkgSnapshot } from '@pnpm/lockfile-utils'
 import { PnpmError } from '@pnpm/error'
 import { WANTED_LOCKFILE } from '@pnpm/constants'
 import { readModulesManifest } from '@pnpm/modules-yaml'
+import { isGitHostedPkgUrl } from '@pnpm/pick-fetcher'
 import realpathMissing from 'realpath-missing'
 import semver from 'semver'
 import { type Config } from '@pnpm/config'
 
-type GetPatchedDependencyOptions = {
+export type GetPatchedDependencyOptions = {
   lockfileDir: string
 } & Pick<Config, 'virtualStoreDir' | 'modulesDir'>
 
@@ -22,7 +23,7 @@ export async function getPatchedDependency (rawDependency: string, opts: GetPatc
   if (!preferredVersions.length) {
     throw new PnpmError(
       'PATCH_VERSION_NOT_FOUND',
-      `Can not find ${rawDependency} in project ${opts.lockfileDir}, ${versions.length ? `you can specify currently installed version: ${versions.join(', ')}.` : `did you forget to install ${rawDependency}?`}`
+      `Can not find ${rawDependency} in project ${opts.lockfileDir}, ${versions.length ? `you can specify currently installed version: ${versions.map(({ version }) => version).join(', ')}.` : `did you forget to install ${rawDependency}?`}`
     )
   }
 
@@ -34,17 +35,26 @@ export async function getPatchedDependency (rawDependency: string, opts: GetPatc
       type: 'select',
       name: 'version',
       message: 'Choose which version to patch',
-      choices: versions,
+      choices: preferredVersions.map(preferred => ({
+        name: preferred.version,
+        message: preferred.version,
+        value: preferred.gitTarballUrl ?? preferred.version,
+        hint: preferred.gitTarballUrl ? 'Git Hosted' : undefined,
+      })),
+      result (selected) {
+        const selectedVersion = preferredVersions.find(preferred => preferred.version === selected)!
+        return selectedVersion.gitTarballUrl ?? selected
+      },
     })
     dep.pref = version
   } else {
-    dep.pref = preferredVersions[0]
+    const preferred = preferredVersions[0]
+    dep.pref = preferred.gitTarballUrl ?? preferred.version
   }
-
   return dep
 }
 
-async function getVersionsFromLockfile (dep: ParseWantedDependencyResult, opts: GetPatchedDependencyOptions) {
+export async function getVersionsFromLockfile (dep: ParseWantedDependencyResult, opts: GetPatchedDependencyOptions) {
   const modulesDir = await realpathMissing(path.join(opts.lockfileDir, opts.modulesDir ?? 'node_modules'))
   const modules = await readModulesManifest(modulesDir)
   const lockfile = (modules?.virtualStoreDir && await readCurrentLockfile(modules.virtualStoreDir, {
@@ -61,12 +71,17 @@ async function getVersionsFromLockfile (dep: ParseWantedDependencyResult, opts:
   const pkgName = dep.alias && dep.pref ? dep.alias : (dep.pref ?? dep.alias)
 
   const versions = Object.entries(lockfile.packages ?? {})
-    .map(([depPath, pkgSnapshot]) => nameVerFromPkgSnapshot(depPath, pkgSnapshot))
+    .map(([depPath, pkgSnapshot]) => {
+      const tarball = (pkgSnapshot.resolution as TarballResolution)?.tarball ?? ''
+      return {
+        ...nameVerFromPkgSnapshot(depPath, pkgSnapshot),
+        gitTarballUrl: isGitHostedPkgUrl(tarball) ? tarball : undefined,
+      }
+    })
     .filter(({ name }) => name === pkgName)
-    .map(({ version }) => version)
 
   return {
     versions,
-    preferredVersions: versions.filter(version => dep.alias && dep.pref ? semver.satisfies(version, dep.pref) : true),
+    preferredVersions: versions.filter(({ version }) => dep.alias && dep.pref ? semver.satisfies(version, dep.pref) : true),
   }
 }

patching/plugin-commands-patching/src/patchCommit.ts

@@ -14,8 +14,9 @@ import escapeStringRegexp from 'escape-string-regexp'
 import renderHelp from 'render-help'
 import tempy from 'tempy'
 import { writePackage } from './writePackage'
-import { parseWantedDependency } from '@pnpm/parse-wanted-dependency'
+import { type ParseWantedDependencyResult, parseWantedDependency } from '@pnpm/parse-wanted-dependency'
 import packlist from 'npm-packlist'
+import { type GetPatchedDependencyOptions, getVersionsFromLockfile } from './getPatchedDependency'
 
 export const rcOptionsTypes = cliOptionsTypes
 
@@ -50,9 +51,16 @@ export async function handler (opts: install.InstallCommandOptions & Pick<Config
   await fs.promises.mkdir(patchesDir, { recursive: true })
   const patchedPkgManifest = await readPackageJsonFromDir(userDir)
   const pkgNameAndVersion = `${patchedPkgManifest.name}@${patchedPkgManifest.version}`
+  const gitTarballUrl = await getGitTarballUrlFromLockfile({
+    alias: patchedPkgManifest.name,
+    pref: patchedPkgManifest.version,
+  }, {
+    lockfileDir,
+    modulesDir: opts.modulesDir,
+    virtualStoreDir: opts.virtualStoreDir,
+  })
   const srcDir = tempy.directory()
-  await writePackage(parseWantedDependency(pkgNameAndVersion), srcDir, opts)
-
+  await writePackage(parseWantedDependency(gitTarballUrl ? `${patchedPkgManifest.name}@${gitTarballUrl}` : pkgNameAndVersion), srcDir, opts)
   const patchedPkgDir = await preparePkgFilesForDiff(userDir)
   const patchContent = await diffFolders(srcDir, patchedPkgDir)
 
@@ -174,3 +182,8 @@ async function areAllFilesInPkg (files: string[], basePath: string) {
   })
   return equals(allFiles.sort(), files.sort())
 }
+
+async function getGitTarballUrlFromLockfile (dep: ParseWantedDependencyResult, opts: GetPatchedDependencyOptions) {
+  const { preferredVersions } = await getVersionsFromLockfile(dep, opts)
+  return preferredVersions[0]?.gitTarballUrl
+}

patching/plugin-commands-patching/test/patch.test.ts

@@ -366,11 +366,21 @@ describe('prompt to choose version', () => {
     prompt.mockResolvedValue({
       version: '5.3.0',
     })
-
+    prompt.mockClear()
     const output = await patch.handler(defaultPatchOption, ['chalk'])
 
-    expect(prompt.mock.calls[0][0].choices).toEqual(expect.arrayContaining(['5.3.0', '4.1.2']))
-    prompt.mockClear()
+    expect(prompt.mock.calls[0][0].choices).toEqual(expect.arrayContaining([
+      {
+        name: '4.1.2',
+        message: '4.1.2',
+        value: '4.1.2',
+      },
+      {
+        name: '5.3.0',
+        message: '5.3.0',
+        value: '5.3.0',
+      },
+    ]))
 
     const patchDir = getPatchDirFromPatchOutput(output)
     const tempDir = os.tmpdir()
@@ -506,6 +516,7 @@ describe('patch and commit in workspaces', () => {
         version: '1.0.0',
         dependencies: {
           'is-positive': '1.0.0',
+          hi: '0.0.0',
         },
       },
       {
@@ -514,6 +525,7 @@ describe('patch and commit in workspaces', () => {
         dependencies: {
           'is-positive': '1.0.0',
           'project-1': '1',
+          hi: 'github:zkochan/hi#4cdebec76b7b9d1f6e219e06c42d92a6b8ea60cd',
         },
       },
     ])
@@ -569,7 +581,6 @@ describe('patch and commit in workspaces', () => {
       workspaceDir: process.cwd(),
       saveLockfile: true,
       frozenLockfile: false,
-      fixLockfile: true,
     }, [patchDir])
 
     const { manifest } = await readProjectManifest(process.cwd())
@@ -646,6 +657,69 @@ describe('patch and commit in workspaces', () => {
     expect(fs.readFileSync('../project-2/node_modules/is-positive/index.js', 'utf8')).not.toContain('// test patching')
     expect(fs.existsSync('../project-2/node_modules/is-positive/license')).toBe(true)
   })
+
+  test('patch and patch-commit for git hosted dependency', async () => {
+    const { allProjects, allProjectsGraph, selectedProjectsGraph } = await readProjects(process.cwd(), [])
+    await install.handler({
+      ...DEFAULT_OPTS,
+      cacheDir,
+      storeDir,
+      allProjects,
+      allProjectsGraph,
+      dir: process.cwd(),
+      lockfileDir: process.cwd(),
+      selectedProjectsGraph,
+      workspaceDir: process.cwd(),
+      saveLockfile: true,
+    })
+
+    prompt.mockResolvedValue({
+      version: 'https://codeload.github.com/zkochan/hi/tar.gz/4cdebec76b7b9d1f6e219e06c42d92a6b8ea60cd',
+    })
+    prompt.mockClear()
+    const output = await patch.handler(defaultPatchOption, ['hi'])
+    expect(prompt.mock.calls[0][0].choices).toEqual(expect.arrayContaining([
+      {
+        name: '0.0.0',
+        message: '0.0.0',
+        value: '0.0.0',
+      },
+      {
+        name: '1.0.0',
+        message: '1.0.0',
+        value: 'https://codeload.github.com/zkochan/hi/tar.gz/4cdebec76b7b9d1f6e219e06c42d92a6b8ea60cd',
+        hint: 'Git Hosted',
+      },
+    ]))
+    const patchDir = getPatchDirFromPatchOutput(output)
+    expect(fs.existsSync(patchDir)).toBe(true)
+    expect(fs.readFileSync(path.join(patchDir, 'index.js'), 'utf8')).toContain('module.exports = \'Hi\'')
+    fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8')
+
+    await patchCommit.handler({
+      ...DEFAULT_OPTS,
+      allProjects,
+      allProjectsGraph,
+      selectedProjectsGraph,
+      dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
+      cacheDir,
+      storeDir,
+      lockfileDir: process.cwd(),
+      workspaceDir: process.cwd(),
+      saveLockfile: true,
+      frozenLockfile: false,
+    }, [patchDir])
+
+    const { manifest } = await readProjectManifest(process.cwd())
+    expect(manifest.pnpm?.patchedDependencies).toStrictEqual({
+      'hi@1.0.0': 'patches/hi@1.0.0.patch',
+    })
+    const patchContent = fs.readFileSync('patches/hi@1.0.0.patch', 'utf8')
+    expect(patchContent).toContain('diff --git')
+    expect(patchContent).toContain('// test patching')
+    expect(fs.readFileSync('./project-2/node_modules/hi/index.js', 'utf8')).toContain('// test patching')
+  })
 })
 
 describe('patch with custom modules-dir and virtual-store-dir', () => {

patching/plugin-commands-patching/tsconfig.json

@@ -24,6 +24,9 @@
     {
       "path": "../../config/pick-registry-for-package"
     },
+    {
+      "path": "../../fetching/pick-fetcher"
+    },
     {
       "path": "../../lockfile/lockfile-file"
     },

pnpm-lock.yaml

@@ -2758,6 +2758,9 @@ importers:
       '@pnpm/patching.apply-patch':
         specifier: workspace:*
         version: link:../apply-patch
+      '@pnpm/pick-fetcher':
+        specifier: workspace:*
+        version: link:../../fetching/pick-fetcher
       '@pnpm/pick-registry-for-package':
         specifier: workspace:*
         version: link:../../config/pick-registry-for-package