Skip to content
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.

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

Address issue #4507: don't warn when installing from a commit hash #4674

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/4507.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Don't log a warning when installing a dependency from Git if the name looks
like a commit hash.
18 changes: 15 additions & 3 deletions src/pip/_internal/vcs/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import logging
import os.path
import re

from pip._vendor.packaging.version import parse as parse_version
from pip._vendor.six.moves.urllib import parse as urllib_parse
Expand All @@ -20,6 +21,13 @@
logger = logging.getLogger(__name__)


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


def looks_like_hash(sha):
return bool(HASH_REGEX.match(sha))


class Git(VersionControl):
name = 'git'
dirname = '.git'
Expand Down Expand Up @@ -97,12 +105,16 @@ def check_rev_options(self, dest, rev_options):
elif rev in revisions:
# a local tag or branch name
return rev_options.make_new(revisions[rev])
else:

# Do not show a warning for the common case of something that has
# the form of a Git commit hash.
if not looks_like_hash(rev):
logger.warning(
"Could not find a tag or branch '%s', assuming commit or ref",
"Did not find branch or tag '%s', assuming ref or revision.",
rev,
)
return rev_options

return rev_options

def check_version(self, dest, rev_options):
"""
Expand Down
22 changes: 22 additions & 0 deletions tests/functional/test_install_vcs_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,28 @@ def test_check_rev_options_should_handle_ambiguous_commit(get_refs_mock):
assert new_options.rev == '123456'


@patch('pip._internal.vcs.git.Git.get_short_refs')
def test_check_rev_options_not_found_warning(get_refs_mock, caplog):
get_refs_mock.return_value = {}
git = Git()

sha = 40 * 'a'
rev_options = git.make_rev_options(sha)
new_options = git.check_rev_options('.', rev_options)
assert new_options.rev == sha

rev_options = git.make_rev_options(sha[:6])
new_options = git.check_rev_options('.', rev_options)
assert new_options.rev == 'aaaaaa'

# Check that a warning got logged only for the abbreviated hash.
messages = [r.getMessage() for r in caplog.records]
messages = [msg for msg in messages if msg.startswith('Did not find ')]
assert messages == [
"Did not find branch or tag 'aaaaaa', assuming ref or revision."
]


# TODO(pnasrat) fix all helpers to do right things with paths on windows.
@pytest.mark.skipif("sys.platform == 'win32'")
@pytest.mark.network
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/test_vcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from pip._internal.vcs import RevOptions, VersionControl
from pip._internal.vcs.bazaar import Bazaar
from pip._internal.vcs.git import Git
from pip._internal.vcs.git import Git, looks_like_hash
from pip._internal.vcs.mercurial import Mercurial
from pip._internal.vcs.subversion import Subversion
from tests.lib import pyversion
Expand Down Expand Up @@ -99,6 +99,15 @@ def dist():
return dist


def test_looks_like_hash():
assert looks_like_hash(40 * 'a')
assert looks_like_hash(40 * 'A')
# Test a string containing all valid characters.
assert looks_like_hash(18 * 'a' + '0123456789abcdefABCDEF')
assert not looks_like_hash(40 * 'g')
assert not looks_like_hash(39 * 'a')


def test_git_get_src_requirements(git, dist):
ret = git.get_src_requirement(dist, location='.')

Expand Down