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

…4674)

* Add failing test for issue #4507.

* Add looks_like_hash() with tests.

* Add news file.

* Address issue #4507 by using looks_like_hash().

* Tweak warning text.

* Fix test after rebasing.

* Remove extra line.

news/4507.feature

@@ -0,0 +1,2 @@
+Don't log a warning when installing a dependency from Git if the name looks
+like a commit hash.

src/pip/_internal/vcs/git.py

@@ -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
@@ -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'
@@ -100,12 +108,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):
         """

tests/functional/test_install_vcs_git.py

@@ -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

tests/unit/test_vcs.py

@@ -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
@@ -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='.')