Merge pull request #6356 from cjerdonek/vcs-class-methods-2

Make VersionControl methods into class methods (part 2)

src/pip/_internal/vcs/__init__.py

@@ -364,22 +364,25 @@ def get_url_rev_options(self, url):
 
         return url, rev_options
 
-    def normalize_url(self, url):
+    @staticmethod
+    def normalize_url(url):
         # type: (str) -> str
         """
         Normalize a URL for comparison by unquoting it and removing any
         trailing slash.
         """
         return urllib_parse.unquote(url).rstrip('/')
 
-    def compare_urls(self, url1, url2):
+    @classmethod
+    def compare_urls(cls, url1, url2):
         # type: (str, str) -> bool
         """
         Compare two repo URLs for identity, ignoring incidental differences.
         """
-        return (self.normalize_url(url1) == self.normalize_url(url2))
+        return (cls.normalize_url(url1) == cls.normalize_url(url2))
 
-    def fetch_new(self, dest, url, rev_options):
+    @classmethod
+    def fetch_new(cls, dest, url, rev_options):
         """
         Fetch a revision from a repository, in the case that this is the
         first fetch from the repository.
@@ -408,7 +411,8 @@ def update(self, dest, url, rev_options):
         """
         raise NotImplementedError
 
-    def is_commit_id_equal(self, dest, name):
+    @classmethod
+    def is_commit_id_equal(cls, dest, name):
         """
         Return whether the id of the current commit equals the given name.
 

src/pip/_internal/vcs/bazaar.py

@@ -46,7 +46,8 @@ def export(self, location):
             show_stdout=False,
         )
 
-    def fetch_new(self, dest, url, rev_options):
+    @classmethod
+    def fetch_new(cls, dest, url, rev_options):
         rev_display = rev_options.to_display()
         logger.info(
             'Checking out %s%s to %s',
@@ -55,7 +56,7 @@ def fetch_new(self, dest, url, rev_options):
             display_path(dest),
         )
         cmd_args = ['branch', '-q'] + rev_options.to_args() + [url, dest]
-        self.run_command(cmd_args)
+        cls.run_command(cmd_args)
 
     def switch(self, dest, url, rev_options):
         self.run_command(['switch', url], cwd=dest)
@@ -93,7 +94,8 @@ def get_revision(cls, location):
         )
         return revision.splitlines()[-1]
 
-    def is_commit_id_equal(self, dest, name):
+    @classmethod
+    def is_commit_id_equal(cls, dest, name):
         """Always assume the versions don't match"""
         return False
 

src/pip/_internal/vcs/git.py

@@ -78,7 +78,8 @@ def get_git_version(self):
         version = '.'.join(version.split('.')[:3])
         return parse_version(version)
 
-    def get_current_branch(self, location):
+    @classmethod
+    def get_current_branch(cls, location):
         """
         Return the current branch, or None if HEAD isn't at a branch
         (e.g. detached HEAD).
@@ -88,7 +89,7 @@ def get_current_branch(self, location):
         # command to exit with status code 1 instead of 128 in this case
         # and to suppress the message to stderr.
         args = ['symbolic-ref', '-q', 'HEAD']
-        output = self.run_command(
+        output = cls.run_command(
             args, extra_ok_returncodes=(1, ), show_stdout=False, cwd=location,
         )
         ref = output.strip()
@@ -110,7 +111,8 @@ def export(self, location):
                 show_stdout=False, cwd=temp_dir.path
             )
 
-    def get_revision_sha(self, dest, rev):
+    @classmethod
+    def get_revision_sha(cls, dest, rev):
         """
         Return (sha_or_none, is_branch), where sha_or_none is a commit hash
         if the revision names a remote branch or tag, otherwise None.
@@ -120,8 +122,8 @@ def get_revision_sha(self, dest, rev):
           rev: the revision name.
         """
         # Pass rev to pre-filter the list.
-        output = self.run_command(['show-ref', rev], cwd=dest,
-                                  show_stdout=False, on_returncode='ignore')
+        output = cls.run_command(['show-ref', rev], cwd=dest,
+                                 show_stdout=False, on_returncode='ignore')
         refs = {}
         for line in output.strip().splitlines():
             try:
@@ -144,7 +146,8 @@ def get_revision_sha(self, dest, rev):
 
         return (sha, False)
 
-    def resolve_revision(self, dest, url, rev_options):
+    @classmethod
+    def resolve_revision(cls, dest, url, rev_options):
         """
         Resolve a revision to a new RevOptions object with the SHA1 of the
         branch, tag, or ref if found.
@@ -153,7 +156,7 @@ def resolve_revision(self, dest, url, rev_options):
           rev_options: a RevOptions object.
         """
         rev = rev_options.arg_rev
-        sha, is_branch = self.get_revision_sha(dest, rev)
+        sha, is_branch = cls.get_revision_sha(dest, rev)
 
         if sha is not None:
             rev_options = rev_options.make_new(sha)
@@ -173,17 +176,18 @@ def resolve_revision(self, dest, url, rev_options):
             return rev_options
 
         # If it looks like a ref, we have to fetch it explicitly.
-        self.run_command(
+        cls.run_command(
             ['fetch', '-q', url] + rev_options.to_args(),
             cwd=dest,
         )
         # Change the revision to the SHA of the ref we fetched
-        sha = self.get_revision(dest, rev='FETCH_HEAD')
+        sha = cls.get_revision(dest, rev='FETCH_HEAD')
         rev_options = rev_options.make_new(sha)
 
         return rev_options
 
-    def is_commit_id_equal(self, dest, name):
+    @classmethod
+    def is_commit_id_equal(cls, dest, name):
         """
         Return whether the current commit hash equals the given name.
 
@@ -195,37 +199,38 @@ def is_commit_id_equal(self, dest, name):
             # Then avoid an unnecessary subprocess call.
             return False
 
-        return self.get_revision(dest) == name
+        return cls.get_revision(dest) == name
 
-    def fetch_new(self, dest, url, rev_options):
+    @classmethod
+    def fetch_new(cls, dest, url, rev_options):
         rev_display = rev_options.to_display()
         logger.info(
             'Cloning %s%s to %s', redact_password_from_url(url),
             rev_display, display_path(dest),
         )
-        self.run_command(['clone', '-q', url, dest])
+        cls.run_command(['clone', '-q', url, dest])
 
         if rev_options.rev:
             # Then a specific revision was requested.
-            rev_options = self.resolve_revision(dest, url, rev_options)
+            rev_options = cls.resolve_revision(dest, url, rev_options)
             branch_name = getattr(rev_options, 'branch_name', None)
             if branch_name is None:
                 # Only do a checkout if the current commit id doesn't match
                 # the requested revision.
-                if not self.is_commit_id_equal(dest, rev_options.rev):
+                if not cls.is_commit_id_equal(dest, rev_options.rev):
                     cmd_args = ['checkout', '-q'] + rev_options.to_args()
-                    self.run_command(cmd_args, cwd=dest)
-            elif self.get_current_branch(dest) != branch_name:
+                    cls.run_command(cmd_args, cwd=dest)
+            elif cls.get_current_branch(dest) != branch_name:
                 # Then a specific branch was requested, and that branch
                 # is not yet checked out.
                 track_branch = 'origin/{}'.format(branch_name)
                 cmd_args = [
                     'checkout', '-b', branch_name, '--track', track_branch,
                 ]
-                self.run_command(cmd_args, cwd=dest)
+                cls.run_command(cmd_args, cwd=dest)
 
         #: repo may contain submodules
-        self.update_submodules(dest)
+        cls.update_submodules(dest)
 
     def switch(self, dest, url, rev_options):
         self.run_command(['config', 'remote.origin.url', url], cwd=dest)
@@ -329,10 +334,11 @@ def get_url_rev_and_auth(cls, url):
 
         return url, rev, user_pass
 
-    def update_submodules(self, location):
+    @classmethod
+    def update_submodules(cls, location):
         if not os.path.exists(os.path.join(location, '.gitmodules')):
             return
-        self.run_command(
+        cls.run_command(
             ['submodule', 'update', '--init', '--recursive', '-q'],
             cwd=location,
         )

src/pip/_internal/vcs/mercurial.py

@@ -32,17 +32,18 @@ def export(self, location):
                 ['archive', location], show_stdout=False, cwd=temp_dir.path
             )
 
-    def fetch_new(self, dest, url, rev_options):
+    @classmethod
+    def fetch_new(cls, dest, url, rev_options):
         rev_display = rev_options.to_display()
         logger.info(
             'Cloning hg %s%s to %s',
             url,
             rev_display,
             display_path(dest),
         )
-        self.run_command(['clone', '--noupdate', '-q', url, dest])
+        cls.run_command(['clone', '--noupdate', '-q', url, dest])
         cmd_args = ['update', '-q'] + rev_options.to_args()
-        self.run_command(cmd_args, cwd=dest)
+        cls.run_command(cmd_args, cwd=dest)
 
     def switch(self, dest, url, rev_options):
         repo_config = os.path.join(dest, self.dirname, 'hgrc')
@@ -95,7 +96,8 @@ def get_requirement_revision(cls, location):
             show_stdout=False, cwd=location).strip()
         return current_rev_hash
 
-    def is_commit_id_equal(self, dest, name):
+    @classmethod
+    def is_commit_id_equal(cls, dest, name):
         """Always assume the versions don't match"""
         return False
 

src/pip/_internal/vcs/subversion.py

@@ -46,7 +46,8 @@ def export(self, location):
             cmd_args = ['export'] + rev_options.to_args() + [url, location]
             self.run_command(cmd_args, show_stdout=False)
 
-    def fetch_new(self, dest, url, rev_options):
+    @classmethod
+    def fetch_new(cls, dest, url, rev_options):
         rev_display = rev_options.to_display()
         logger.info(
             'Checking out %s%s to %s',
@@ -55,7 +56,7 @@ def fetch_new(self, dest, url, rev_options):
             display_path(dest),
         )
         cmd_args = ['checkout', '-q'] + rev_options.to_args() + [url, dest]
-        self.run_command(cmd_args)
+        cls.run_command(cmd_args)
 
     def switch(self, dest, url, rev_options):
         cmd_args = ['switch'] + rev_options.to_args() + [url, dest]
@@ -190,7 +191,8 @@ def _get_svn_url_rev(cls, location):
 
         return url, rev
 
-    def is_commit_id_equal(self, dest, name):
+    @classmethod
+    def is_commit_id_equal(cls, dest, name):
         """Always assume the versions don't match"""
         return False
 

tests/functional/test_vcs_git.py

@@ -44,8 +44,7 @@ def add_commits(script, dest, count):
 
 
 def check_rev(repo_dir, rev, expected):
-    git = Git()
-    assert git.get_revision_sha(repo_dir, rev) == expected
+    assert Git.get_revision_sha(repo_dir, rev) == expected
 
 
 def test_git_dir_ignored(tmpdir):
@@ -114,17 +113,16 @@ def test_get_current_branch(script):
     script.run('git', 'init', cwd=repo_dir)
     sha = do_commit(script, repo_dir)
 
-    git = Git()
-    assert git.get_current_branch(repo_dir) == 'master'
+    assert Git.get_current_branch(repo_dir) == 'master'
 
     # Switch to a branch with the same SHA as "master" but whose name
     # is alphabetically after.
     checkout_new_branch(script, repo_dir, 'release')
-    assert git.get_current_branch(repo_dir) == 'release'
+    assert Git.get_current_branch(repo_dir) == 'release'
 
     # Also test the detached HEAD case.
     checkout_ref(script, repo_dir, sha)
