Merge pull request #7969 from deveshks/fix-svn-version-check

Author: pradyunsg

news/7968.bugfix

@@ -0,0 +1 @@
+The VCS commands run by pip as subprocesses don't merge stdout and stderr anymore, improving the output parsing by subsequent commands.

src/pip/_internal/cli/base_command.py

@@ -28,6 +28,7 @@
     CommandError,
     InstallationError,
     PreviousBuildDirError,
+    SubProcessError,
     UninstallationError,
 )
 from pip._internal.utils.deprecation import deprecated
@@ -201,7 +202,8 @@ def _main(self, args):
             logger.debug('Exception information:', exc_info=True)
 
             return PREVIOUS_BUILD_DIR_ERROR
-        except (InstallationError, UninstallationError, BadCommand) as exc:
+        except (InstallationError, UninstallationError, BadCommand,
+                SubProcessError) as exc:
             logger.critical(str(exc))
             logger.debug('Exception information:', exc_info=True)
 

src/pip/_internal/exceptions.py

@@ -84,6 +84,11 @@ class CommandError(PipError):
     """Raised when there is an error in command-line arguments"""
 
 
+class SubProcessError(PipError):
+    """Raised when there is an error raised while executing a
+    command in subprocess"""
+
+
 class PreviousBuildDirError(PipError):
     """Raised when there's a previous conflicting build directory"""
 

src/pip/_internal/vcs/bazaar.py

@@ -54,8 +54,7 @@ def export(self, location, url):
 
         url, rev_options = self.get_url_rev_options(url)
         self.run_command(
-            make_command('export', location, url, rev_options.to_args()),
-            show_stdout=False,
+            make_command('export', location, url, rev_options.to_args())
         )
 
     def fetch_new(self, dest, url, rev_options):
@@ -92,7 +91,7 @@ def get_url_rev_and_auth(cls, url):
 
     @classmethod
     def get_remote_url(cls, location):
-        urls = cls.run_command(['info'], show_stdout=False, cwd=location)
+        urls = cls.run_command(['info'], cwd=location)
         for line in urls.splitlines():
             line = line.strip()
             for x in ('checkout of branch: ',
@@ -107,7 +106,7 @@ def get_remote_url(cls, location):
     @classmethod
     def get_revision(cls, location):
         revision = cls.run_command(
-            ['revno'], show_stdout=False, cwd=location,
+            ['revno'], cwd=location,
         )
         return revision.splitlines()[-1]
 

src/pip/_internal/vcs/git.py

@@ -11,7 +11,7 @@
 from pip._vendor.six.moves.urllib import parse as urllib_parse
 from pip._vendor.six.moves.urllib import request as urllib_request
 
-from pip._internal.exceptions import BadCommand, InstallationError
+from pip._internal.exceptions import BadCommand, SubProcessError
 from pip._internal.utils.misc import display_path, hide_url
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
@@ -78,7 +78,7 @@ def is_immutable_rev_checkout(self, url, dest):
 
     def get_git_version(self):
         VERSION_PFX = 'git version '
-        version = self.run_command(['version'], show_stdout=False)
+        version = self.run_command(['version'])
         if version.startswith(VERSION_PFX):
             version = version[len(VERSION_PFX):].split()[0]
         else:
@@ -101,7 +101,7 @@ def get_current_branch(cls, location):
         # and to suppress the message to stderr.
         args = ['symbolic-ref', '-q', 'HEAD']
         output = cls.run_command(
-            args, extra_ok_returncodes=(1, ), show_stdout=False, cwd=location,
+            args, extra_ok_returncodes=(1, ), cwd=location,
         )
         ref = output.strip()
 
@@ -120,7 +120,7 @@ def export(self, location, url):
             self.unpack(temp_dir.path, url=url)
             self.run_command(
                 ['checkout-index', '-a', '-f', '--prefix', location],
-                show_stdout=False, cwd=temp_dir.path
+                cwd=temp_dir.path
             )
 
     @classmethod
@@ -134,8 +134,13 @@ def get_revision_sha(cls, dest, rev):
           rev: the revision name.
         """
         # Pass rev to pre-filter the list.
-        output = cls.run_command(['show-ref', rev], cwd=dest,
-                                 show_stdout=False, on_returncode='ignore')
+
+        output = ''
+        try:
+            output = cls.run_command(['show-ref', rev], cwd=dest)
+        except SubProcessError:
+            pass
+
         refs = {}
         for line in output.strip().splitlines():
             try:
@@ -286,7 +291,7 @@ def get_remote_url(cls, location):
         # exits with return code 1 if there are no matching lines.
         stdout = cls.run_command(
             ['config', '--get-regexp', r'remote\..*\.url'],
-            extra_ok_returncodes=(1, ), show_stdout=False, cwd=location,
+            extra_ok_returncodes=(1, ), cwd=location,
         )
         remotes = stdout.splitlines()
         try:
@@ -306,7 +311,7 @@ def get_revision(cls, location, rev=None):
         if rev is None:
             rev = 'HEAD'
         current_rev = cls.run_command(
-            ['rev-parse', rev], show_stdout=False, cwd=location,
+            ['rev-parse', rev], cwd=location,
         )
         return current_rev.strip()
 
@@ -319,7 +324,7 @@ def get_subdirectory(cls, location):
         # find the repo root
         git_dir = cls.run_command(
             ['rev-parse', '--git-dir'],
-            show_stdout=False, cwd=location).strip()
+            cwd=location).strip()
         if not os.path.isabs(git_dir):
             git_dir = os.path.join(location, git_dir)
         repo_root = os.path.abspath(os.path.join(git_dir, '..'))
@@ -378,15 +383,13 @@ def get_repository_root(cls, location):
             r = cls.run_command(
                 ['rev-parse', '--show-toplevel'],
                 cwd=location,
-                show_stdout=False,
-                on_returncode='raise',
                 log_failed_cmd=False,
             )
         except BadCommand:
             logger.debug("could not determine if %s is under git control "
                          "because git is not available", location)
             return None
-        except InstallationError:
+        except SubProcessError:
             return None
         return os.path.normpath(r.rstrip('\r\n'))
 

src/pip/_internal/vcs/mercurial.py

@@ -8,7 +8,7 @@
 
 from pip._vendor.six.moves import configparser
 
-from pip._internal.exceptions import BadCommand, InstallationError
+from pip._internal.exceptions import BadCommand, SubProcessError
 from pip._internal.utils.misc import display_path
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
@@ -47,7 +47,7 @@ def export(self, location, url):
             self.unpack(temp_dir.path, url=url)
 
             self.run_command(
-                ['archive', location], show_stdout=False, cwd=temp_dir.path
+                ['archive', location], cwd=temp_dir.path
             )
 
     def fetch_new(self, dest, url, rev_options):
@@ -92,7 +92,7 @@ def update(self, dest, url, rev_options):
     def get_remote_url(cls, location):
         url = cls.run_command(
             ['showconfig', 'paths.default'],
-            show_stdout=False, cwd=location).strip()
+            cwd=location).strip()
         if cls._is_local_repository(url):
             url = path_to_url(url)
         return url.strip()
@@ -103,8 +103,7 @@ def get_revision(cls, location):
         Return the repository-local changeset revision number, as an integer.
         """
         current_revision = cls.run_command(
-            ['parents', '--template={rev}'],
-            show_stdout=False, cwd=location).strip()
+            ['parents', '--template={rev}'], cwd=location).strip()
         return current_revision
 
     @classmethod
