Extend permission error fix to reinstall and uninstall

docs/changelog.md

@@ -2,7 +2,7 @@ dev
 
 - Fix to `pipx ensurepath` to fix behavior in user locales other than UTF-8, to fix #644. The internal change is to use userpath v1.6.0 or greater. (#700)
 - Fix virtual environment inspection for Python releases that uses an int for its release serial number. (#706)
-- Fix PermissionError when overwriting a running file on windows. i.e. via `pipx upgrade pipx`
+- Fix PermissionError when deleting or overwriting a running file on windows. i.e. via `pipx upgrade pipx`
 
 0.16.3
 

src/pipx/commands/common.py

@@ -18,7 +18,7 @@
 from pipx.emojis import hazard, stars
 from pipx.package_specifier import parse_specifier_for_install, valid_pypi_name
 from pipx.pipx_metadata_file import PackageInfo
-from pipx.util import PipxError, mkdir, pipx_wrap, rmdir
+from pipx.util import PipxError, mkdir, pipx_wrap, rmdir, safe_unlink
 from pipx.venv import Venv
 
 logger = logging.getLogger(__name__)
@@ -93,11 +93,7 @@ def _copy_package_apps(
             mkdir(dest.parent)
         if dest.exists():
             logger.warning(f"{hazard}  Overwriting file {str(dest)} with {str(src)}")
-            try:
-                dest.unlink()
-            except OSError:
-                tmp_dest = PIPX_TEMP_DIR / dest.name
-                dest.rename(tmp_dest)
+            safe_unlink(dest)
         if src.exists():
             shutil.copy(src, dest)
 

src/pipx/commands/reinstall.py

@@ -8,8 +8,13 @@
 from pipx.commands.inject import inject_dep
 from pipx.commands.install import install
 from pipx.commands.uninstall import uninstall
-from pipx.constants import EXIT_CODE_OK, EXIT_CODE_REINSTALL_VENV_NONEXISTENT, ExitCode
-from pipx.emojis import sleep
+from pipx.constants import (
+    EXIT_CODE_OK,
+    EXIT_CODE_REINSTALL_INVALID_PYTHON,
+    EXIT_CODE_REINSTALL_VENV_NONEXISTENT,
+    ExitCode,
+)
+from pipx.emojis import error, hazard, sleep
 from pipx.util import PipxError
 from pipx.venv import Venv, VenvContainer
 
@@ -22,6 +27,13 @@ def reinstall(
         print(f"Nothing to reinstall for {venv_dir.name} {sleep}")
         return EXIT_CODE_REINSTALL_VENV_NONEXISTENT
 
+    if Path(python).is_relative_to(venv_dir):
+        print(
+            f"{error} Error, the python executable would be deleted!",
+             "Change it using the --python option or PIPX_DEFAULT_PYTHON environment variable."
+        )
+        return EXIT_CODE_REINSTALL_INVALID_PYTHON
+
     venv = Venv(venv_dir, verbose=verbose)
 
     if venv.pipx_metadata.main_package.package_or_url is not None:

src/pipx/commands/uninstall.py

@@ -16,7 +16,7 @@
 )
 from pipx.emojis import hazard, sleep, stars
 from pipx.pipx_metadata_file import PackageInfo
-from pipx.util import rmdir
+from pipx.util import rmdir, safe_unlink
 from pipx.venv import Venv, VenvContainer
 from pipx.venv_inspect import VenvMetadata
 
@@ -125,7 +125,7 @@ def uninstall(venv_dir: Path, local_bin_dir: Path, verbose: bool) -> ExitCode:
 
     for bin_dir_app_path in bin_dir_app_paths:
         try:
-            bin_dir_app_path.unlink()
+            safe_unlink(bin_dir_app_path)
         except FileNotFoundError:
             logger.info(f"tried to remove but couldn't find {bin_dir_app_path}")
         else:

src/pipx/constants.py

@@ -27,6 +27,7 @@
 EXIT_CODE_UNINSTALL_VENV_NONEXISTENT = ExitCode(1)
 EXIT_CODE_UNINSTALL_ERROR = ExitCode(1)
 EXIT_CODE_REINSTALL_VENV_NONEXISTENT = ExitCode(1)
+EXIT_CODE_REINSTALL_INVALID_PYTHON = ExitCode(1)
 
 pipx_log_file: Optional[Path] = None
 

src/pipx/emojis.py

@@ -28,8 +28,10 @@ def use_emojis() -> bool:
 if EMOJI_SUPPORT:
     stars = "✨ 🌟 ✨"
     hazard = "⚠️"
+    error = "⛔"
     sleep = "😴"
 else:
     stars = ""
     hazard = ""
+    error = ""
     sleep = ""

src/pipx/main.py

@@ -25,7 +25,7 @@
 from pipx.constants import ExitCode
 from pipx.emojis import hazard
 from pipx.interpreter import DEFAULT_PYTHON
-from pipx.util import PipxError, mkdir, pipx_wrap
+from pipx.util import PipxError, mkdir, pipx_wrap, rmdir
 from pipx.venv import VenvContainer
 from pipx.version import __version__
 
@@ -721,12 +721,8 @@ def setup(args: argparse.Namespace) -> None:
     mkdir(constants.LOCAL_BIN_DIR)
     mkdir(constants.PIPX_VENV_CACHEDIR)
 
+    rmdir(constants.PIPX_TEMP_DIR, False)
     mkdir(constants.PIPX_TEMP_DIR)
-    temp_children = constants.PIPX_TEMP_DIR.iterdir()
-    if temp_children:
-        logger.debug("Cleaning temp folder")
-        for f in temp_children:
-            f.unlink()
 
     old_pipx_venv_location = constants.PIPX_LOCAL_VENVS / "pipx-app"
     if old_pipx_venv_location.exists():

src/pipx/util.py

@@ -1,7 +1,9 @@
 import logging
 import os
+import random
 import re
 import shutil
+import string
 import subprocess
 import sys
 import textwrap
@@ -21,7 +23,7 @@
 
 import pipx.constants
 from pipx.animate import show_cursor
-from pipx.constants import WINDOWS
+from pipx.constants import PIPX_TEMP_DIR, WINDOWS
 
 logger = logging.getLogger(__name__)
 
@@ -38,8 +40,12 @@ class RelevantSearch(NamedTuple):
     pattern: Pattern[str]
     category: str
 
+def _get_temp_file(path: Path) -> Path:
+    prefix = "".join(random.choices(string.ascii_lowercase, k=8))
+    return PIPX_TEMP_DIR / f"{prefix}.{path.name}"
 
-def rmdir(path: Path) -> None:
+
+def rmdir(path: Path, safe_rm: bool = True) -> None:
     logger.info(f"removing directory {path}")
     try:
         if WINDOWS:
@@ -49,6 +55,20 @@ def rmdir(path: Path) -> None:
     except FileNotFoundError:
         pass
 
+    # move it to be deleted later if it still exists
+    if path.is_dir():
+        if safe_rm:
+            logger.warning(
+                f"Failed to delete {path}. Will moving it to a temp folder to delete later."
+            )
+
+            tmp_dir = _get_temp_file(path)
+            path.rename(tmp_dir)
+        else:
+            logger.warning(
+                f"Failed to delete {path}. You may need to delete it manually."
+            )
+
 
 def mkdir(path: Path) -> None:
     if path.is_dir():
@@ -57,6 +77,20 @@ def mkdir(path: Path) -> None:
     path.mkdir(parents=True, exist_ok=True)
 
 
+def safe_unlink(file: Path) -> 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)
+
+    if not file.is_file():
+        return
+    try:
+        file.unlink()
+    except PermissionError:
+        tmp_file = _get_temp_file(file)
+        file.rename(tmp_file)
+
+
 def get_pypackage_bin_path(binary_name: str) -> Path:
     return (
         Path("__pypackages__")