Fix pip freeze to use modern format for git repos (#9822)

Pip dropped support for `git+ssh@` style requirements (see #7554)
in favour of `git+ssh://` but didn't propagate the change to
 `pip freeze` which resultantly returns invalid requirements.
Fix this behaviour.

Fixes #9625.
This commit is contained in:
bwoodsend 2021-04-21 10:06:41 +01:00
parent 92862e28ec
commit f533671b0c
4 changed files with 111 additions and 17 deletions

3
news/9822.bugfix.rst Normal file
View File

@ -0,0 +1,3 @@
Fix :ref:`pip freeze` to output packages :ref:`installed from git <vcs support>`
in the correct ``git+protocol://git.example.com/MyProject#egg=MyProject`` format
rather than the old and no longer supported ``git+git@`` format.

View File

@ -1,5 +1,6 @@
import logging
import os.path
import pathlib
import re
import urllib.parse
import urllib.request
@ -322,7 +323,36 @@ class Git(VersionControl):
found_remote = remote
break
url = found_remote.split(' ')[1]
return url.strip()
return cls._git_remote_to_pip_url(url.strip())
@staticmethod
def _git_remote_to_pip_url(url):
# type: (str) -> str
"""
Convert a remote url from what git uses to what pip accepts.
There are 3 legal forms **url** may take:
1. A fully qualified url: ssh://git@example.com/foo/bar.git
2. A local project.git folder: /path/to/bare/repository.git
3. SCP shorthand for form 1: git@example.com:foo/bar.git
Form 1 is output as-is. Form 2 must be converted to URI and form 3 must
be converted to form 1.
See the corresponding test test_git_remote_url_to_pip() for examples of
sample inputs/outputs.
"""
if re.match(r"\w+://", url):
# This is already valid. Pass it though as-is.
return url
if os.path.exists(url):
# A local bare remote (git clone --mirror).
# Needs a file:// prefix.
return pathlib.PurePath(url).as_uri()
# SCP shorthand. e.g. git@example.com:foo/bar.git
# Should add an ssh:// prefix and replace the ':' with a '/'.
return "ssh://" + url.replace(":", "/")
@classmethod
def has_commit(cls, location, rev):
@ -440,5 +470,12 @@ class Git(VersionControl):
return None
return os.path.normpath(r.rstrip('\r\n'))
@staticmethod
def should_add_vcs_url_prefix(repo_url):
# type: (str) -> bool
"""In either https or ssh form, requirements must be prefixed with git+.
"""
return True
vcs.register(Git)

View File

@ -409,7 +409,6 @@ def test_freeze_git_remote(script, tmpdir):
expect_stderr=True,
)
origin_remote = pkg_version
other_remote = pkg_version + '-other'
# check frozen remote after clone
result = script.pip('freeze', expect_stderr=True)
expected = textwrap.dedent(
@ -417,18 +416,29 @@ def test_freeze_git_remote(script, tmpdir):
...-e git+{remote}@...#egg=version_pkg
...
"""
).format(remote=origin_remote).strip()
).format(remote=path_to_url(origin_remote)).strip()
_check_output(result.stdout, expected)
# check frozen remote when there is no remote named origin
script.run('git', 'remote', 'remove', 'origin', cwd=repo_dir)
script.run('git', 'remote', 'add', 'other', other_remote, cwd=repo_dir)
script.run('git', 'remote', 'rename', 'origin', 'other', cwd=repo_dir)
result = script.pip('freeze', expect_stderr=True)
expected = textwrap.dedent(
"""
...-e git+{remote}@...#egg=version_pkg
...
"""
).format(remote=other_remote).strip()
).format(remote=path_to_url(origin_remote)).strip()
_check_output(result.stdout, expected)
# When the remote is a local path, it must exist. Otherwise it is assumed to
# be an ssh:// remote. This is a side effect and not intentional behaviour.
other_remote = pkg_version + '-other'
script.run('git', 'remote', 'set-url', 'other', other_remote, cwd=repo_dir)
result = script.pip('freeze', expect_stderr=True)
expected = textwrap.dedent(
"""
...-e git+ssh://{remote}@...#egg=version_pkg
...
"""
).format(remote=other_remote.replace(":", "/")).strip()
_check_output(result.stdout, expected)
# when there are more than one origin, priority is given to the
# remote named origin
@ -439,7 +449,7 @@ def test_freeze_git_remote(script, tmpdir):
...-e git+{remote}@...#egg=version_pkg
...
"""
).format(remote=origin_remote).strip()
).format(remote=path_to_url(origin_remote)).strip()
_check_output(result.stdout, expected)

