Add a RevOptions class (#4707)

* Add the RevOptions class, and test.

* Start using the RevOptions class.

* Add news file.

* Update for mypy.

* Fix test after rebasing.

* Address @xavfernandez's review comments.

src/pip/_internal/vcs/__init__.py

@@ -1,6 +1,7 @@
 """Handles all VCS (version control) support"""
 from __future__ import absolute_import
 
+import copy
 import errno
 import logging
 import os
@@ -16,7 +17,7 @@
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:
-    from typing import Dict, Tuple
+    from typing import Dict, Optional, Tuple
     from pip._internal.basecommand import Command
 
 __all__ = ['vcs', 'get_src_requirement']
@@ -25,6 +26,67 @@
 logger = logging.getLogger(__name__)
 
 
+class RevOptions(object):
+
+    """
+    Encapsulates a VCS-specific revision to install, along with any VCS
+    install options.
+
+    Instances of this class should be treated as if immutable.
+    """
+
+    def __init__(self, vcs, rev=None, extra_args=None):
+        """
+        Args:
+          vcs: a VersionControl object.
+          rev: the name of the revision to install.
+          extra_args: a list of extra options.
+        """
+        if extra_args is None:
+            extra_args = []
+
+        self.extra_args = extra_args
+        self.rev = rev
+        self.vcs = vcs
+
+    def __repr__(self):
+        return '<RevOptions {}: rev={!r}>'.format(self.vcs.name, self.rev)
+
+    @property
+    def arg_rev(self):
+        if self.rev is None:
+            return self.vcs.default_arg_rev
+
+        return self.rev
+
+    def to_args(self):
+        """
+        Return the VCS-specific command arguments.
+        """
+        args = []
+        rev = self.arg_rev
+        if rev is not None:
+            args += self.vcs.get_base_rev_args(rev)
+        args += self.extra_args
+
+        return args
+
+    def to_display(self):
+        if not self.rev:
+            return ''
+
+        return ' (to revision {})'.format(self.rev)
+
+    def make_new(self, rev):
+        """
+        Make a copy of the current instance, but with a new rev.
+
+        Args:
+          rev: the name of the revision for the new object.
+        """
+        return self.vcs.make_rev_options(rev, extra_args=self.extra_args)
+
+
 class VcsSupport(object):
     _registry = {}  # type: Dict[str, Command]
     schemes = ['ssh', 'git', 'hg', 'bzr', 'sftp', 'svn']
@@ -104,11 +166,31 @@ class VersionControl(object):
     dirname = ''
     # List of supported schemes for this Version Control
     schemes = ()  # type: Tuple[str, ...]
+    default_arg_rev = None  # type: Optional[str]
 
     def __init__(self, url=None, *args, **kwargs):
         self.url = url
         super(VersionControl, self).__init__(*args, **kwargs)
 
+    def get_base_rev_args(self, rev):
+        """
+        Return the base revision arguments for a vcs command.
+
+        Args:
+          rev: the name of a revision to install.  Cannot be None.
+        """
+        raise NotImplementedError
+
+    def make_rev_options(self, rev=None, extra_args=None):
+        """
+        Return a RevOptions object.
+
+        Args:
+          rev: the name of a revision to install.
+          extra_args: a list of extra options.
+        """
+        return RevOptions(self, rev, extra_args=extra_args)
+
     def _is_local_repository(self, repo):
         """
            posix absolute paths start with os.path.sep,
@@ -180,31 +262,44 @@ def obtain(self, dest):
     def switch(self, dest, url, rev_options):
         """
         Switch the repo at ``dest`` to point to ``URL``.
+
+        Args:
+          rev_options: a RevOptions object.
         """
         raise NotImplementedError
 
     def update(self, dest, rev_options):
         """
         Update an already-existing repo to the given ``rev_options``.
+
+        Args:
+          rev_options: a RevOptions object.
         """
         raise NotImplementedError
 
     def check_version(self, dest, rev_options):
         """
         Return True if the version is identical to what exists and
         doesn't need to be updated.
+
+        Args:
+          rev_options: a RevOptions object.
         """
         raise NotImplementedError
 
-    def check_destination(self, dest, url, rev_options, rev_display):
+    def check_destination(self, dest, url, rev_options):
         """
         Prepare a location to receive a checkout/clone.
 
         Return True if the location is ready for (and requires) a
         checkout/clone, False otherwise.
+
+        Args:
+          rev_options: a RevOptions object.
         """
         checkout = True
         prompt = False
+        rev_display = rev_options.to_display()
         if os.path.exists(dest):
             checkout = False
             if os.path.exists(os.path.join(dest, self.dirname)):

src/pip/_internal/vcs/bazaar.py

@@ -29,6 +29,9 @@ def __init__(self, url=None, *args, **kwargs):
         if getattr(urllib_parse, 'uses_fragment', None):
             urllib_parse.uses_fragment.extend(['lp'])
 
+    def get_base_rev_args(self, rev):
+        return ['-r', rev]
+
     def export(self, location):
         """
         Export the Bazaar repository at the url to the destination location
@@ -49,24 +52,22 @@ def switch(self, dest, url, rev_options):
         self.run_command(['switch', url], cwd=dest)
 
     def update(self, dest, rev_options):
-        self.run_command(['pull', '-q'] + rev_options, cwd=dest)
+        cmd_args = ['pull', '-q'] + rev_options.to_args()
+        self.run_command(cmd_args, cwd=dest)
 
     def obtain(self, dest):
         url, rev = self.get_url_rev()
-        if rev:
-            rev_options = ['-r', rev]
-            rev_display = ' (to revision %s)' % rev
-        else:
-            rev_options = []
-            rev_display = ''
-        if self.check_destination(dest, url, rev_options, rev_display):
+        rev_options = self.make_rev_options(rev)
+        if self.check_destination(dest, url, rev_options):
+            rev_display = rev_options.to_display()
             logger.info(
                 'Checking out %s%s to %s',
                 url,
                 rev_display,
                 display_path(dest),
             )
-            self.run_command(['branch', '-q'] + rev_options + [url, dest])
+            cmd_args = ['branch', '-q'] + rev_options.to_args() + [url, dest]
+            self.run_command(cmd_args)
 
     def get_url_rev(self):
         # hotfix the URL scheme after removing bzr+ from bzr+ssh:// readd it

src/pip/_internal/vcs/git.py

@@ -27,6 +27,7 @@ class Git(VersionControl):
     schemes = (
         'git', 'git+http', 'git+https', 'git+ssh', 'git+git', 'git+file',
     )
+    default_arg_rev = 'origin/HEAD'
 
     def __init__(self, url=None, *args, **kwargs):
 
@@ -49,6 +50,9 @@ def __init__(self, url=None, *args, **kwargs):
 
         super(Git, self).__init__(url, *args, **kwargs)
 
+    def get_base_rev_args(self, rev):
+        return [rev]
+
     def get_git_version(self):
         VERSION_PFX = 'git version '
         version = self.run_command(['version'], show_stdout=False)
@@ -74,20 +78,25 @@ def export(self, location):
                 show_stdout=False, cwd=temp_dir.path
             )
 
