From a3b1d5657892885fc18b989584c57106748126b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 17:21:01 +0200 Subject: [PATCH 1/6] Make _get_editable_info always return editable=True for editables At the beginning this function returns if not dist.editable. So returning editable=False after that does not make sense. Return an editable with the file system location instead when there is any issue getting VCS information. --- src/pip/_internal/operations/freeze.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index be484f47db8..25395c525e5 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -212,7 +212,6 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: f"# '{ex.url}'", ], ) - except BadCommand: logger.warning( "cannot determine version of editable source in %s " @@ -220,22 +219,17 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: location, vcs_backend.name, ) - return _EditableInfo(requirement=None, editable=True, comments=[]) - + return _EditableInfo(requirement=location, editable=True, comments=[]) except InstallationError as exc: - logger.warning( - "Error when trying to get requirement for VCS system %s, " - "falling back to uneditable format", - exc, - ) + logger.warning("Error when trying to get requirement for VCS system %s", exc) else: return _EditableInfo(requirement=req, editable=True, comments=[]) logger.warning("Could not determine repository location of %s", location) return _EditableInfo( - requirement=None, - editable=False, + requirement=location, + editable=True, comments=["## !! Could not determine repository location"], ) From ddcf558a54e6c2ac4dea49af549529a1d5715e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 17:29:58 +0200 Subject: [PATCH 2/6] _dist_is_editable always returns a location --- src/pip/_internal/operations/freeze.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 25395c525e5..74f5f29bbf8 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -30,7 +30,7 @@ class _EditableInfo(NamedTuple): - requirement: Optional[str] + requirement: str editable: bool comments: List[str] @@ -167,8 +167,6 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: Compute and return values (req, editable, comments) for use in FrozenRequirement.from_dist(). """ - if not dist.editable: - return _EditableInfo(requirement=None, editable=False, comments=[]) editable_project_location = dist.editable_project_location assert editable_project_location location = os.path.normcase(os.path.abspath(editable_project_location)) @@ -253,16 +251,17 @@ def from_dist(cls, dist: BaseDistribution) -> "FrozenRequirement": # TODO `get_requirement_info` is taking care of editable requirements. # TODO This should be refactored when we will add detection of # editable that provide .dist-info metadata. - req, editable, comments = _get_editable_info(dist) - if req is None and not editable: - # if PEP 610 metadata is present, attempt to use it + if dist.editable: + req, editable, comments = _get_editable_info(dist) + else: + comments = [] direct_url = dist.direct_url if direct_url: + # if PEP 610 metadata is present, use it req = direct_url_as_pep440_direct_reference(direct_url, dist.raw_name) - comments = [] - if req is None: - # name==version requirement - req = _format_as_name_version(dist) + else: + # name==version requirement + req = _format_as_name_version(dist) return cls(dist.raw_name, req, editable, comments=comments) From 283cabf046c985a1e6030cb7081a25335d480abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 17:32:03 +0200 Subject: [PATCH 3/6] _EditableInfo does not need an editable flag It is always true. --- src/pip/_internal/operations/freeze.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 74f5f29bbf8..3449116e3d4 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -31,7 +31,6 @@ class _EditableInfo(NamedTuple): requirement: str - editable: bool comments: List[str] @@ -164,7 +163,7 @@ def _format_as_name_version(dist: BaseDistribution) -> str: def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: """ - Compute and return values (req, editable, comments) for use in + Compute and return values (req, comments) for use in FrozenRequirement.from_dist(). """ editable_project_location = dist.editable_project_location @@ -184,7 +183,6 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: ) return _EditableInfo( requirement=location, - editable=True, comments=[f"# Editable install with no version control ({display})"], ) @@ -196,14 +194,12 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: display = _format_as_name_version(dist) return _EditableInfo( requirement=location, - editable=True, comments=[f"# Editable {vcs_name} install with no remote ({display})"], ) except RemoteNotValidError as ex: display = _format_as_name_version(dist) return _EditableInfo( requirement=location, - editable=True, comments=[ f"# Editable {vcs_name} install ({display}) with either a deleted " f"local remote or invalid URI:", @@ -217,17 +213,16 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo: location, vcs_backend.name, ) - return _EditableInfo(requirement=location, editable=True, comments=[]) + return _EditableInfo(requirement=location, comments=[]) except InstallationError as exc: logger.warning("Error when trying to get requirement for VCS system %s", exc) else: - return _EditableInfo(requirement=req, editable=True, comments=[]) + return _EditableInfo(requirement=req, comments=[]) logger.warning("Could not determine repository location of %s", location) return _EditableInfo( requirement=location, - editable=True, comments=["## !! Could not determine repository location"], ) @@ -251,8 +246,9 @@ def from_dist(cls, dist: BaseDistribution) -> "FrozenRequirement": # TODO `get_requirement_info` is taking care of editable requirements. # TODO This should be refactored when we will add detection of # editable that provide .dist-info metadata. - if dist.editable: - req, editable, comments = _get_editable_info(dist) + editable = dist.editable + if editable: + req, comments = _get_editable_info(dist) else: comments = [] direct_url = dist.direct_url From 9c31cadf33ef6ef32f6cb4895e00fa11b1040358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 17:33:26 +0200 Subject: [PATCH 4/6] Remove unused type --- src/pip/_internal/operations/freeze.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 3449116e3d4..40b058306d5 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -1,19 +1,8 @@ import collections import logging import os -from typing import ( - Container, - Dict, - Iterable, - Iterator, - List, - NamedTuple, - Optional, - Set, - Union, -) +from typing import Container, Dict, Iterable, Iterator, List, NamedTuple, Optional, Set -from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.utils import canonicalize_name from pip._vendor.packaging.version import Version @@ -231,7 +220,7 @@ class FrozenRequirement: def __init__( self, name: str, - req: Union[str, Requirement], + req: str, editable: bool, comments: Iterable[str] = (), ) -> None: From 36befd83117fd30d9a98668e729af1fdec189185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 17:38:31 +0200 Subject: [PATCH 5/6] Remove resolved TODO comment --- src/pip/_internal/operations/freeze.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 40b058306d5..456554085df 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -232,9 +232,6 @@ def __init__( @classmethod def from_dist(cls, dist: BaseDistribution) -> "FrozenRequirement": - # TODO `get_requirement_info` is taking care of editable requirements. - # TODO This should be refactored when we will add detection of - # editable that provide .dist-info metadata. editable = dist.editable if editable: req, comments = _get_editable_info(dist) From 6ac3ffe17e619b2b4d6a54bac2d6e7d854032428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 28 Aug 2021 18:09:23 +0200 Subject: [PATCH 6/6] Add news --- news/10410.feature.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 news/10410.feature.rst diff --git a/news/10410.feature.rst b/news/10410.feature.rst new file mode 100644 index 00000000000..e3bfdc83b93 --- /dev/null +++ b/news/10410.feature.rst @@ -0,0 +1,3 @@ +``pip freeze`` will now always fallback to reporting the editable project +location when it encounters a VCS error while analyzing an editable +requirement. Before, it sometimes reported the requirement as non-editable.