Skip to content

Commit

Permalink
Merge pull request #12463 from ichard26/errors/uninstall
Browse files Browse the repository at this point in the history
Migrate uninstallation errors to diagnostic error format
  • Loading branch information
pradyunsg committed May 4, 2024
2 parents e8e592a + 3b14deb commit d2f2da9
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 36 deletions.
1 change: 1 addition & 0 deletions news/10421.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reword and improve presentation of uninstallation errors.
2 changes: 0 additions & 2 deletions src/pip/_internal/cli/base_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
InstallationError,
NetworkConnectionError,
PreviousBuildDirError,
UninstallationError,
)
from pip._internal.utils.filesystem import check_path_owner
from pip._internal.utils.logging import BrokenStdoutLoggingError, setup_logging
Expand Down Expand Up @@ -192,7 +191,6 @@ def exc_logging_wrapper(*args: Any) -> int:
return PREVIOUS_BUILD_DIR_ERROR
except (
InstallationError,
UninstallationError,
BadCommand,
NetworkConnectionError,
) as exc:
Expand Down
46 changes: 42 additions & 4 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,6 @@ class InstallationError(PipError):
"""General exception during installation"""


class UninstallationError(PipError):
"""General exception during uninstallation"""


class MissingPyProjectBuildRequires(DiagnosticPipError):
"""Raised when pyproject.toml has `build-system`, but no `build-system.requires`."""

Expand Down Expand Up @@ -726,3 +722,45 @@ def from_config(
exc_info = logger.isEnabledFor(VERBOSE)
logger.warning("Failed to read %s", config, exc_info=exc_info)
return cls(None)


class UninstallMissingRecord(DiagnosticPipError):
reference = "uninstall-no-record-file"

def __init__(self, *, distribution: "BaseDistribution") -> None:
installer = distribution.installer
if not installer or installer == "pip":
dep = f"{distribution.raw_name}=={distribution.version}"
hint = Text.assemble(
"You might be able to recover from this via: ",
(f"pip install --force-reinstall --no-deps {dep}", "green"),
)
else:
hint = Text(
f"The package was installed by {installer}. "
"You should check if it can uninstall the package."
)

super().__init__(
message=Text(f"Cannot uninstall {distribution}"),
context=(
"The package's contents are unknown: "
f"no RECORD file was found for {distribution.raw_name}."
),
hint_stmt=hint,
)


class LegacyDistutilsInstall(DiagnosticPipError):
reference = "uninstall-distutils-installed-package"

def __init__(self, *, distribution: "BaseDistribution") -> None:
super().__init__(
message=Text(f"Cannot uninstall {distribution}"),
context=(
"It is a distutils installed project and thus we cannot accurately "
"determine which files belong to it which would lead to only a partial "
"uninstall."
),
hint_stmt=None,
)
22 changes: 4 additions & 18 deletions src/pip/_internal/req/req_uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from importlib.util import cache_from_source
from typing import Any, Callable, Dict, Generator, Iterable, List, Optional, Set, Tuple

from pip._internal.exceptions import UninstallationError
from pip._internal.exceptions import LegacyDistutilsInstall, UninstallMissingRecord
from pip._internal.locations import get_bin_prefix, get_bin_user
from pip._internal.metadata import BaseDistribution
from pip._internal.utils.compat import WINDOWS
Expand Down Expand Up @@ -61,7 +61,7 @@ def uninstallation_paths(dist: BaseDistribution) -> Generator[str, None, None]:
UninstallPathSet.add() takes care of the __pycache__ .py[co].
If RECORD is not found, raises UninstallationError,
If RECORD is not found, raises an error,
with possible information from the INSTALLER file.
https://packaging.python.org/specifications/recording-installed-packages/
Expand All @@ -71,17 +71,7 @@ def uninstallation_paths(dist: BaseDistribution) -> Generator[str, None, None]:

entries = dist.iter_declared_entries()
if entries is None:
msg = f"Cannot uninstall {dist}, RECORD file not found."
installer = dist.installer
if not installer or installer == "pip":
dep = f"{dist.raw_name}=={dist.version}"
msg += (
" You might be able to recover from this via: "
f"'pip install --force-reinstall --no-deps {dep}'."
)
else:
msg += f" Hint: The package was installed by {installer}."
raise UninstallationError(msg)
raise UninstallMissingRecord(distribution=dist)

for entry in entries:
path = os.path.join(location, entry)
Expand Down Expand Up @@ -509,11 +499,7 @@ def from_dist(cls, dist: BaseDistribution) -> "UninstallPathSet":
paths_to_remove.add(f"{path}.pyo")

elif dist.installed_by_distutils:
raise UninstallationError(
f"Cannot uninstall {dist.raw_name!r}. It is a distutils installed "
"project and thus we cannot accurately determine which files belong "
"to it which would lead to only a partial uninstall."
)
raise LegacyDistutilsInstall(distribution=dist)

elif dist.installed_as_egg:
# package installed by easy_install
Expand Down
22 changes: 10 additions & 12 deletions tests/functional/test_uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ def test_basic_uninstall_distutils(script: PipTestEnvironment) -> None:
result = script.pip(
"uninstall", "distutils_install", "-y", expect_stderr=True, expect_error=True
)
assert "Cannot uninstall distutils-install 0.1" in result.stderr
assert (
"Cannot uninstall 'distutils-install'. It is a distutils installed "
"project and thus we cannot accurately determine which files belong "
"to it which would lead to only a partial uninstall."
"It is a distutils installed project and thus we cannot accurately determine "
"which files belong to it which would lead to only a partial uninstall."
) in result.stderr


Expand Down Expand Up @@ -590,18 +590,16 @@ def test_uninstall_without_record_fails(
installer_path.write_text(installer + os.linesep)

result2 = script.pip("uninstall", "simple.dist", "-y", expect_error=True)
expected_error_message = (
"ERROR: Cannot uninstall simple.dist 0.1, RECORD file not found."
)
assert "Cannot uninstall simple.dist 0.1" in result2.stderr
assert "no RECORD file was found for simple.dist" in result2.stderr
if not isinstance(installer, str) or not installer.strip() or installer == "pip":
expected_error_message += (
" You might be able to recover from this via: "
"'pip install --force-reinstall --no-deps "
"simple.dist==0.1'."
hint = (
"You might be able to recover from this via: "
"pip install --force-reinstall --no-deps simple.dist==0.1"
)
elif installer:
expected_error_message += f" Hint: The package was installed by {installer}."
assert result2.stderr.rstrip() == expected_error_message
hint = f"The package was installed by {installer}."
assert f"hint: {hint}" in result2.stderr
assert_all_changes(result.files_after, result2, ignore_changes)


Expand Down

0 comments on commit d2f2da9

Please sign in to comment.