diff --git a/.changeset/repatch-git-hosted-dependency.md b/.changeset/repatch-git-hosted-dependency.md new file mode 100644 index 0000000000..ccf4cda589 --- /dev/null +++ b/.changeset/repatch-git-hosted-dependency.md @@ -0,0 +1,7 @@ +--- +"@pnpm/patching.commands": patch +"pnpm": patch +"pacquet": patch +--- + +`pnpm patch` now applies the existing patch file to the edit directory of a git-hosted dependency, as it already does for packages from the registry [#9699](https://github.com/pnpm/pnpm/issues/9699). diff --git a/pnpm/crates/cli/src/cli_args/patch/paths.rs b/pnpm/crates/cli/src/cli_args/patch/paths.rs index deb7aa5ffd..770c347f79 100644 --- a/pnpm/crates/cli/src/cli_args/patch/paths.rs +++ b/pnpm/crates/cli/src/cli_args/patch/paths.rs @@ -1,6 +1,7 @@ use super::{ Component, PatchError, PatchTarget, Path, PathBuf, fs, io, is_subdir, lexical_normalize, }; +use indexmap::IndexMap; pub(super) fn resolve_path(dir: &Path, path: &Path) -> PathBuf { if path.is_absolute() { path.to_path_buf() } else { dir.join(path) } @@ -163,15 +164,9 @@ pub(super) fn apply_existing_patch_file( edit_dir: &Path, ) -> Result<(), PatchError> { let Some(patched_dependencies) = &config.patched_dependencies else { return Ok(()) }; - let exact_key = format!("{}@{}", target.alias, target.bare_specifier); - let patch_file = patched_dependencies - .get(&exact_key) - .or_else(|| { - target.apply_to_all - .then(|| patched_dependencies.get(&target.alias)) - .flatten() - }); - let Some(patch_file) = patch_file else { return Ok(()) }; + let Some(patch_file) = find_existing_patch_file(patched_dependencies, target) else { + return Ok(()); + }; let base_dir = config.workspace_dir .as_deref() .unwrap_or_else(|| config.modules_dir_anchor().unwrap_or_else(|| Path::new("."))); @@ -187,6 +182,23 @@ pub(super) fn apply_existing_patch_file( .map_err(PatchError::ApplyExistingPatch) } +/// The patch file configured for `target`, if any. `patch-commit` keys the +/// patch of a git-hosted package by its version, while the target's bare +/// specifier is the package's tarball URL, so both are tried. +pub(super) fn find_existing_patch_file<'a>( + patched_dependencies: &'a IndexMap, + target: &PatchTarget, +) -> Option<&'a String> { + [&target.bare_specifier, &target.version] + .into_iter() + .find_map(|specifier| patched_dependencies.get(&format!("{}@{specifier}", target.alias))) + .or_else(|| { + target.apply_to_all + .then(|| patched_dependencies.get(&target.alias)) + .flatten() + }) +} + struct ExistingPatchFileContext { project_root: PathBuf, patches_dir: PathBuf, diff --git a/pnpm/crates/cli/src/cli_args/patch/tests.rs b/pnpm/crates/cli/src/cli_args/patch/tests.rs index 69ecd6ab3f..b4b49ace19 100644 --- a/pnpm/crates/cli/src/cli_args/patch/tests.rs +++ b/pnpm/crates/cli/src/cli_args/patch/tests.rs @@ -5,7 +5,8 @@ use super::{ }; #[cfg(unix)] use crate::cli_args::patch::paths::checked_existing_patch_file_path; -use crate::cli_args::patch::paths::default_edit_dir_name; +use crate::cli_args::patch::paths::{default_edit_dir_name, find_existing_patch_file}; +use indexmap::IndexMap; use std::{io::IsTerminal, path::Path}; use tempfile::tempdir; @@ -223,6 +224,27 @@ fn default_edit_dir_name_falls_back_to_alias_then_requested_package() { assert_eq!(default_edit_dir_name("chalk@npm:chalk@5.3.0", &target), "chalk@npm:chalk@5.3.0"); } +/// Regression test for . +#[test] +fn existing_patch_file_of_a_git_hosted_package_is_found_by_version() { + let tarball = "https://codeload.github.com/example/hi/tar.gz/deadbeef"; + let target = PatchTarget { + alias: "hi".to_string(), + version: "1.0.0".to_string(), + bare_specifier: tarball.to_string(), + apply_to_all: false, + git_tarball_url: Some(tarball.to_string()), + package_key: format!("hi@{tarball}").parse().expect("package key"), + }; + let patched_dependencies = + IndexMap::from([("hi@1.0.0".to_string(), "patches/hi@1.0.0.patch".to_string())]); + + assert_eq!( + find_existing_patch_file(&patched_dependencies, &target).map(String::as_str), + Some("patches/hi@1.0.0.patch"), + ); +} + struct FakePrompt { selected: usize, apply_to_all: bool, diff --git a/pnpm11/patching/commands/src/getPatchedDependency.ts b/pnpm11/patching/commands/src/getPatchedDependency.ts index 5e98a28d2c..7ac6ca9935 100644 --- a/pnpm11/patching/commands/src/getPatchedDependency.ts +++ b/pnpm11/patching/commands/src/getPatchedDependency.ts @@ -13,7 +13,14 @@ export type GetPatchedDependencyOptions = { lockfileDir: string } & Pick -export type GetPatchedDependencyResult = ParseWantedDependencyResult & { applyToAll: boolean } +export type GetPatchedDependencyResult = ParseWantedDependencyResult & { + applyToAll: boolean + /** + * The installed version of the chosen package. It differs from `bareSpecifier` + * when the package is git-hosted, in which case `bareSpecifier` is its tarball URL. + */ + version?: string +} export async function getPatchedDependency (rawDependency: string, opts: GetPatchedDependencyOptions): Promise { const dep = parseWantedDependency(rawDependency) @@ -29,10 +36,10 @@ export async function getPatchedDependency (rawDependency: string, opts: GetPatc dep.alias = dep.alias ?? rawDependency if (preferredVersions.length > 1) { - let version: string + let bareSpecifier: string let applyToAll: boolean try { - version = await select({ + bareSpecifier = await select({ message: 'Choose which version to patch', choices: preferredVersions.map(preferred => ({ name: preferred.version, @@ -53,7 +60,8 @@ export async function getPatchedDependency (rawDependency: string, opts: GetPatc return { ...dep, applyToAll, - bareSpecifier: version, + bareSpecifier, + version: preferredVersions.find(preferred => (preferred.gitTarballUrl ?? preferred.version) === bareSpecifier)?.version, } } else { const preferred = preferredVersions[0] @@ -62,12 +70,14 @@ export async function getPatchedDependency (rawDependency: string, opts: GetPatc ...dep, applyToAll: false, bareSpecifier: preferred.gitTarballUrl, + version: preferred.version, } } return { ...dep, applyToAll: !dep.bareSpecifier, bareSpecifier: preferred.version, + version: preferred.version, } } } diff --git a/pnpm11/patching/commands/src/patch.ts b/pnpm11/patching/commands/src/patch.ts index 9c0494f0b0..079044e0a7 100644 --- a/pnpm11/patching/commands/src/patch.ts +++ b/pnpm11/patching/commands/src/patch.ts @@ -133,7 +133,7 @@ To commit your changes, run: function tryPatchWithExistingPatchFile ( { - patchedDep: { applyToAll, alias, bareSpecifier }, + patchedDep: { applyToAll, alias, bareSpecifier, version }, patchedDir, patchedDependencies, lockfileDir, @@ -149,6 +149,9 @@ function tryPatchWithExistingPatchFile ( if (bareSpecifier) { existingPatchFile = patchedDependencies[`${alias}@${bareSpecifier}`] } + if (!existingPatchFile && version) { + existingPatchFile = patchedDependencies[`${alias}@${version}`] + } if (!existingPatchFile && applyToAll) { existingPatchFile = patchedDependencies[alias] } diff --git a/pnpm11/patching/commands/test/patch.test.ts b/pnpm11/patching/commands/test/patch.test.ts index bc6874c304..2d14e8e67f 100644 --- a/pnpm11/patching/commands/test/patch.test.ts +++ b/pnpm11/patching/commands/test/patch.test.ts @@ -1518,6 +1518,15 @@ describe('patch and commit in workspaces', () => { expect(patchContent).toContain('diff --git') expect(patchContent).toContain('// test patching') expect(fs.readFileSync('./project-2/node_modules/hi/index.js', 'utf8')).toContain('// test patching') + + // re-patch + fs.rmSync(patchDir, { recursive: true }) + const repatchOutput = await patch.handler({ + ...defaultPatchOption, + patchedDependencies: workspaceManifest!.patchedDependencies, + }, ['hi']) + const repatchDir = getPatchDirFromPatchOutput(repatchOutput) + expect(fs.readFileSync(path.join(repatchDir, 'index.js'), 'utf8')).toContain('// test patching') }) })