-    def check_rev_options(self, rev, dest, rev_options):
+    def check_rev_options(self, dest, rev_options):
         """Check the revision options before checkout to compensate that tags
         and branches may need origin/ as a prefix.
-        Returns the SHA1 of the branch or tag if found.
+        Returns a new RevOptions object for the SHA1 of the branch or tag
+        if found.
+
+        Args:
+          rev_options: a RevOptions object.
         """
         revisions = self.get_short_refs(dest)
 
+        rev = rev_options.arg_rev
         origin_rev = 'origin/%s' % rev
         if origin_rev in revisions:
             # remote branch
-            return [revisions[origin_rev]]
+            return rev_options.make_new(revisions[origin_rev])
         elif rev in revisions:
             # a local tag or branch name
-            return [revisions[rev]]
+            return rev_options.make_new(revisions[rev])
         else:
             logger.warning(
                 "Could not find a tag or branch '%s', assuming commit or ref",
@@ -101,12 +110,16 @@ def check_version(self, dest, rev_options):
         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.
+
+        Args:
+          rev_options: a RevOptions object.
         """
-        return self.get_revision(dest).startswith(rev_options[0])
+        return self.get_revision(dest).startswith(rev_options.arg_rev)
 
     def switch(self, dest, url, rev_options):
         self.run_command(['config', 'remote.origin.url', url], cwd=dest)
-        self.run_command(['checkout', '-q'] + rev_options, cwd=dest)
+        cmd_args = ['checkout', '-q'] + rev_options.to_args()
+        self.run_command(cmd_args, cwd=dest)
 
         self.update_submodules(dest)
 
@@ -118,36 +131,28 @@ def update(self, dest, rev_options):
         else:
             self.run_command(['fetch', '-q'], cwd=dest)
         # Then reset to wanted revision (maybe even origin/master)
-        if rev_options:
-            rev_options = self.check_rev_options(
-                rev_options[0], dest, rev_options,
-            )
-        self.run_command(['reset', '--hard', '-q'] + rev_options, cwd=dest)
+        rev_options = self.check_rev_options(dest, rev_options)
+        cmd_args = ['reset', '--hard', '-q'] + rev_options.to_args()
+        self.run_command(cmd_args, cwd=dest)
         #: update submodules
         self.update_submodules(dest)
 
     def obtain(self, dest):
         url, rev = self.get_url_rev()
-        if rev:
-            rev_options = [rev]
-            rev_display = ' (to %s)' % rev
-        else:
-            rev_options = ['origin/HEAD']
-            rev_display = ''
-        if self.check_destination(dest, url, rev_options, rev_display):
+        rev_options = self.make_rev_options(rev)
+        if self.check_destination(dest, url, rev_options):
+            rev_display = rev_options.to_display()
             logger.info(
                 'Cloning %s%s to %s', url, rev_display, display_path(dest),
             )
             self.run_command(['clone', '-q', url, dest])
 
             if rev:
-                rev_options = self.check_rev_options(rev, dest, rev_options)
+                rev_options = self.check_rev_options(dest, rev_options)
                 # Only do a checkout if rev_options differs from HEAD
                 if not self.check_version(dest, rev_options):
-                    self.run_command(
-                        ['fetch', '-q', url] + rev_options,
-                        cwd=dest,
-                    )
+                    cmd_args = ['fetch', '-q', url] + rev_options.to_args()
+                    self.run_command(cmd_args, cwd=dest,)
                     self.run_command(
                         ['checkout', '-q', 'FETCH_HEAD'],
                         cwd=dest,

src/pip/_internal/vcs/mercurial.py

@@ -19,6 +19,9 @@ class Mercurial(VersionControl):
     repo_name = 'clone'
     schemes = ('hg', 'hg+http', 'hg+https', 'hg+ssh', 'hg+static-http')
 
+    def get_base_rev_args(self, rev):
+        return [rev]
+
     def export(self, location):
         """Export the Hg repository at the url to the destination location"""
         with TempDirectory(kind="export") as temp_dir:
@@ -41,29 +44,28 @@ def switch(self, dest, url, rev_options):
                 'Could not switch Mercurial repository to %s: %s', url, exc,
             )
         else:
-            self.run_command(['update', '-q'] + rev_options, cwd=dest)
+            cmd_args = ['update', '-q'] + rev_options.to_args()
+            self.run_command(cmd_args, cwd=dest)
 
     def update(self, dest, rev_options):
         self.run_command(['pull', '-q'], cwd=dest)
-        self.run_command(['update', '-q'] + rev_options, cwd=dest)
+        cmd_args = ['update', '-q'] + rev_options.to_args()
+        self.run_command(cmd_args, cwd=dest)
 
     def obtain(self, dest):
         url, rev = self.get_url_rev()
-        if rev:
-            rev_options = [rev]
-            rev_display = ' (to revision %s)' % rev
-        else:
-            rev_options = []
-            rev_display = ''
-        if self.check_destination(dest, url, rev_options, rev_display):
+        rev_options = self.make_rev_options(rev)
+        if self.check_destination(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])
-            self.run_command(['update', '-q'] + rev_options, cwd=dest)
+            cmd_args = ['update', '-q'] + rev_options.to_args()
+            self.run_command(cmd_args, cwd=dest)
 
     def get_url(self, location):
         url = self.run_command(