Properly support interactive Subversion client

news/6386.bugfix

@@ -0,0 +1,2 @@
+Extend to Subversion 1.8+ the behavior of calling Subversion in
+interactive mode when pip is run interactively.

src/pip/_internal/vcs/subversion.py

@@ -20,6 +20,7 @@
 
 if MYPY_CHECK_RUNNING:
     from typing import List, Optional, Tuple
+    from pip._internal.vcs import RevOptions
 
 logger = logging.getLogger(__name__)
 
@@ -48,26 +49,34 @@ def export(self, location, url):
                 # Subversion doesn't like to check out over an existing
                 # directory --force fixes this, but was only added in svn 1.5
                 rmtree(location)
-            cmd_args = ['export'] + rev_options.to_args() + [url, location]
+            cmd_args = (['export'] + self.get_remote_call_options() +
+                        rev_options.to_args() + [url, location])
             self.run_command(cmd_args, show_stdout=False)
 
     def fetch_new(self, dest, url, rev_options):
+        # type: (str, str, RevOptions) -> None
         rev_display = rev_options.to_display()
         logger.info(
             'Checking out %s%s to %s',
             url,
             rev_display,
             display_path(dest),
         )
-        cmd_args = ['checkout', '-q'] + rev_options.to_args() + [url, dest]
+        cmd_args = (['checkout', '-q'] +
+                    self.get_remote_call_options() +
+                    rev_options.to_args() + [url, dest])
         self.run_command(cmd_args)
 
     def switch(self, dest, url, rev_options):
-        cmd_args = ['switch'] + rev_options.to_args() + [url, dest]
+        # type: (str, str, RevOptions) -> None
+        cmd_args = (['switch'] + self.get_remote_call_options() +
+                    rev_options.to_args() + [url, dest])
         self.run_command(cmd_args)
 
     def update(self, dest, url, rev_options):
-        cmd_args = ['update'] + rev_options.to_args() + [dest]
+        # type: (str, str, RevOptions) -> None
+        cmd_args = (['update'] + self.get_remote_call_options() +
+                    rev_options.to_args() + [dest])
         self.run_command(cmd_args)
 
     @classmethod
@@ -177,6 +186,11 @@ def _get_svn_url_rev(cls, location):
         else:
             try:
                 # subversion >= 1.7
+                # Note that using get_remote_call_options is not necessary here
+                # because `svn info` is being run against a local directory.
+                # We don't need to worry about making sure interactive mode
+                # is being used to prompt for passwords, because passwords
+                # are only potentially needed for remote server requests.
                 xml = cls.run_command(
                     ['info', '--xml', location],
                     show_stdout=False,

tests/functional/test_install_vcs_svn.py

@@ -1,30 +1,66 @@
-import pytest
+from unittest import TestCase
+
 from mock import patch
 
+from pip._internal.vcs import RevOptions
 from pip._internal.vcs.subversion import Subversion
 
 
-@patch('pip._internal.vcs.call_subprocess')
-@pytest.mark.network
-def test_obtain_should_recognize_auth_info_url(call_subprocess_mock, script):
-    url = 'svn+http://username:[email protected]/'
-    svn = Subversion()
-    svn.obtain(script.scratch_path / 'test', url=url)
-    assert call_subprocess_mock.call_args[0][0] == [
-        svn.name, 'checkout', '-q', '--username', 'username', '--password',
-        'password', 'http://svn.example.com/',
-        script.scratch_path / 'test',
-    ]
-
-
-@patch('pip._internal.vcs.call_subprocess')
-@pytest.mark.network
-def test_export_should_recognize_auth_info_url(call_subprocess_mock, script):
-    url = 'svn+http://username:[email protected]/'
-    svn = Subversion()
-    svn.export(script.scratch_path / 'test', url=url)
-    assert call_subprocess_mock.call_args[0][0] == [
-        svn.name, 'export', '--username', 'username', '--password',
-        'password', 'http://svn.example.com/',
-        script.scratch_path / 'test',
-    ]
+class TestSubversionArgs(TestCase):
+    def setUp(self):
+        patcher = patch('pip._internal.vcs.call_subprocess')
+        self.addCleanup(patcher.stop)
+        self.call_subprocess_mock = patcher.start()
+
+        # Test Data.
+        self.url = 'svn+http://username:[email protected]/'
+        # use_interactive is set to False to test that remote call options are
+        # properly added.
+        self.svn = Subversion(use_interactive=False)
+        self.rev_options = RevOptions(Subversion)
+        self.dest = '/tmp/test'
+
+    def assert_call_args(self, args):
+        assert self.call_subprocess_mock.call_args[0][0] == args
+
+    def test_obtain(self):
+        self.svn.obtain(self.dest, self.url)
+        self.assert_call_args(
+            ['svn', 'checkout', '-q', '--non-interactive', '--username',
+             'username', '--password', 'password',
+             'http://svn.example.com/', '/tmp/test'])
+
+    def test_export(self):
+        self.svn.export(self.dest, self.url)
+        self.assert_call_args(
+            ['svn', 'export', '--non-interactive', '--username', 'username',
+             '--password', 'password', 'http://svn.example.com/',
+             '/tmp/test'])
+
+    def test_fetch_new(self):
+        self.svn.fetch_new(self.dest, self.url, self.rev_options)
+        self.assert_call_args(
+            ['svn', 'checkout', '-q', '--non-interactive',
+             'svn+http://username:[email protected]/',
+             '/tmp/test'])
+
+    def test_fetch_new_revision(self):
+        rev_options = RevOptions(Subversion, '123')
+        self.svn.fetch_new(self.dest, self.url, rev_options)
+        self.assert_call_args(
+            ['svn', 'checkout', '-q', '--non-interactive',
+             '-r', '123',
+             'svn+http://username:[email protected]/',
+             '/tmp/test'])
+
+    def test_switch(self):
+        self.svn.switch(self.dest, self.url, self.rev_options)
+        self.assert_call_args(
+            ['svn', 'switch', '--non-interactive',
+             'svn+http://username:[email protected]/',
+             '/tmp/test'])
+
+    def test_update(self):
+        self.svn.update(self.dest, self.url, self.rev_options)
+        self.assert_call_args(
+            ['svn', 'update', '--non-interactive', '/tmp/test'])