From 0b761a164c6429db4a6b0e0e173c3f7dba2546a0 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Fri, 25 Dec 2020 11:24:38 -0800 Subject: [PATCH] Harmonize type signature of VersionControl.get_remote_url() subclasses In the base class, the signature is defined as: type: (str) -> str Further, the docstring says: Raises RemoteNotFoundError if the repository does not have a remote url configured. However, some subclasses were returning None instead of raising RemoteNotFoundError. This violated the type signature and forced calling code to handle multiple error paradigms. Now, all subclasses implement the base's signature. This allowed simplifying some call sites as they can assume None will not be returned. This mismatch was noticed while trying to remove "mypy: disallow-untyped-defs=False" comments. --- ...71b0-98f9-4e1f-a541-af95fb990af9.trivial.rst | 0 src/pip/_internal/operations/freeze.py | 3 +-- src/pip/_internal/vcs/bazaar.py | 5 +++-- src/pip/_internal/vcs/git.py | 1 + src/pip/_internal/vcs/mercurial.py | 1 + src/pip/_internal/vcs/subversion.py | 5 +++-- src/pip/_internal/vcs/versioncontrol.py | 4 +--- tests/functional/test_vcs_bazaar.py | 13 +++++++++++++ tests/functional/test_vcs_subversion.py | 17 +++++++++++++++++ 9 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 news/738a71b0-98f9-4e1f-a541-af95fb990af9.trivial.rst create mode 100644 tests/functional/test_vcs_subversion.py diff --git a/news/738a71b0-98f9-4e1f-a541-af95fb990af9.trivial.rst b/news/738a71b0-98f9-4e1f-a541-af95fb990af9.trivial.rst new file mode 100644 index 000000000..e69de29bb diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 3529c55ed..0c26f9c7e 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -225,8 +225,7 @@ def get_requirement_info(dist): "falling back to uneditable format", exc ) else: - if req is not None: - return (req, True, []) + return (req, True, []) logger.warning( 'Could not determine repository location of %s', location diff --git a/src/pip/_internal/vcs/bazaar.py b/src/pip/_internal/vcs/bazaar.py index 22969c726..d55399b7a 100644 --- a/src/pip/_internal/vcs/bazaar.py +++ b/src/pip/_internal/vcs/bazaar.py @@ -9,7 +9,7 @@ from pip._internal.utils.misc import display_path, rmtree from pip._internal.utils.subprocess import make_command from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.utils.urls import path_to_url -from pip._internal.vcs.versioncontrol import VersionControl, vcs +from pip._internal.vcs.versioncontrol import RemoteNotFoundError, VersionControl, vcs if MYPY_CHECK_RUNNING: from typing import Optional, Tuple @@ -89,6 +89,7 @@ class Bazaar(VersionControl): @classmethod def get_remote_url(cls, location): + # type: (str) -> str urls = cls.run_command(['info'], cwd=location) for line in urls.splitlines(): line = line.strip() @@ -99,7 +100,7 @@ class Bazaar(VersionControl): if cls._is_local_repository(repo): return path_to_url(repo) return repo - return None + raise RemoteNotFoundError @classmethod def get_revision(cls, location): diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 46f15fc8b..688f132a4 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -303,6 +303,7 @@ class Git(VersionControl): @classmethod def get_remote_url(cls, location): + # type: (str) -> str """ Return URL of the first remote encountered. diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py index 1c8426674..e7988d1ac 100644 --- a/src/pip/_internal/vcs/mercurial.py +++ b/src/pip/_internal/vcs/mercurial.py @@ -87,6 +87,7 @@ class Mercurial(VersionControl): @classmethod def get_remote_url(cls, location): + # type: (str) -> str url = cls.run_command( ['showconfig', 'paths.default'], cwd=location).strip() diff --git a/src/pip/_internal/vcs/subversion.py b/src/pip/_internal/vcs/subversion.py index 3bb7ea0f8..85ce2aa91 100644 --- a/src/pip/_internal/vcs/subversion.py +++ b/src/pip/_internal/vcs/subversion.py @@ -14,7 +14,7 @@ from pip._internal.utils.misc import ( ) from pip._internal.utils.subprocess import make_command from pip._internal.utils.typing import MYPY_CHECK_RUNNING -from pip._internal.vcs.versioncontrol import VersionControl, vcs +from pip._internal.vcs.versioncontrol import RemoteNotFoundError, VersionControl, vcs _svn_xml_url_re = re.compile('url="([^"]+)"') _svn_rev_re = re.compile(r'committed-rev="(\d+)"') @@ -110,6 +110,7 @@ class Subversion(VersionControl): @classmethod def get_remote_url(cls, location): + # type: (str) -> str # In cases where the source is in a subdirectory, not alongside # setup.py we have to look up in the location until we find a real # setup.py @@ -125,7 +126,7 @@ class Subversion(VersionControl): "parent directories)", orig_location, ) - return None + raise RemoteNotFoundError return cls._get_svn_url_rev(location)[0] diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index 35caf1915..b78cbabf3 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/src/pip/_internal/vcs/versioncontrol.py @@ -410,7 +410,7 @@ class VersionControl(object): @classmethod def get_src_requirement(cls, repo_dir, project_name): - # type: (str, str) -> Optional[str] + # type: (str, str) -> str """ Return the requirement string to use to redownload the files currently at the given repository directory. @@ -423,8 +423,6 @@ class VersionControl(object): {repository_url}@{revision}#egg={project_name} """ repo_url = cls.get_remote_url(repo_dir) - if repo_url is None: - return None if cls.should_add_vcs_url_prefix(repo_url): repo_url = '{}+{}'.format(cls.name, repo_url) diff --git a/tests/functional/test_vcs_bazaar.py b/tests/functional/test_vcs_bazaar.py index d928da8b3..ad24d73d5 100644 --- a/tests/functional/test_vcs_bazaar.py +++ b/tests/functional/test_vcs_bazaar.py @@ -8,6 +8,7 @@ import pytest from pip._internal.utils.misc import hide_url from pip._internal.vcs.bazaar import Bazaar +from pip._internal.vcs.versioncontrol import RemoteNotFoundError from tests.lib import ( _test_path_to_file_url, _vcs_add, @@ -65,3 +66,15 @@ def test_export_rev(script, tmpdir): with open(export_dir / 'test_file', 'r') as f: assert f.read() == 'something initial' + + +@need_bzr +def test_get_remote_url__no_remote(script, tmpdir): + repo_dir = tmpdir / 'temp-repo' + repo_dir.mkdir() + repo_dir = str(repo_dir) + + script.run('bzr', 'init', repo_dir) + + with pytest.raises(RemoteNotFoundError): + Bazaar().get_remote_url(repo_dir) diff --git a/tests/functional/test_vcs_subversion.py b/tests/functional/test_vcs_subversion.py new file mode 100644 index 000000000..c71c793f8 --- /dev/null +++ b/tests/functional/test_vcs_subversion.py @@ -0,0 +1,17 @@ +import pytest + +from pip._internal.vcs.subversion import Subversion +from pip._internal.vcs.versioncontrol import RemoteNotFoundError +from tests.lib import _create_svn_repo, need_svn + + +@need_svn +def test_get_remote_url__no_remote(script, tmpdir): + repo_dir = tmpdir / 'temp-repo' + repo_dir.mkdir() + repo_dir = str(repo_dir) + + _create_svn_repo(script, repo_dir) + + with pytest.raises(RemoteNotFoundError): + Subversion().get_remote_url(repo_dir)