Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate uninstallation errors to diagnostic error format #12463

Merged
merged 1 commit into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading