Merge pull request #6439 from johnthagen/svn-interactive

Subversion interactive support

src/pip/_internal/vcs/subversion.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import re
+import sys
 
 from pip._internal.utils.logging import indent_log
 from pip._internal.utils.misc import (
@@ -18,7 +19,7 @@
 
 
 if MYPY_CHECK_RUNNING:
-    from typing import Optional, Tuple
+    from typing import List, Optional, Tuple
 
 logger = logging.getLogger(__name__)
 
@@ -37,36 +38,6 @@ def should_add_vcs_url_prefix(cls, remote_url):
     def get_base_rev_args(rev):
         return ['-r', rev]
 
-    def get_vcs_version(self):
-        # type: () -> Optional[Tuple[int, ...]]
-        """Return the version of the currently installed Subversion client.
-
-        :return: A tuple containing the parts of the version information or
-            ``None`` if the version returned from ``svn`` could not be parsed.
-        :raises: BadCommand: If ``svn`` is not installed.
-        """
-        # Example versions:
-        #   svn, version 1.10.3 (r1842928)
-        #      compiled Feb 25 2019, 14:20:39 on x86_64-apple-darwin17.0.0
-        #   svn, version 1.7.14 (r1542130)
-        #      compiled Mar 28 2018, 08:49:13 on x86_64-pc-linux-gnu
-        version_prefix = 'svn, version '
-        version = self.run_command(['--version'], show_stdout=False)
-        if not version.startswith(version_prefix):
-            return None
-
-        version = version[len(version_prefix):].split()[0]
-        version_list = version.split('.')
-        try:
-            parsed_version = tuple(map(int, version_list))
-        except ValueError:
-            return None
-
-        if not parsed_version:
-            return None
-
-        return parsed_version
-
     def export(self, location, url):
         """Export the svn repository at the url to the destination location"""
         url, rev_options = self.get_url_rev_options(url)
@@ -230,5 +201,102 @@ def is_commit_id_equal(cls, dest, name):
         """Always assume the versions don't match"""
         return False
 
+    def __init__(self, use_interactive=None):
+        # type: (bool) -> None
+        if use_interactive is None:
+            use_interactive = sys.stdin.isatty()
+        self.use_interactive = use_interactive
+
+        # This member is used to cache the fetched version of the current
+        # ``svn`` client.
+        # Special value definitions:
+        #   None: Not evaluated yet.
+        #   Empty tuple: Could not parse version.
+        self._vcs_version = None  # type: Optional[Tuple[int, ...]]
+
+        super(Subversion, self).__init__()
+
+    def call_vcs_version(self):
+        # type: () -> Tuple[int, ...]
+        """Query the version of the currently installed Subversion client.
+
+        :return: A tuple containing the parts of the version information or
+            ``()`` if the version returned from ``svn`` could not be parsed.
+        :raises: BadCommand: If ``svn`` is not installed.
+        """
+        # Example versions:
+        #   svn, version 1.10.3 (r1842928)
+        #      compiled Feb 25 2019, 14:20:39 on x86_64-apple-darwin17.0.0
+        #   svn, version 1.7.14 (r1542130)
+        #      compiled Mar 28 2018, 08:49:13 on x86_64-pc-linux-gnu
+        version_prefix = 'svn, version '
+        version = self.run_command(['--version'], show_stdout=False)
+        if not version.startswith(version_prefix):
+            return ()
+
+        version = version[len(version_prefix):].split()[0]
+        version_list = version.split('.')
+        try:
+            parsed_version = tuple(map(int, version_list))
+        except ValueError:
+            return ()
+
+        return parsed_version
+
+    def get_vcs_version(self):
+        # type: () -> Tuple[int, ...]
+        """Return the version of the currently installed Subversion client.
+
+        If the version of the Subversion client has already been queried,
+        a cached value will be used.
+
+        :return: A tuple containing the parts of the version information or
+            ``()`` if the version returned from ``svn`` could not be parsed.
+        :raises: BadCommand: If ``svn`` is not installed.
+        """
+        if self._vcs_version is not None:
+            # Use cached version, if available.
+            # If parsing the version failed previously (empty tuple),
+            # do not attempt to parse it again.
+            return self._vcs_version
+
+        vcs_version = self.call_vcs_version()
+        self._vcs_version = vcs_version
+        return vcs_version
+
+    def get_remote_call_options(self):
+        # type: () -> List[str]
+        """Return options to be used on calls to Subversion that contact the server.
+
+        These options are applicable for the following ``svn`` subcommands used
+        in this class.
+
+            - checkout
+            - export
+            - info
+            - switch
+            - update
+
+        :return: A list of command line arguments to pass to ``svn``.
+        """
+        if not self.use_interactive:
+            # --non-interactive switch is available since Subversion 0.14.4.
+            # Subversion < 1.8 runs in interactive mode by default.
+            return ['--non-interactive']
+
+        svn_version = self.get_vcs_version()
+        # By default, Subversion >= 1.8 runs in non-interactive mode if
+        # stdin is not a TTY. Since that is how pip invokes SVN, in
+        # call_subprocess(), pip must pass --force-interactive to ensure
+        # the user can be prompted for a password, if required.
+        #   SVN added the --force-interactive option in SVN 1.8. Since
+        # e.g. RHEL/CentOS 7, which is supported until 2024, ships with
+        # SVN 1.7, pip should continue to support SVN 1.7. Therefore, pip
+        # can't safely add the option if the SVN version is < 1.8 (or unknown).
+        if svn_version >= (1, 8):
+            return ['--force-interactive']
+
+        return []
+
 
 vcs.register(Subversion)

tests/unit/test_vcs.py

@@ -379,12 +379,32 @@ def test_get_git_version():
     assert git_version >= parse_version('1.0.0')
 
 
+@pytest.mark.parametrize('use_interactive,is_atty,expected', [
+    (None, False, False),
+    (None, True, True),
+    (False, False, False),
+    (False, True, False),
+    (True, False, True),
+    (True, True, True),
+])
+@patch('sys.stdin.isatty')
+def test_subversion__init_use_interactive(
+        mock_isatty, use_interactive, is_atty, expected):
+    """
+    Test Subversion.__init__() with mocked sys.stdin.isatty() output.
+    """
+    mock_isatty.return_value = is_atty
+    svn = Subversion(use_interactive=use_interactive)
+    assert svn.use_interactive == expected
+
+
 @pytest.mark.svn
-def test_subversion__get_vcs_version():
+def test_subversion__call_vcs_version():
     """
-    Test Subversion.get_vcs_version() against local ``svn``.
+    Test Subversion.call_vcs_version() against local ``svn``.
     """
-    version = Subversion().get_vcs_version()
+    version = Subversion().call_vcs_version()
+    # All Subversion releases since 1.0.0 have used three parts.
     assert len(version) == 3
     for part in version:
         assert isinstance(part, int)
@@ -396,30 +416,82 @@ def test_subversion__get_vcs_version():
      '   compiled Feb 25 2019, 14:20:39 on x86_64-apple-darwin17.0.0',
      (1, 10, 3)),
     ('svn, version 1.9.7 (r1800392)', (1, 9, 7)),
