Skip to content

Commit

Permalink
Explicitly recognise SCP-shorthand git remotes.
Browse files Browse the repository at this point in the history
  • Loading branch information
bwoodsend committed Jun 3, 2021
1 parent f533671 commit 8b8fa2b
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 13 deletions.
10 changes: 9 additions & 1 deletion src/pip/_internal/operations/freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def get_requirement_info(dist):

location = os.path.normcase(os.path.abspath(dist.location))

from pip._internal.vcs import RemoteNotFoundError, vcs
from pip._internal.vcs import RemoteNotFoundError, RemoteNotValidError, vcs
vcs_backend = vcs.get_backend_for_dir(location)

if vcs_backend is None:
Expand All @@ -200,6 +200,14 @@ def get_requirement_info(dist):
)
]
return (location, True, comments)
except RemoteNotValidError as ex:
req = dist.as_requirement()
comments = [
f"# Editable {type(vcs_backend).__name__} install ({req}) with "
f"either a deleted local remote or invalid URI:",
f"# '{ex.url}'",
]
return (location, True, comments)

except BadCommand:
logger.warning(
Expand Down
1 change: 1 addition & 0 deletions src/pip/_internal/vcs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import pip._internal.vcs.subversion # noqa: F401
from pip._internal.vcs.versioncontrol import ( # noqa: F401
RemoteNotFoundError,
RemoteNotValidError,
is_url,
make_vcs_requirement_url,
vcs,
Expand Down
22 changes: 19 additions & 3 deletions src/pip/_internal/vcs/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from pip._internal.vcs.versioncontrol import (
AuthInfo,
RemoteNotFoundError,
RemoteNotValidError,
RevOptions,
VersionControl,
find_path_to_setup_from_repo_root,
Expand All @@ -30,6 +31,18 @@

HASH_REGEX = re.compile('^[a-fA-F0-9]{40}$')

# SCP (Secure copy protocol) shorthand. e.g. 'git@example.com:foo/bar.git'
SCP_REGEX = re.compile(r"""^
# Optional user, e.g. 'git@'
(\w+@)?
# Server, e.g. 'github.com'.
([^/:]+):
# The server-side path. e.g. 'user/project.git'. Must start with an
# alphanumeric character so as not to be confusable with a Windows paths
# like 'C:/foo/bar' or 'C:\foo\bar'.
(\w[^:]*)
$""", re.VERBOSE)


def looks_like_hash(sha):
# type: (str) -> bool
Expand Down Expand Up @@ -350,9 +363,12 @@ def _git_remote_to_pip_url(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(":", "/")
scp_match = SCP_REGEX.match(url)
if scp_match:
# Add an ssh:// prefix and replace the ':' with a '/'.
return scp_match.expand(r"ssh://\1\2/\3")
# Otherwise, bail out.
raise RemoteNotValidError(url)

@classmethod
def has_commit(cls, location, rev):
Expand Down
6 changes: 6 additions & 0 deletions src/pip/_internal/vcs/versioncontrol.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ class RemoteNotFoundError(Exception):
pass


class RemoteNotValidError(Exception):
def __init__(self, url: str):
super().__init__(url)
self.url = url


class RevOptions:

"""
Expand Down
17 changes: 9 additions & 8 deletions tests/functional/test_freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,18 +428,19 @@ def test_freeze_git_remote(script, tmpdir):
"""
).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.
# When the remote is a local path, it must exist.
# If it doesn't, it gets flagged as invalid.
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
...
expected = os.path.normcase(textwrap.dedent(
f"""
...# Editable Git...(version-pkg...)...
# '{other_remote}'
-e {repo_dir}...
"""
).format(remote=other_remote.replace(":", "/")).strip()
_check_output(result.stdout, expected)
).strip())
_check_output(os.path.normcase(result.stdout), expected)
# when there are more than one origin, priority is given to the
# remote named origin
script.run('git', 'remote', 'add', 'origin', origin_remote, cwd=repo_dir)
Expand Down
20 changes: 19 additions & 1 deletion tests/unit/test_vcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pip._internal.utils.misc import hide_url, hide_value
from pip._internal.vcs import make_vcs_requirement_url
from pip._internal.vcs.bazaar import Bazaar
from pip._internal.vcs.git import Git, looks_like_hash
from pip._internal.vcs.git import Git, RemoteNotValidError, looks_like_hash
from pip._internal.vcs.mercurial import Mercurial
from pip._internal.vcs.subversion import Subversion
from pip._internal.vcs.versioncontrol import RevOptions, VersionControl
Expand Down Expand Up @@ -144,6 +144,24 @@ def test_git_remote_url_to_pip(url, target):
assert Git._git_remote_to_pip_url(url) == target


@pytest.mark.parametrize("url, platform", [
# Windows paths with the ':' drive prefix look dangerously close to SCP.
("c:/piffle/wiffle/waffle/poffle.git", "nt"),
(r"c:\faffle\waffle\woffle\piffle.git", "nt"),
# Unix paths less so but test them anyway.
("/muffle/fuffle/pufffle/fluffle.git", "posix"),
])
def test_paths_are_not_mistaken_for_scp_shorthand(url, platform):
# File paths should not be mistaken for SCP shorthand. If they do then
# 'c:/piffle/wiffle' would end up as 'ssh://c/piffle/wiffle'.
from pip._internal.vcs.git import SCP_REGEX
assert not SCP_REGEX.match(url)

if platform == os.name:
with pytest.raises(RemoteNotValidError):
Git._git_remote_to_pip_url(url)


def test_git_remote_local_path(tmpdir):
path = pathlib.Path(tmpdir, "project.git")
path.mkdir()
Expand Down

0 comments on commit 8b8fa2b

Please sign in to comment.