From 48df3afdfb6111fcd5a9d6d422e6cb12c006667a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:19:57 +0000 Subject: [PATCH 01/12] feat(apply): add --reinstall flag to reinstall clean package before applying diff https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- patch_package_py/cli.py | 12 +++++++- patch_package_py/core.py | 24 +++++++++++++--- tests/test_core.py | 62 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/patch_package_py/cli.py b/patch_package_py/cli.py index 8dc0290..ebf568c 100644 --- a/patch_package_py/cli.py +++ b/patch_package_py/cli.py @@ -96,7 +96,12 @@ def cmd_apply(args): return for patch_file in patch_files: - apply_patch(patch_file, site_packages_dir) + apply_patch( + patch_file, + site_packages_dir, + env_path=env_path if args.reinstall else None, + reinstall=args.reinstall, + ) def cli(): @@ -142,6 +147,11 @@ def cli(): # apply command apply_parser = subparsers.add_parser("apply", help="Apply patches") apply_parser.add_argument("-e", "--env-path", help="Environment Path") + apply_parser.add_argument( + "--reinstall", + action="store_true", + help="Reinstall the clean package before applying each patch", + ) apply_parser.set_defaults(func=cmd_apply) args = parser.parse_args() diff --git a/patch_package_py/core.py b/patch_package_py/core.py index 1228698..878aff5 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -313,7 +313,13 @@ def commit_changes( logger.info(f"Patch created and applied for {package_name}=={version}") -def apply_patch(patch_file: Path, site_packages_dir: Path) -> None: +def apply_patch( + patch_file: Path, + site_packages_dir: Path, + *, + env_path: Union[Path, None] = None, + reinstall: bool = False, +) -> None: # Parse package name and version from patch file name patch_name = patch_file.stem # Remove .patch extension if "+" not in patch_name: @@ -339,6 +345,11 @@ def apply_patch(patch_file: Path, site_packages_dir: Path) -> None: f"Version mismatch: patch is for {package_name}=={version} but installed version is {installed_version}" ) + if reinstall: + if env_path is None: + raise ValueError("env_path is required when reinstall=True") + restore_clean_package(package_name, version, env_path) + # First, check if the patch is already applied using dry-run try: subprocess.check_call( @@ -356,9 +367,14 @@ def apply_patch(patch_file: Path, site_packages_dir: Path) -> None: stdout=subprocess.DEVNULL, ) except subprocess.CalledProcessError: - logger.warning( - f"Patch `{patch_name}` appears to be already applied, skipping...", - ) + if reinstall: + logger.error( + f"Failed to apply patch `{patch_name}` after reinstalling clean package.", + ) + else: + logger.warning( + f"Patch `{patch_name}` appears to be already applied, skipping...", + ) return # If dry-run succeeds, apply the patch for real diff --git a/tests/test_core.py b/tests/test_core.py index ee74ba7..b276115 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -210,6 +210,68 @@ def test_apply_patch_already_applied(self, tmp_path: Path, caplog): assert "already applied" in caplog.text + def test_apply_patch_reinstall_calls_restore(self, tmp_path: Path): + """Test that reinstall=True calls restore_clean_package before patching.""" + site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + patch_file = tmp_path / "mypackage+1.0.0.patch" + patch_file.write_text( + "--- a/mypackage/core.py\n" + "+++ b/mypackage/core.py\n" + "@@ -1,2 +1,2 @@\n" + " def hello():\n" + "- return 'hello'\n" + "+ return 'hello world'\n" + ) + env_path = tmp_path / ".venv" + + with patch("subprocess.check_call") as mock_check_call: + apply_patch(patch_file, site_packages, env_path=env_path, reinstall=True) + + commands = [call_args.args[0] for call_args in mock_check_call.call_args_list] + assert commands[0] == [ + "uv", + "pip", + "install", + "--force-reinstall", + "--no-deps", + "mypackage==1.0.0", + "--python", + str(venv_python(env_path)), + ] + # dry-run + actual apply should follow + assert mock_check_call.call_count == 3 + + def test_apply_patch_reinstall_requires_env_path(self, tmp_path: Path): + """Test that reinstall=True without env_path raises ValueError.""" + site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + patch_file = tmp_path / "mypackage+1.0.0.patch" + patch_file.write_text("some patch content") + + with pytest.raises(ValueError, match="env_path is required"): + apply_patch(patch_file, site_packages, reinstall=True) + + def test_apply_patch_reinstall_dry_run_failure_logs_error( + self, tmp_path: Path, caplog + ): + """Test that a broken patch after reinstall logs an error (not 'already applied').""" + site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + patch_file = tmp_path / "mypackage+1.0.0.patch" + patch_file.write_text("some patch content") + env_path = tmp_path / ".venv" + + calls = [None] # first call (restore) succeeds; second (dry-run) fails + + def side_effect(cmd, *args, **kwargs): + if cmd[0] == "uv": + return None + raise subprocess.CalledProcessError(1, "patch") + + with patch("subprocess.check_call", side_effect=side_effect): + apply_patch(patch_file, site_packages, env_path=env_path, reinstall=True) + + assert "already applied" not in caplog.text + assert "Failed to apply patch" in caplog.text + class TestCommitChanges: """Tests for creating patch files via commit_changes.""" From c764949b49c23e00974209a151662ed43a3645d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:22:48 +0000 Subject: [PATCH 02/12] refactor(apply): rename reinstall -> restore to align with commit's --skip-restore https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- patch_package_py/cli.py | 8 ++++---- patch_package_py/core.py | 10 +++++----- tests/test_core.py | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/patch_package_py/cli.py b/patch_package_py/cli.py index ebf568c..4f7b27a 100644 --- a/patch_package_py/cli.py +++ b/patch_package_py/cli.py @@ -99,8 +99,8 @@ def cmd_apply(args): apply_patch( patch_file, site_packages_dir, - env_path=env_path if args.reinstall else None, - reinstall=args.reinstall, + env_path=env_path if args.restore else None, + restore=args.restore, ) @@ -148,9 +148,9 @@ def cli(): apply_parser = subparsers.add_parser("apply", help="Apply patches") apply_parser.add_argument("-e", "--env-path", help="Environment Path") apply_parser.add_argument( - "--reinstall", + "--restore", action="store_true", - help="Reinstall the clean package before applying each patch", + help="Restore the clean package before applying each patch", ) apply_parser.set_defaults(func=cmd_apply) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index 878aff5..a92c350 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -318,7 +318,7 @@ def apply_patch( site_packages_dir: Path, *, env_path: Union[Path, None] = None, - reinstall: bool = False, + restore: bool = False, ) -> None: # Parse package name and version from patch file name patch_name = patch_file.stem # Remove .patch extension @@ -345,9 +345,9 @@ def apply_patch( f"Version mismatch: patch is for {package_name}=={version} but installed version is {installed_version}" ) - if reinstall: + if restore: if env_path is None: - raise ValueError("env_path is required when reinstall=True") + raise ValueError("env_path is required when restore=True") restore_clean_package(package_name, version, env_path) # First, check if the patch is already applied using dry-run @@ -367,9 +367,9 @@ def apply_patch( stdout=subprocess.DEVNULL, ) except subprocess.CalledProcessError: - if reinstall: + if restore: logger.error( - f"Failed to apply patch `{patch_name}` after reinstalling clean package.", + f"Failed to apply patch `{patch_name}` after restoring clean package.", ) else: logger.warning( diff --git a/tests/test_core.py b/tests/test_core.py index b276115..aa5fd54 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -210,8 +210,8 @@ def test_apply_patch_already_applied(self, tmp_path: Path, caplog): assert "already applied" in caplog.text - def test_apply_patch_reinstall_calls_restore(self, tmp_path: Path): - """Test that reinstall=True calls restore_clean_package before patching.""" + def test_apply_patch_restore_calls_restore(self, tmp_path: Path): + """Test that restore=True calls restore_clean_package before patching.""" site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text( @@ -225,7 +225,7 @@ def test_apply_patch_reinstall_calls_restore(self, tmp_path: Path): env_path = tmp_path / ".venv" with patch("subprocess.check_call") as mock_check_call: - apply_patch(patch_file, site_packages, env_path=env_path, reinstall=True) + apply_patch(patch_file, site_packages, env_path=env_path, restore=True) commands = [call_args.args[0] for call_args in mock_check_call.call_args_list] assert commands[0] == [ @@ -241,19 +241,19 @@ def test_apply_patch_reinstall_calls_restore(self, tmp_path: Path): # dry-run + actual apply should follow assert mock_check_call.call_count == 3 - def test_apply_patch_reinstall_requires_env_path(self, tmp_path: Path): - """Test that reinstall=True without env_path raises ValueError.""" + def test_apply_patch_restore_requires_env_path(self, tmp_path: Path): + """Test that restore=True without env_path raises ValueError.""" site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") with pytest.raises(ValueError, match="env_path is required"): - apply_patch(patch_file, site_packages, reinstall=True) + apply_patch(patch_file, site_packages, restore=True) - def test_apply_patch_reinstall_dry_run_failure_logs_error( + def test_apply_patch_restore_dry_run_failure_logs_error( self, tmp_path: Path, caplog ): - """Test that a broken patch after reinstall logs an error (not 'already applied').""" + """Test that a broken patch after restore logs an error (not 'already applied').""" site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") @@ -267,7 +267,7 @@ def side_effect(cmd, *args, **kwargs): raise subprocess.CalledProcessError(1, "patch") with patch("subprocess.check_call", side_effect=side_effect): - apply_patch(patch_file, site_packages, env_path=env_path, reinstall=True) + apply_patch(patch_file, site_packages, env_path=env_path, restore=True) assert "already applied" not in caplog.text assert "Failed to apply patch" in caplog.text From e962eab29eccaacf4f0aaa9efc05924766e34c1e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:24:37 +0000 Subject: [PATCH 03/12] fix: remove unused variable and apply ruff format https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- tests/test_core.py | 2 -- tests/test_e2e.py | 12 +++--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index aa5fd54..28239e3 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -259,8 +259,6 @@ def test_apply_patch_restore_dry_run_failure_logs_error( patch_file.write_text("some patch content") env_path = tmp_path / ".venv" - calls = [None] # first call (restore) succeeds; second (dry-run) fails - def side_effect(cmd, *args, **kwargs): if cmd[0] == "uv": return None diff --git a/tests/test_e2e.py b/tests/test_e2e.py index b29d083..309db2b 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -31,9 +31,7 @@ def project(tmp_path, monkeypatch): monkeypatch.chdir(project_dir) target_env = project_dir / ".venv" - subprocess.check_call( - ["uv", "venv", str(target_env), "--python", sys.executable] - ) + subprocess.check_call(["uv", "venv", str(target_env), "--python", sys.executable]) subprocess.check_call( [ "uv", @@ -84,9 +82,7 @@ def test_new_workspace_has_existing_patch_applied( # Workspace 2 with amend: should carry over the patch ws2 = tmp_path / "ws2" monkeypatch.setattr(tempfile, "mkdtemp", _make_mock_mkdtemp(ws2)) - prepare_patch_workspace( - module_path, PACKAGE, version, target_env, amend=True - ) + prepare_patch_workspace(module_path, PACKAGE, version, target_env, amend=True) ws2_sp = find_site_packages(ws2 / "venv") assert "# patched by e2e test" in (ws2_sp / "six.py").read_text() @@ -148,9 +144,7 @@ def test_amend_with_bad_patch_recovers_to_clean_state( ws = tmp_path / "ws" monkeypatch.setattr(tempfile, "mkdtemp", _make_mock_mkdtemp(ws)) - prepare_patch_workspace( - module_path, PACKAGE, version, target_env, amend=True - ) + prepare_patch_workspace(module_path, PACKAGE, version, target_env, amend=True) ws_sp = find_site_packages(ws / "venv") git_path = ws_sp.parent From 4f4aac063d7214a9ee03d98ec0a633d5f8d8aa83 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:31:19 +0000 Subject: [PATCH 04/12] fix(commit): update --skip-restore description to use consistent "restore" terminology https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- patch_package_py/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patch_package_py/cli.py b/patch_package_py/cli.py index 4f7b27a..a6e399a 100644 --- a/patch_package_py/cli.py +++ b/patch_package_py/cli.py @@ -140,7 +140,7 @@ def cli(): commit_parser.add_argument( "--skip-restore", action="store_true", - help="Skip reinstalling the target package before applying the new patch", + help="Skip restoring the clean package before applying the new patch", ) commit_parser.set_defaults(func=cmd_commit) From fe813505a06e8b01c6aa572fe30d04f853cb0f74 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:31:57 +0000 Subject: [PATCH 05/12] docs(skill): document --restore flag for apply command https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- skills/patch-package-py/SKILL.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/skills/patch-package-py/SKILL.md b/skills/patch-package-py/SKILL.md index 77c4d61..68f40dd 100644 --- a/skills/patch-package-py/SKILL.md +++ b/skills/patch-package-py/SKILL.md @@ -16,7 +16,7 @@ The CLI has three commands: ```bash p12y patch [-e ] [--amend] p12y commit [--skip-restore] -p12y apply [-e ] +p12y apply [-e ] [--restore] ``` ## CLI availability and uv-based workflow @@ -99,10 +99,9 @@ Use this sequence when guiding a user through patching a package: uv run p12y commit ``` - `commit` writes the patch file, reinstalls the original package in the + `commit` writes the patch file, restores the clean package in the environment selected during `patch`, then applies the new patch. Use - `--skip-restore` when the target environment is already prepared for direct - patch application. + `--skip-restore` when the target environment is already in a clean state. 6. Apply all patch files in `patches/` to the selected environment: @@ -116,6 +115,12 @@ Use this sequence when guiding a user through patching a package: uv run p12y apply -e ``` + Use `--restore` to restore each package to its clean state before applying the patch. This is useful when the environment may already contain a previous version of the patch or manual edits: + + ```bash + uv run p12y apply --restore + ``` + 7. Run a small import verification check for the patched behavior. ## Custom environment paths @@ -138,5 +143,5 @@ Run `uv run p12y commit ` from the project root so the patch file lan - `Could not determine site-packages directory`: pass a virtual environment path that contains `Lib/site-packages` on Windows or `lib/python*/site-packages` on Unix-like systems. - `Version mismatch`: recreate the patch for the installed version, or install the package version named in the patch file. - `Invalid patch file name format`: use `patches/+.patch`. -- `appears to be already applied`: the selected environment already contains the patch changes. +- `appears to be already applied`: the selected environment already contains the patch changes. Run `uv run p12y apply --restore` to restore the clean package first and then re-apply. - `No changes detected`: edit files inside the path printed by `uv run p12y patch`, then rerun `uv run p12y commit `. From 83d82ccc37153918c627014518621f5cad2732a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:41:05 +0000 Subject: [PATCH 06/12] fix(apply): restore before validation; raise on dry-run failure in restore mode - Move restore_clean_package call before resolve/version checks so --restore can recover a missing or version-drifted package - Raise RuntimeError (non-zero exit) when dry-run fails after restore instead of silently returning https://claude.ai/code/session_01UUAdSdSQWt6eAP11HMQU3J --- patch_package_py/core.py | 23 +++++++++++------------ tests/test_core.py | 14 ++++++-------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index a92c350..952db3b 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -330,6 +330,11 @@ def apply_patch( package_name, version = patch_name.rsplit("+", 1) + if restore: + if env_path is None: + raise ValueError("env_path is required when restore=True") + restore_clean_package(package_name, version, env_path) + # Verify the package exists in site_packages_dir with matching version resolver = Resolver() result = resolver.resolve_in_site_packages(site_packages_dir, package_name) @@ -345,11 +350,6 @@ def apply_patch( f"Version mismatch: patch is for {package_name}=={version} but installed version is {installed_version}" ) - if restore: - if env_path is None: - raise ValueError("env_path is required when restore=True") - restore_clean_package(package_name, version, env_path) - # First, check if the patch is already applied using dry-run try: subprocess.check_call( @@ -368,13 +368,12 @@ def apply_patch( ) except subprocess.CalledProcessError: if restore: - logger.error( - f"Failed to apply patch `{patch_name}` after restoring clean package.", - ) - else: - logger.warning( - f"Patch `{patch_name}` appears to be already applied, skipping...", - ) + raise RuntimeError( + f"Failed to apply patch `{patch_name}` after restoring clean package." + ) from None + logger.warning( + f"Patch `{patch_name}` appears to be already applied, skipping...", + ) return # If dry-run succeeds, apply the patch for real diff --git a/tests/test_core.py b/tests/test_core.py index 28239e3..0c74619 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -250,10 +250,8 @@ def test_apply_patch_restore_requires_env_path(self, tmp_path: Path): with pytest.raises(ValueError, match="env_path is required"): apply_patch(patch_file, site_packages, restore=True) - def test_apply_patch_restore_dry_run_failure_logs_error( - self, tmp_path: Path, caplog - ): - """Test that a broken patch after restore logs an error (not 'already applied').""" + def test_apply_patch_restore_dry_run_failure_raises(self, tmp_path: Path): + """Test that a broken patch after restore raises RuntimeError (non-zero exit).""" site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") @@ -264,12 +262,12 @@ def side_effect(cmd, *args, **kwargs): return None raise subprocess.CalledProcessError(1, "patch") - with patch("subprocess.check_call", side_effect=side_effect): + with ( + patch("subprocess.check_call", side_effect=side_effect), + pytest.raises(RuntimeError, match="Failed to apply patch"), + ): apply_patch(patch_file, site_packages, env_path=env_path, restore=True) - assert "already applied" not in caplog.text - assert "Failed to apply patch" in caplog.text - class TestCommitChanges: """Tests for creating patch files via commit_changes.""" From 7a03878e140f63c28fa89c49d1f9db64c83e62e3 Mon Sep 17 00:00:00 2001 From: cxx Date: Tue, 26 May 2026 23:55:19 +0800 Subject: [PATCH 07/12] fix(apply): wrap real patch apply in restore-aware error handling When restore=True and the real patch application fails (after a successful dry-run), raise RuntimeError instead of leaking a raw CalledProcessError. Adds test coverage for this scenario. Co-Authored-By: Claude Opus 4.6 --- patch_package_py/core.py | 29 ++++++++++++++++++----------- tests/test_core.py | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index 952db3b..8f24653 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -377,14 +377,21 @@ def apply_patch( return # If dry-run succeeds, apply the patch for real - subprocess.check_call( - [ - "patch", - "-p1", - "-N", - "--forward", - "-i", - str(patch_file.absolute()), - ], - cwd=site_packages_dir, - ) + try: + subprocess.check_call( + [ + "patch", + "-p1", + "-N", + "--forward", + "-i", + str(patch_file.absolute()), + ], + cwd=site_packages_dir, + ) + except subprocess.CalledProcessError: + if restore: + raise RuntimeError( + f"Failed to apply patch `{patch_name}` after restoring clean package." + ) from None + raise diff --git a/tests/test_core.py b/tests/test_core.py index 0c74619..123373c 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -268,6 +268,31 @@ def side_effect(cmd, *args, **kwargs): ): apply_patch(patch_file, site_packages, env_path=env_path, restore=True) + def test_apply_patch_restore_real_apply_failure_raises(self, tmp_path: Path): + """Test that a real apply failure after successful dry-run raises RuntimeError.""" + site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + patch_file = tmp_path / "mypackage+1.0.0.patch" + patch_file.write_text("some patch content") + env_path = tmp_path / ".venv" + + call_count = 0 + + def side_effect(cmd, *args, **kwargs): + nonlocal call_count + call_count += 1 + if cmd[0] == "uv": + return None + # First patch call (dry-run) succeeds, second (real apply) fails + if call_count == 3: + raise subprocess.CalledProcessError(1, "patch") + return None + + with ( + patch("subprocess.check_call", side_effect=side_effect), + pytest.raises(RuntimeError, match="Failed to apply patch"), + ): + apply_patch(patch_file, site_packages, env_path=env_path, restore=True) + class TestCommitChanges: """Tests for creating patch files via commit_changes.""" From b527f1c1dd910dd5470fc8d70c2762da00a2475e Mon Sep 17 00:00:00 2001 From: cxx Date: Wed, 27 May 2026 00:02:17 +0800 Subject: [PATCH 08/12] test: move restore happy path to e2e, keep error cases as unit tests The restore happy path test now uses a real venv and real patch command instead of mocking subprocess. Error branch tests (missing env_path, dry-run failure, real apply failure) stay in unit tests since they need to simulate specific failure scenarios. Co-Authored-By: Claude Opus 4.6 --- tests/test_core.py | 31 ------------------------------- tests/test_e2e.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index 123373c..bf2c5a7 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -210,37 +210,6 @@ def test_apply_patch_already_applied(self, tmp_path: Path, caplog): assert "already applied" in caplog.text - def test_apply_patch_restore_calls_restore(self, tmp_path: Path): - """Test that restore=True calls restore_clean_package before patching.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") - patch_file = tmp_path / "mypackage+1.0.0.patch" - patch_file.write_text( - "--- a/mypackage/core.py\n" - "+++ b/mypackage/core.py\n" - "@@ -1,2 +1,2 @@\n" - " def hello():\n" - "- return 'hello'\n" - "+ return 'hello world'\n" - ) - env_path = tmp_path / ".venv" - - with patch("subprocess.check_call") as mock_check_call: - apply_patch(patch_file, site_packages, env_path=env_path, restore=True) - - commands = [call_args.args[0] for call_args in mock_check_call.call_args_list] - assert commands[0] == [ - "uv", - "pip", - "install", - "--force-reinstall", - "--no-deps", - "mypackage==1.0.0", - "--python", - str(venv_python(env_path)), - ] - # dry-run + actual apply should follow - assert mock_check_call.call_count == 3 - def test_apply_patch_restore_requires_env_path(self, tmp_path: Path): """Test that restore=True without env_path raises ValueError.""" site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 309db2b..8c2fdab 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -6,6 +6,7 @@ from patch_package_py.core import ( Resolver, + apply_patch, commit_changes, find_site_packages, prepare_patch_workspace, @@ -170,3 +171,34 @@ def test_amend_with_bad_patch_recovers_to_clean_state( text=True, ) assert ignored == "" + + +class TestRestoreApplyE2E: + def test_restore_reinstalls_then_applies_patch(self, tmp_path, monkeypatch, project): + """--restore should reinstall the package to a clean state, then apply the patch.""" + module_path = project["module_path"] + version = project["version"] + target_env = project["target_env"] + + # Create a patch via the normal workflow + ws = tmp_path / "ws" + monkeypatch.setattr(tempfile, "mkdtemp", _make_mock_mkdtemp(ws)) + prepare_patch_workspace(module_path, PACKAGE, version, target_env) + + ws_sp = find_site_packages(ws / "venv") + six_py = ws_sp / "six.py" + original = six_py.read_text() + six_py.write_text(original + "\n# restore-e2e-marker\n") + + commit_changes(PACKAGE, version, ws_sp, target_env) + + patch_file = project["dir"] / "patches" / f"{PACKAGE}+{version}.patch" + assert patch_file.exists() + + # The patch is now applied in target_env. Apply again with --restore + # should succeed (reinstall cleans the already-patched state). + site_packages = find_site_packages(target_env) + apply_patch(patch_file, site_packages, env_path=target_env, restore=True) + + patched_content = (site_packages / "six.py").read_text() + assert "# restore-e2e-marker" in patched_content From 67825d057fc927b36bfade567004d9511eafaf0c Mon Sep 17 00:00:00 2001 From: cxx Date: Wed, 27 May 2026 00:10:50 +0800 Subject: [PATCH 09/12] fix(apply): move restore after version check to prevent silent version change Previously --restore ran before version validation, which meant it would force-reinstall the version from the patch filename regardless of what the project actually had installed. Now version mismatch is caught first, and restore only reinstalls when the versions already match. Co-Authored-By: Claude Opus 4.6 --- patch_package_py/core.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index 8f24653..e9ab61e 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -330,11 +330,6 @@ def apply_patch( package_name, version = patch_name.rsplit("+", 1) - if restore: - if env_path is None: - raise ValueError("env_path is required when restore=True") - restore_clean_package(package_name, version, env_path) - # Verify the package exists in site_packages_dir with matching version resolver = Resolver() result = resolver.resolve_in_site_packages(site_packages_dir, package_name) @@ -350,6 +345,11 @@ def apply_patch( f"Version mismatch: patch is for {package_name}=={version} but installed version is {installed_version}" ) + if restore: + if env_path is None: + raise ValueError("env_path is required when restore=True") + restore_clean_package(package_name, version, env_path) + # First, check if the patch is already applied using dry-run try: subprocess.check_call( From b685905fc7af01adb2f93add4f6a0eedb89a944d Mon Sep 17 00:00:00 2001 From: cxx Date: Wed, 27 May 2026 00:13:42 +0800 Subject: [PATCH 10/12] fix(apply): preserve exception chain in restore error handling Remove `from None` so the underlying CalledProcessError is chained, making failures easier to diagnose. Co-Authored-By: Claude Opus 4.6 --- patch_package_py/core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index e9ab61e..0743f5b 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -366,11 +366,11 @@ def apply_patch( stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, ) - except subprocess.CalledProcessError: + except subprocess.CalledProcessError as e: if restore: raise RuntimeError( f"Failed to apply patch `{patch_name}` after restoring clean package." - ) from None + ) from e logger.warning( f"Patch `{patch_name}` appears to be already applied, skipping...", ) @@ -389,9 +389,9 @@ def apply_patch( ], cwd=site_packages_dir, ) - except subprocess.CalledProcessError: + except subprocess.CalledProcessError as e: if restore: raise RuntimeError( f"Failed to apply patch `{patch_name}` after restoring clean package." - ) from None + ) from e raise From 09f48a1b20a6d5119e5826c6d91a10dd4762b100 Mon Sep 17 00:00:00 2001 From: cxx Date: Sat, 30 May 2026 10:15:26 +0800 Subject: [PATCH 11/12] refactor(apply): replace redundant site_packages_dir param with env_path apply_patch now derives site_packages_dir internally via find_site_packages, eliminating the redundant parameter that callers always computed from env_path. Co-Authored-By: Claude Opus 4.6 --- patch_package_py/cli.py | 12 ++++++--- patch_package_py/core.py | 10 +++----- tests/test_core.py | 54 +++++++++++++++++----------------------- tests/test_e2e.py | 5 ++-- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/patch_package_py/cli.py b/patch_package_py/cli.py index a6e399a..0bb5cf7 100644 --- a/patch_package_py/cli.py +++ b/patch_package_py/cli.py @@ -78,11 +78,18 @@ def cmd_apply(args): env_path = ( Path(args.env_path) if args.env_path is not None else Path.cwd() / ".venv" ) - site_packages_dir = find_site_packages(env_path) if not patches_dir.exists(): return + try: + site_packages_dir = find_site_packages(env_path) + except FileNotFoundError: + logger.error( + f"Error: Could not determine site-packages directory for {env_path}", + ) + sys.exit(1) + if not site_packages_dir.exists(): logger.error( f"Error: Site-packages directory {site_packages_dir} does not exist", @@ -98,8 +105,7 @@ def cmd_apply(args): for patch_file in patch_files: apply_patch( patch_file, - site_packages_dir, - env_path=env_path if args.restore else None, + env_path, restore=args.restore, ) diff --git a/patch_package_py/core.py b/patch_package_py/core.py index 0743f5b..6a9301c 100644 --- a/patch_package_py/core.py +++ b/patch_package_py/core.py @@ -292,9 +292,8 @@ def commit_changes( if restore_target_package: restore_clean_package(package_name, version, target_env_path) - current_site_packages = find_site_packages(target_env_path) try: - apply_patch(patch_file_path, current_site_packages) + apply_patch(patch_file_path, target_env_path) except subprocess.CalledProcessError: msg = "Error: failed to apply the patch after creation." if restore_target_package: @@ -315,9 +314,8 @@ def commit_changes( def apply_patch( patch_file: Path, - site_packages_dir: Path, + env_path: Path, *, - env_path: Union[Path, None] = None, restore: bool = False, ) -> None: # Parse package name and version from patch file name @@ -330,6 +328,8 @@ def apply_patch( package_name, version = patch_name.rsplit("+", 1) + site_packages_dir = find_site_packages(env_path) + # Verify the package exists in site_packages_dir with matching version resolver = Resolver() result = resolver.resolve_in_site_packages(site_packages_dir, package_name) @@ -346,8 +346,6 @@ def apply_patch( ) if restore: - if env_path is None: - raise ValueError("env_path is required when restore=True") restore_clean_package(package_name, version, env_path) # First, check if the patch is already applied using dry-run diff --git a/tests/test_core.py b/tests/test_core.py index bf2c5a7..9495628 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -125,10 +125,14 @@ def test_resolve_in_site_packages_not_found(self, tmp_path: Path): class TestApplyPatch: """Integration tests for the apply_patch workflow.""" - def _setup_site_packages(self, tmp_path: Path, package_name: str, version: str): - """Helper to create a mock site-packages with a package installed.""" - site_packages = tmp_path / "site-packages" - site_packages.mkdir() + def _setup_env(self, tmp_path: Path, package_name: str, version: str): + """Helper to create a mock venv with a package installed.""" + env_path = tmp_path / ".venv" + if os.name == "nt": + site_packages = env_path / "Lib" / "site-packages" + else: + site_packages = env_path / "lib" / "python3.9" / "site-packages" + site_packages.mkdir(parents=True) # Create dist-info dist_info = ( @@ -146,40 +150,40 @@ def _setup_site_packages(self, tmp_path: Path, package_name: str, version: str): (pkg_dir / "__init__.py").write_text('__version__ = "1.0.0"\n') (pkg_dir / "core.py").write_text("def hello():\n return 'hello'\n") - return site_packages + return env_path def test_apply_patch_invalid_name_format(self, tmp_path: Path, caplog): """Test that invalid patch file name is skipped.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "invalid_name.patch" patch_file.write_text("some patch content") - apply_patch(patch_file, site_packages) + apply_patch(patch_file, env_path) assert "Invalid patch file name format" in caplog.text def test_apply_patch_package_not_found(self, tmp_path: Path, caplog): """Test that missing package is skipped.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "otherpackage+1.0.0.patch" patch_file.write_text("some patch content") - apply_patch(patch_file, site_packages) + apply_patch(patch_file, env_path) assert "not found in site-packages" in caplog.text def test_apply_patch_version_mismatch(self, tmp_path: Path): """Test that version mismatch raises error.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+2.0.0.patch" patch_file.write_text("some patch content") with pytest.raises(ValueError, match="Version mismatch"): - apply_patch(patch_file, site_packages) + apply_patch(patch_file, env_path) def test_apply_patch_success(self, tmp_path: Path): """Test successful patch application.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text( "--- a/mypackage/core.py\n" @@ -191,40 +195,29 @@ def test_apply_patch_success(self, tmp_path: Path): ) with patch("subprocess.check_call") as mock_check_call: - apply_patch(patch_file, site_packages) + apply_patch(patch_file, env_path) # Should be called twice: dry-run and actual apply assert mock_check_call.call_count == 2 def test_apply_patch_already_applied(self, tmp_path: Path, caplog): """Test that already applied patch is skipped.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") with patch("subprocess.check_call") as mock_check_call: - # Simulate dry-run failure (patch already applied) mock_check_call.side_effect = subprocess.CalledProcessError(1, "patch") - apply_patch(patch_file, site_packages) + apply_patch(patch_file, env_path) assert "already applied" in caplog.text - def test_apply_patch_restore_requires_env_path(self, tmp_path: Path): - """Test that restore=True without env_path raises ValueError.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") - patch_file = tmp_path / "mypackage+1.0.0.patch" - patch_file.write_text("some patch content") - - with pytest.raises(ValueError, match="env_path is required"): - apply_patch(patch_file, site_packages, restore=True) - def test_apply_patch_restore_dry_run_failure_raises(self, tmp_path: Path): """Test that a broken patch after restore raises RuntimeError (non-zero exit).""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") - env_path = tmp_path / ".venv" def side_effect(cmd, *args, **kwargs): if cmd[0] == "uv": @@ -235,14 +228,13 @@ def side_effect(cmd, *args, **kwargs): patch("subprocess.check_call", side_effect=side_effect), pytest.raises(RuntimeError, match="Failed to apply patch"), ): - apply_patch(patch_file, site_packages, env_path=env_path, restore=True) + apply_patch(patch_file, env_path, restore=True) def test_apply_patch_restore_real_apply_failure_raises(self, tmp_path: Path): """Test that a real apply failure after successful dry-run raises RuntimeError.""" - site_packages = self._setup_site_packages(tmp_path, "mypackage", "1.0.0") + env_path = self._setup_env(tmp_path, "mypackage", "1.0.0") patch_file = tmp_path / "mypackage+1.0.0.patch" patch_file.write_text("some patch content") - env_path = tmp_path / ".venv" call_count = 0 @@ -260,7 +252,7 @@ def side_effect(cmd, *args, **kwargs): patch("subprocess.check_call", side_effect=side_effect), pytest.raises(RuntimeError, match="Failed to apply patch"), ): - apply_patch(patch_file, site_packages, env_path=env_path, restore=True) + apply_patch(patch_file, env_path, restore=True) class TestCommitChanges: diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 8c2fdab..e441bc8 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -197,8 +197,7 @@ def test_restore_reinstalls_then_applies_patch(self, tmp_path, monkeypatch, proj # The patch is now applied in target_env. Apply again with --restore # should succeed (reinstall cleans the already-patched state). - site_packages = find_site_packages(target_env) - apply_patch(patch_file, site_packages, env_path=target_env, restore=True) + apply_patch(patch_file, target_env, restore=True) - patched_content = (site_packages / "six.py").read_text() + patched_content = (find_site_packages(target_env) / "six.py").read_text() assert "# restore-e2e-marker" in patched_content From d9a8db1df251cd1b3de12f3831bfa529b1062598 Mon Sep 17 00:00:00 2001 From: cxx Date: Sat, 30 May 2026 10:30:26 +0800 Subject: [PATCH 12/12] fix(test): align terminology from "reinstall" to "restore" in e2e tests Co-Authored-By: Claude Opus 4.6 --- tests/test_e2e.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index e441bc8..7571ad6 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -175,7 +175,7 @@ def test_amend_with_bad_patch_recovers_to_clean_state( class TestRestoreApplyE2E: def test_restore_reinstalls_then_applies_patch(self, tmp_path, monkeypatch, project): - """--restore should reinstall the package to a clean state, then apply the patch.""" + """--restore should restore the package to a clean state, then apply the patch.""" module_path = project["module_path"] version = project["version"] target_env = project["target_env"] @@ -196,7 +196,7 @@ def test_restore_reinstalls_then_applies_patch(self, tmp_path, monkeypatch, proj assert patch_file.exists() # The patch is now applied in target_env. Apply again with --restore - # should succeed (reinstall cleans the already-patched state). + # should succeed (restore cleans the already-patched state). apply_patch(patch_file, target_env, restore=True) patched_content = (find_site_packages(target_env) / "six.py").read_text()