-    ('svn, version 1.9.7a1 (r1800392)', None),
+    ('svn, version 1.9.7a1 (r1800392)', ()),
     ('svn, version 1.9 (r1800392)', (1, 9)),
-    ('svn, version .9.7 (r1800392)', None),
-    ('svn version 1.9.7 (r1800392)', None),
-    ('svn 1.9.7', None),
-    ('svn, version . .', None),
-    ('', None),
+    ('svn, version .9.7 (r1800392)', ()),
+    ('svn version 1.9.7 (r1800392)', ()),
+    ('svn 1.9.7', ()),
+    ('svn, version . .', ()),
+    ('', ()),
 ])
 @patch('pip._internal.vcs.subversion.Subversion.run_command')
-def test_subversion__get_vcs_version_patched(mock_run_command, svn_output,
-                                             expected_version):
+def test_subversion__call_vcs_version_patched(
+        mock_run_command, svn_output, expected_version):
     """
-    Test Subversion.get_vcs_version() against patched output.
+    Test Subversion.call_vcs_version() against patched output.
     """
     mock_run_command.return_value = svn_output
-    version = Subversion().get_vcs_version()
+    version = Subversion().call_vcs_version()
     assert version == expected_version
 
 
 @patch('pip._internal.vcs.subversion.Subversion.run_command')
-def test_subversion__get_vcs_version_svn_not_installed(mock_run_command):
+def test_subversion__call_vcs_version_svn_not_installed(mock_run_command):
     """
-    Test Subversion.get_vcs_version() when svn is not installed.
+    Test Subversion.call_vcs_version() when svn is not installed.
     """
     mock_run_command.side_effect = BadCommand
     with pytest.raises(BadCommand):
-        Subversion().get_vcs_version()
+        Subversion().call_vcs_version()
+
+
+@pytest.mark.parametrize('version', [
+    (),
+    (1,),
+    (1, 8),
+    (1, 8, 0),
+])
+def test_subversion__get_vcs_version_cached(version):
+    """
+    Test Subversion.get_vcs_version() with previously cached result.
+    """
+    svn = Subversion()
+    svn._vcs_version = version
+    assert svn.get_vcs_version() == version
+
+
+@pytest.mark.parametrize('vcs_version', [
+    (),
+    (1, 7),
+    (1, 8, 0),
+])
+@patch('pip._internal.vcs.subversion.Subversion.call_vcs_version')
+def test_subversion__get_vcs_version_call_vcs(mock_call_vcs, vcs_version):
+    """
+    Test Subversion.get_vcs_version() with mocked output from
+    call_vcs_version().
+    """
+    mock_call_vcs.return_value = vcs_version
+    svn = Subversion()
+    assert svn.get_vcs_version() == vcs_version
+
+    # Check that the version information is cached.
+    assert svn._vcs_version == vcs_version
+
+
+@pytest.mark.parametrize('use_interactive,vcs_version,expected_options', [
+    (False, (), ['--non-interactive']),
+    (False, (1, 7, 0), ['--non-interactive']),
+    (False, (1, 8, 0), ['--non-interactive']),
+    (True, (), []),
+    (True, (1, 7, 0), []),
+    (True, (1, 8, 0), ['--force-interactive']),
+])
+def test_subversion__get_remote_call_options(
+        use_interactive, vcs_version, expected_options):
+    """
+    Test Subversion.get_remote_call_options().
+    """
+    svn = Subversion(use_interactive=use_interactive)
+    svn._vcs_version = vcs_version
+    assert svn.get_remote_call_options() == expected_options