@@ -115,7 +114,7 @@ def get_requirement_revision(cls, location):
         """
         current_rev_hash = cls.run_command(
             ['parents', '--template={node}'],
-            show_stdout=False, cwd=location).strip()
+            cwd=location).strip()
         return current_rev_hash
 
     @classmethod
@@ -131,7 +130,7 @@ def get_subdirectory(cls, location):
         """
         # find the repo root
         repo_root = cls.run_command(
-            ['root'], show_stdout=False, cwd=location).strip()
+            ['root'], cwd=location).strip()
         if not os.path.isabs(repo_root):
             repo_root = os.path.abspath(os.path.join(location, repo_root))
         return find_path_to_setup_from_repo_root(location, repo_root)
@@ -145,15 +144,13 @@ def get_repository_root(cls, location):
             r = cls.run_command(
                 ['root'],
                 cwd=location,
-                show_stdout=False,
-                on_returncode='raise',
                 log_failed_cmd=False,
             )
         except BadCommand:
             logger.debug("could not determine if %s is under hg control "
                          "because hg is not available", location)
             return None
-        except InstallationError:
+        except SubProcessError:
             return None
         return os.path.normpath(r.rstrip('\r\n'))
 

src/pip/_internal/vcs/subversion.py

@@ -132,7 +132,7 @@ def get_remote_url(cls, location):
 
     @classmethod
     def _get_svn_url_rev(cls, location):
-        from pip._internal.exceptions import InstallationError
+        from pip._internal.exceptions import SubProcessError
 
         entries_path = os.path.join(location, cls.dirname, 'entries')
         if os.path.exists(entries_path):
@@ -165,13 +165,12 @@ def _get_svn_url_rev(cls, location):
                 # are only potentially needed for remote server requests.
                 xml = cls.run_command(
                     ['info', '--xml', location],
-                    show_stdout=False,
                 )
                 url = _svn_info_xml_url_re.search(xml).group(1)
                 revs = [
                     int(m.group(1)) for m in _svn_info_xml_rev_re.finditer(xml)
                 ]
-            except InstallationError:
+            except SubProcessError:
                 url, revs = None, []
 
         if revs:
@@ -215,7 +214,8 @@ def call_vcs_version(self):
         #   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)
+        version = self.run_command(['--version'])
+
         if not version.startswith(version_prefix):
             return ()
 
@@ -297,7 +297,7 @@ def export(self, location, url):
                 'export', self.get_remote_call_options(),
                 rev_options.to_args(), url, location,
             )
-            self.run_command(cmd_args, show_stdout=False)
+            self.run_command(cmd_args)
 
     def fetch_new(self, dest, url, rev_options):
         # type: (str, HiddenText, RevOptions) -> None