View File

@ -1,4 +1,5 @@
import os
import pathlib
from unittest import TestCase
from unittest.mock import patch
@ -108,10 +109,14 @@ def test_looks_like_hash(sha, expected):
@pytest.mark.parametrize('vcs_cls, remote_url, expected', [
# Git is one of the subclasses using the base class implementation.
(Git, 'git://example.com/MyProject', False),
# Mercurial is one of the subclasses using the base class implementation.
# `hg://` isn't a real prefix but it tests the default behaviour.
(Mercurial, 'hg://user@example.com/MyProject', False),
(Mercurial, 'http://example.com/MyProject', True),
# The Git subclasses should return true in all cases.
(Git, 'git://example.com/MyProject', True),
(Git, 'http://example.com/MyProject', True),
# Subversion is the only subclass overriding the base class implementation.
# Subversion also overrides the base class implementation.
(Subversion, 'svn://example.com/MyProject', True),
])
def test_should_add_vcs_url_prefix(vcs_cls, remote_url, expected):
@ -119,26 +124,65 @@ def test_should_add_vcs_url_prefix(vcs_cls, remote_url, expected):
assert actual == expected
@pytest.mark.parametrize("url, target", [
# A fully qualified remote url. No changes needed.
("ssh://bob@server/foo/bar.git", "ssh://bob@server/foo/bar.git"),
("git://bob@server/foo/bar.git", "git://bob@server/foo/bar.git"),
# User is optional and does not need a default.
("ssh://server/foo/bar.git", "ssh://server/foo/bar.git"),
# The common scp shorthand for ssh remotes. Pip won't recognise these as
# git remotes until they have a 'ssh://' prefix and the ':' in the middle
# is gone.
("git@example.com:foo/bar.git", "ssh://git@example.com/foo/bar.git"),
("example.com:foo.git", "ssh://example.com/foo.git"),
# Http(s) remote names are already complete and should remain unchanged.
("https://example.com/foo", "https://example.com/foo"),
("http://example.com/foo/bar.git", "http://example.com/foo/bar.git"),
("https://bob@example.com/foo", "https://bob@example.com/foo"),
])
def test_git_remote_url_to_pip(url, target):
assert Git._git_remote_to_pip_url(url) == target
def test_git_remote_local_path(tmpdir):
path = pathlib.Path(tmpdir, "project.git")
path.mkdir()
# Path must exist to be recognised as a local git remote.
assert Git._git_remote_to_pip_url(str(path)) == path.as_uri()
@patch('pip._internal.vcs.git.Git.get_remote_url')
@patch('pip._internal.vcs.git.Git.get_revision')
@patch('pip._internal.vcs.git.Git.get_subdirectory')
@pytest.mark.parametrize(
"git_url, target_url_prefix",
[
(
"https://github.com/pypa/pip-test-package",
"git+https://github.com/pypa/pip-test-package",
),
(
"git@github.com:pypa/pip-test-package",
"git+ssh://git@github.com/pypa/pip-test-package",
),
],
ids=["https", "ssh"],
)
@pytest.mark.network
def test_git_get_src_requirements(
mock_get_subdirectory, mock_get_revision, mock_get_remote_url
mock_get_subdirectory, mock_get_revision, mock_get_remote_url,
git_url, target_url_prefix,
):
git_url = 'https://github.com/pypa/pip-test-package'
sha = '5547fa909e83df8bd743d3978d6667497983a4b7'
mock_get_remote_url.return_value = git_url
mock_get_remote_url.return_value = Git._git_remote_to_pip_url(git_url)
mock_get_revision.return_value = sha
mock_get_subdirectory.return_value = None
ret = Git.get_src_requirement('.', 'pip-test-package')
assert ret == (
'git+https://github.com/pypa/pip-test-package'
'@5547fa909e83df8bd743d3978d6667497983a4b7#egg=pip_test_package'
)
target = f"{target_url_prefix}@{sha}#egg=pip_test_package"
assert ret == target
@patch('pip._internal.vcs.git.Git.get_revision_sha')