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

Introduce DiagnosticPipError and use it "once" #10538

Merged
merged 3 commits into from
Oct 29, 2021
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
6 changes: 6 additions & 0 deletions src/pip/_internal/cli/base_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from pip._internal.exceptions import (
BadCommand,
CommandError,
DiagnosticPipError,
InstallationError,
NetworkConnectionError,
PreviousBuildDirError,
Expand Down Expand Up @@ -169,6 +170,11 @@ def exc_logging_wrapper(*args: Any) -> int:
logger.debug("Exception information:", exc_info=True)

return PREVIOUS_BUILD_DIR_ERROR
except DiagnosticPipError as exc:
logger.critical(str(exc))
logger.debug("Exception information:", exc_info=True)

return ERROR
except (
InstallationError,
UninstallationError,
Expand Down
122 changes: 120 additions & 2 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,102 @@
"""Exceptions used throughout package"""

import configparser
import re
from itertools import chain, groupby, repeat
from typing import TYPE_CHECKING, Dict, List, Optional, Union
from typing import TYPE_CHECKING, Dict, Iterator, List, Optional, Union

from pip._vendor.pkg_resources import Distribution
from pip._vendor.requests.models import Request, Response

if TYPE_CHECKING:
from hashlib import _Hash
from typing import Literal

from pip._internal.metadata import BaseDistribution
from pip._internal.req.req_install import InstallRequirement


#
# Scaffolding
#
def _is_kebab_case(s: str) -> bool:
return re.match(r"^[a-z]+(-[a-z]+)*$", s) is not None


def _prefix_with_indent(prefix: str, s: str, indent: Optional[str] = None) -> str:
if indent is None:
indent = " " * len(prefix)
else:
assert len(indent) == len(prefix)
message = s.replace("\n", "\n" + indent)
return f"{prefix}{message}\n"


class PipError(Exception):
"""Base pip exception"""
"""The base pip error."""


class DiagnosticPipError(PipError):
"""A pip error, that presents diagnostic information to the user.

This contains a bunch of logic, to enable pretty presentation of our error
messages. Each error gets a unique reference. Each error can also include
additional context, a hint and/or a note -- which are presented with the
main error message in a consistent style.
"""

reference: str

def __init__(
self,
*,
message: str,
context: Optional[str],
hint_stmt: Optional[str],
attention_stmt: Optional[str] = None,
reference: Optional[str] = None,
kind: 'Literal["error", "warning"]' = "error",
) -> None:

# Ensure a proper reference is provided.
if reference is None:
assert hasattr(self, "reference"), "error reference not provided!"
reference = self.reference
assert _is_kebab_case(reference), "error reference must be kebab-case!"

super().__init__(f"{reference}: {message}")

self.kind = kind
self.message = message
self.context = context

self.reference = reference
self.attention_stmt = attention_stmt
self.hint_stmt = hint_stmt

def __str__(self) -> str:
return "".join(self._string_parts())

def _string_parts(self) -> Iterator[str]:
# Present the main message, with relevant context indented.
yield f"{self.message}\n"
if self.context is not None:
yield f"\n{self.context}\n"

# Space out the note/hint messages.
if self.attention_stmt is not None or self.hint_stmt is not None:
yield "\n"

if self.attention_stmt is not None:
yield _prefix_with_indent("Note: ", self.attention_stmt)

if self.hint_stmt is not None:
yield _prefix_with_indent("Hint: ", self.hint_stmt)


#
# Actual Errors
#
class ConfigurationError(PipError):
"""General exception in configuration"""

Expand All @@ -30,6 +109,45 @@ class UninstallationError(PipError):
"""General exception during uninstallation"""


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

reference = "missing-pyproject-build-system-requires"

def __init__(self, *, package: str) -> None:
super().__init__(
message=f"Can not process {package}",
context=(
"This package has an invalid pyproject.toml file.\n"
"The [build-system] table is missing the mandatory `requires` key."
),
attention_stmt=(
"This is an issue with the package mentioned above, not pip."
),
hint_stmt="See PEP 518 for the detailed specification.",
)


class InvalidPyProjectBuildRequires(DiagnosticPipError):
"""Raised when pyproject.toml an invalid `build-system.requires`."""

reference = "invalid-pyproject-build-system-requires"

def __init__(self, *, package: str, reason: str) -> None:
super().__init__(
message=f"Can not process {package}",
context=(
"This package has an invalid `build-system.requires` key in "
"pyproject.toml.\n"
f"{reason}"
),
hint_stmt="See PEP 518 for the detailed specification.",
attention_stmt=(
"This is an issue with the package mentioned above, not pip."
),
)


class NoneMetadataError(PipError):
"""
Raised when accessing "METADATA" or "PKG-INFO" metadata for a
Expand Down
43 changes: 14 additions & 29 deletions src/pip/_internal/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from pip._vendor import tomli
from pip._vendor.packaging.requirements import InvalidRequirement, Requirement

from pip._internal.exceptions import InstallationError
from pip._internal.exceptions import (
InstallationError,
InvalidPyProjectBuildRequires,
MissingPyProjectBuildRequires,
)


def _is_list_of_str(obj: Any) -> bool:
Expand Down Expand Up @@ -119,47 +123,28 @@ def load_pyproject_toml(

# Ensure that the build-system section in pyproject.toml conforms
# to PEP 518.
error_template = (
"{package} has a pyproject.toml file that does not comply "
"with PEP 518: {reason}"
)

# Specifying the build-system table but not the requires key is invalid
if "requires" not in build_system:
raise InstallationError(
error_template.format(
package=req_name,
reason=(
"it has a 'build-system' table but not "
"'build-system.requires' which is mandatory in the table"
),
)
)
raise MissingPyProjectBuildRequires(package=req_name)

# Error out if requires is not a list of strings
requires = build_system["requires"]
if not _is_list_of_str(requires):
raise InstallationError(
error_template.format(
package=req_name,
reason="'build-system.requires' is not a list of strings.",
)
raise InvalidPyProjectBuildRequires(
package=req_name,
reason="It is not a list of strings.",
)

# Each requirement must be valid as per PEP 508
for requirement in requires:
try:
Requirement(requirement)
except InvalidRequirement:
raise InstallationError(
error_template.format(
package=req_name,
reason=(
"'build-system.requires' contains an invalid "
"requirement: {!r}".format(requirement)
),
)
)
except InvalidRequirement as error:
raise InvalidPyProjectBuildRequires(
package=req_name,
reason=f"It contains an invalid requirement: {requirement!r}",
) from error

backend = build_system.get("build-backend")
backend_path = build_system.get("backend-path", [])
Expand Down
16 changes: 13 additions & 3 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ def test_pep518_refuses_invalid_requires(script, data, common_wheels):
expect_error=True,
)
assert result.returncode == 1
assert "does not comply with PEP 518" in result.stderr

# Ensure the relevant things are mentioned.
assert "PEP 518" in result.stderr
assert "not a list of strings" in result.stderr
assert "build-system.requires" in result.stderr
assert "pyproject.toml" in result.stderr


def test_pep518_refuses_invalid_build_system(script, data, common_wheels):
Expand All @@ -120,7 +125,12 @@ def test_pep518_refuses_invalid_build_system(script, data, common_wheels):
expect_error=True,
)
assert result.returncode == 1
assert "does not comply with PEP 518" in result.stderr

# Ensure the relevant things are mentioned.
assert "PEP 518" in result.stderr
assert "mandatory `requires` key" in result.stderr
assert "[build-system] table" in result.stderr
assert "pyproject.toml" in result.stderr


def test_pep518_allows_missing_requires(script, data, common_wheels):
Expand All @@ -132,7 +142,7 @@ def test_pep518_allows_missing_requires(script, data, common_wheels):
expect_stderr=True,
)
# Make sure we don't warn when this occurs.
assert "does not comply with PEP 518" not in result.stderr
assert "PEP 518" not in result.stderr

# We want it to go through isolation for now.
assert "Installing build dependencies" in result.stdout, result.stdout
Expand Down
Loading