diff --git a/changelog.d/1847.bugfix.md b/changelog.d/1847.bugfix.md new file mode 100644 index 0000000000..aa7638257f --- /dev/null +++ b/changelog.d/1847.bugfix.md @@ -0,0 +1 @@ +Keep pipx startup usable when locked files prevent trash cleanup. diff --git a/src/pipx/util.py b/src/pipx/util.py index 8008d64f09..d0730f852c 100644 --- a/src/pipx/util.py +++ b/src/pipx/util.py @@ -53,8 +53,10 @@ def rmdir(path: Path, safe_rm: bool = True) -> None: # Windows doesn't let us delete or overwrite files that are being run # But it does let us rename/move it. To get around this issue, we can move # the file to a temporary folder (to be deleted at a later time) - # So, if safe_rm is True, we ignore any errors and move the file to the trash with below code - shutil.rmtree(path, ignore_errors=safe_rm) + # Always ignore errors so locked files don't make cleanup fatal. If the + # directory is still present afterwards, safe_rm controls whether it is + # moved to the trash or left in place with a warning. + shutil.rmtree(path, ignore_errors=True) # move it to be deleted later if it still exists if path.is_dir(): diff --git a/tests/test_util.py b/tests/test_util.py index 5c2dcd9d08..04d7055e3d 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,10 +1,38 @@ from __future__ import annotations +import logging import sys +from typing import TYPE_CHECKING import pytest -from pipx.util import run_subprocess +from pipx.util import rmdir, run_subprocess + +if TYPE_CHECKING: + from pathlib import Path + + +def test_rmdir_without_safe_rm_is_non_fatal_for_locked_files( + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + trash_dir = tmp_path / "trash" + trash_dir.mkdir() + (trash_dir / "locked.dll").write_text("locked") + + def fake_rmtree(path: Path, ignore_errors: bool = False) -> None: + assert path == trash_dir + if not ignore_errors: + raise PermissionError("locked file") + + monkeypatch.setattr("pipx.util.shutil.rmtree", fake_rmtree) + + with caplog.at_level(logging.WARNING, logger="pipx.util"): + rmdir(trash_dir, safe_rm=False) + + assert trash_dir.is_dir() + assert f"Failed to delete {trash_dir}. You may need to delete it manually." in caplog.text @pytest.mark.parametrize(