Skip to content

Handle invalid versions gracefully for all list formats #13345

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Handle invalid versions gracefully for all list formats #13345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions news/13345.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
``pip list`` with the ``json`` or ``freeze`` format enabled will no longer
crash when encountering a package with an invalid version.
18 changes: 12 additions & 6 deletions src/pip/_internal/commands/list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import TYPE_CHECKING, Generator, List, Optional, Sequence, Tuple, cast

from pip._vendor.packaging.utils import canonicalize_name
from pip._vendor.packaging.version import Version
from pip._vendor.packaging.version import InvalidVersion, Version

from pip._internal.cli import cmdoptions
from pip._internal.cli.index_command import IndexGroupCommand
Expand Down Expand Up @@ -285,12 +285,14 @@ def output_package_listing(
self.output_package_listing_columns(data, header)
elif options.list_format == "freeze":
for dist in packages:
try:
req_string = f"{dist.raw_name}=={dist.version}"
except InvalidVersion:
req_string = f"{dist.raw_name}==={dist.raw_version}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pip list --format=freeze does not produce identical output as pip freeze. For instance direct URLs are not supported. So I'm not sure emitting a === will be what user expect here, but to be honest I don't know why this output format for pip list exists in addition to pip freeze.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, are you saying that since pip list --format=freeze has never matched the freeze format that people may not expect anything but name==version? That's fair, although I have doubts on how problematic === would be. A) previously, pip list would crash as it had no way of handling invalid versions, and B) the verbose mode already augments the pip list --format=freeze format.

The alternative is that we simply hide the package with broken metadata or let pip continue crashing, and I don't think that's a particularly great outcome. We could warn that a package is hidden due to broken metadata, but given other parts of pip handle invalid versions gracefully, that would be also unexpected IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you saying that since pip list --format=freeze has never matched the freeze format that people may not expect anything but name==version?

Yes, that, but maybe I'm overthinking it.

if options.verbose >= 1:
write_output(
"%s==%s (%s)", dist.raw_name, dist.version, dist.location
)
write_output("%s (%s)", req_string, dist.location)
else:
write_output("%s==%s", dist.raw_name, dist.version)
write_output(req_string)
elif options.list_format == "json":
write_output(format_for_json(packages, options))

Expand Down Expand Up @@ -374,9 +376,13 @@ def wheel_build_tag(dist: BaseDistribution) -> Optional[str]:
def format_for_json(packages: "_ProcessedDists", options: Values) -> str:
data = []
for dist in packages:
try:
version = str(dist.version)
except InvalidVersion:
version = dist.raw_version
info = {
"name": dist.raw_name,
"version": str(dist.version),
"version": version,
}
if options.verbose >= 1:
info["location"] = dist.location or ""
Expand Down
7 changes: 5 additions & 2 deletions tests/functional/test_invalid_versions_and_specifiers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,15 @@ def test_upgrade_require_invalid_version(
script.pip("install", "--index-url", index_url, "require-invalid-version")


def test_list_invalid_version(script: PipTestEnvironment, data: TestData) -> None:
@pytest.mark.parametrize("format", ["columns", "freeze", "json"])
def test_list_invalid_version(
script: PipTestEnvironment, data: TestData, format: str
) -> None:
"""
Test that pip can list an environment containing a package with a legacy version.
"""
_install_invalid_version(script, data)
script.pip("list")
script.pip("list", f"--format={format}")


def test_freeze_invalid_version(script: PipTestEnvironment, data: TestData) -> None:
Expand Down