Choose a better name, and other clean-ups.

src/pip/_internal/vcs/__init__.py

@@ -279,13 +279,13 @@ class VersionControl(object):
         """
         raise NotImplementedError
 
-    def check_version(self, dest, commit_id):
+    def does_commit_id_equal(self, dest, name):
         """
-        Return True if the version is identical to what exists and
-        doesn't need to be updated.
+        Return whether the id of the current commit equals the given name.
 
         Args:
-          rev_options: a RevOptions object.
+          dest: the repository directory.
+          name: a string name.
         """
         raise NotImplementedError
 
@@ -313,8 +313,7 @@ class VersionControl(object):
                         display_path(dest),
                         url,
                     )
-                    commit_id = rev_options[0]
-                    if not self.check_version(dest, commit_id):
+                    if not self.does_commit_id_equal(dest, rev_options.rev):
                         logger.info(
                             'Updating %s %s%s',
                             display_path(dest),
@@ -406,8 +405,7 @@ class VersionControl(object):
 
     def get_revision(self, location):
         """
-        Return the current revision of the files at location
-        Used in get_info
+        Return the current commit id of the files at the given location.
         """
         raise NotImplementedError
 

src/pip/_internal/vcs/bazaar.py

@@ -104,7 +104,7 @@ class Bazaar(VersionControl):
         current_rev = self.get_revision(location)
         return '%s@%s#egg=%s' % (repo, current_rev, egg_project_name)
 
-    def check_version(self, dest, commit_id):
+    def does_commit_id_equal(self, dest, name):
         """Always assume the versions don't match"""
         return False
 

src/pip/_internal/vcs/git.py

@@ -119,17 +119,19 @@ class Git(VersionControl):
 
         return rev_options
 
-    def check_version(self, dest, commit_id):
+    def does_commit_id_equal(self, dest, name):
         """
-        Compare the current sha to the ref. ref may be a branch or tag name,
-        but current rev will always point to a sha. This means that a branch
-        or tag will never compare as True. So this ultimately only matches
-        against exact shas.
+        Return whether the current commit hash equals the given name.
 
         Args:
-          rev_options: a RevOptions object.
+          dest: the repository directory.
+          name: a string name.
         """
-        return self.get_revision(dest) == commit_id
+        if not name:
+            # Then avoid an unnecessary subprocess call.
+            return False
+
+        return self.get_revision(dest) == name
 
     def switch(self, dest, url, rev_options):
         self.run_command(['config', 'remote.origin.url', url], cwd=dest)
@@ -164,9 +166,9 @@ class Git(VersionControl):
 
             if rev:
                 rev_options = self.check_rev_options(dest, rev_options)
-                commit_hash = rev_options.rev
-                # Only do a checkout if HEAD differs from commit_hash.
-                if not self.check_version(dest, commit_hash):
+                # Only do a checkout if the current commit id doesn't match
+                # the requested revision.
+                if not self.does_commit_id_equal(dest, rev_options.rev):
                     cmd_args = ['fetch', '-q', url] + rev_options.to_args()
                     self.run_command(cmd_args, cwd=dest)
                     self.run_command(

src/pip/_internal/vcs/mercurial.py

@@ -97,7 +97,7 @@ class Mercurial(VersionControl):
         current_rev_hash = self.get_revision_hash(location)
         return '%s@%s#egg=%s' % (repo, current_rev_hash, egg_project_name)
 
-    def check_version(self, dest, commit_id):
+    def does_commit_id_equal(self, dest, name):
         """Always assume the versions don't match"""
         return False
 

src/pip/_internal/vcs/subversion.py

@@ -217,7 +217,7 @@ class Subversion(VersionControl):
         rev = self.get_revision(location)
         return 'svn+%s@%s#egg=%s' % (repo, rev, egg_project_name)
 
-    def check_version(self, dest, commit_id):
+    def does_commit_id_equal(self, dest, name):
         """Always assume the versions don't match"""
         return False
 

tests/functional/test_install_vcs_git.py

@@ -58,13 +58,11 @@ def test_get_short_refs_should_ignore_no_branch(script):
     assert result['branch0.1'] == commit, result
 
 
-def call_check_version(vcs, path, rev):
-    rev_options = vcs.make_rev_options(rev)
-    return vcs.check_version(path, rev_options)
-
-
 @pytest.mark.network
-def test_check_version(script):
+def test_does_commit_id_equal(script):
+    """
+    Test Git.does_commit_id_equal().
+    """
     version_pkg_path = _create_test_package(script)
     script.run('git', 'branch', 'branch0.1', cwd=version_pkg_path)
     commit = script.run(
@@ -72,10 +70,12 @@ def test_check_version(script):
         cwd=version_pkg_path
     ).stdout.strip()
     git = Git()
-    assert call_check_version(git, version_pkg_path, commit)
-    assert call_check_version(git, version_pkg_path, commit[:7])
-    assert not call_check_version(git, version_pkg_path, 'branch0.1')
-    assert not call_check_version(git, version_pkg_path, 'abc123')
+    assert git.does_commit_id_equal(version_pkg_path, commit)
+    assert not git.does_commit_id_equal(version_pkg_path, commit[:7])
+    assert not git.does_commit_id_equal(version_pkg_path, 'branch0.1')
+    assert not git.does_commit_id_equal(version_pkg_path, 'abc123')
+    # Also check passing a None value.
+    assert not git.does_commit_id_equal(version_pkg_path, None)
 
 
 @patch('pip._internal.vcs.git.Git.get_short_refs')

tests/unit/test_vcs.py

@@ -118,15 +118,19 @@ def test_git_get_src_requirements(git, dist):
     ])
 
 
-@pytest.mark.parametrize('commit,result', (
+@pytest.mark.parametrize('rev_name,result', (
     ('5547fa909e83df8bd743d3978d6667497983a4b7', True),
     ('5547fa909', False),
     ('5678', False),
     ('abc123', False),
     ('foo', False),
+    (None, False),
 ))
-def test_git_check_version(git, commit, result):
-    assert git.check_version('/path', commit) is result
+def test_git_does_commit_id_equal(git, rev_name, result):
+    """
+    Test Git.does_commit_id_equal().
+    """
+    assert git.does_commit_id_equal('/path', rev_name) is result
 
 
 def test_translate_egg_surname():