-    assert git.get_current_branch(repo_dir) is None
+    assert Git.get_current_branch(repo_dir) is None
 
 
 def test_get_current_branch__branch_and_tag_same_name(script, tmpdir):
@@ -139,12 +137,11 @@ def test_get_current_branch__branch_and_tag_same_name(script, tmpdir):
     # Create a tag with the same name as the branch.
     script.run('git', 'tag', 'dev', cwd=repo_dir)
 
-    git = Git()
-    assert git.get_current_branch(repo_dir) == 'dev'
+    assert Git.get_current_branch(repo_dir) == 'dev'
 
     # Now try with the tag checked out.
     checkout_ref(script, repo_dir, 'refs/tags/dev')
-    assert git.get_current_branch(repo_dir) is None
+    assert Git.get_current_branch(repo_dir) is None
 
 
 def test_get_revision_sha(script):
@@ -212,10 +209,10 @@ def test_is_commit_id_equal(script):
         'git', 'rev-parse', 'HEAD',
         cwd=version_pkg_path
     ).stdout.strip()
-    git = Git()
-    assert git.is_commit_id_equal(version_pkg_path, commit)
-    assert not git.is_commit_id_equal(version_pkg_path, commit[:7])
-    assert not git.is_commit_id_equal(version_pkg_path, 'branch0.1')
-    assert not git.is_commit_id_equal(version_pkg_path, 'abc123')
+
+    assert Git.is_commit_id_equal(version_pkg_path, commit)
+    assert not Git.is_commit_id_equal(version_pkg_path, commit[:7])
+    assert not Git.is_commit_id_equal(version_pkg_path, 'branch0.1')
+    assert not Git.is_commit_id_equal(version_pkg_path, 'abc123')
     # Also check passing a None value.
-    assert not git.is_commit_id_equal(version_pkg_path, None)
+    assert not Git.is_commit_id_equal(version_pkg_path, None)

tests/unit/test_vcs.py

@@ -132,8 +132,7 @@ def test_git_resolve_revision_rev_exists(get_sha_mock):
     url = 'git+https://git.example.com'
     rev_options = Git.make_rev_options('develop')
 
-    git = Git()
-    new_options = git.resolve_revision('.', url, rev_options)
+    new_options = Git.resolve_revision('.', url, rev_options)
     assert new_options.rev == '123456'
 
 
@@ -143,8 +142,7 @@ def test_git_resolve_revision_rev_not_found(get_sha_mock):
     url = 'git+https://git.example.com'
     rev_options = Git.make_rev_options('develop')
 
-    git = Git()
-    new_options = git.resolve_revision('.', url, rev_options)
+    new_options = Git.resolve_revision('.', url, rev_options)
     assert new_options.rev == 'develop'
 
 
@@ -155,12 +153,11 @@ def test_git_resolve_revision_not_found_warning(get_sha_mock, caplog):
     sha = 40 * 'a'
     rev_options = Git.make_rev_options(sha)
 
-    git = Git()
-    new_options = git.resolve_revision('.', url, rev_options)
+    new_options = Git.resolve_revision('.', url, rev_options)
     assert new_options.rev == sha
 
     rev_options = Git.make_rev_options(sha[:6])
-    new_options = git.resolve_revision('.', url, rev_options)
+    new_options = Git.resolve_revision('.', url, rev_options)
     assert new_options.rev == 'aaaaaa'
 
     # Check that a warning got logged only for the abbreviated hash.
@@ -185,7 +182,7 @@ def test_git_is_commit_id_equal(mock_get_revision, rev_name, result):
     Test Git.is_commit_id_equal().
     """
     mock_get_revision.return_value = '5547fa909e83df8bd743d3978d6667497983a4b7'
-    assert Git().is_commit_id_equal('/path', rev_name) is result
+    assert Git.is_commit_id_equal('/path', rev_name) is result
 
 
 # The non-SVN backends all use the same get_netloc_and_auth(), so only test