vcs: capture subprocess stdout only

Author: sbidoul

news/8876.bugfix.rst

@@ -0,0 +1,3 @@
+Fixed hanging VCS subprocess calls when the VCS outputs a large amount of data
+on stderr. Restored logging of VCS errors that was inadvertently removed in pip
+20.2.

src/pip/_internal/utils/subprocess.py

@@ -190,9 +190,6 @@ def call_subprocess(
             cwd=cwd,
             env=env,
         )
-        assert proc.stdin
-        assert proc.stdout
-        proc.stdin.close()
     except Exception as exc:
         if log_failed_cmd:
             subprocess_logger.critical(
@@ -201,7 +198,10 @@ def call_subprocess(
         raise
     all_output = []
     if not stdout_only:
-        # In this mode, stdout and stderr are in the same pip.
+        assert proc.stdout
+        assert proc.stdin
+        proc.stdin.close()
+        # In this mode, stdout and stderr are in the same pipe.
         while True:
             # The "line" value is a unicode string in Python 2.
             line = console_to_str(proc.stdout.readline())
@@ -224,7 +224,7 @@ def call_subprocess(
         output = ''.join(all_output)
     else:
         # In this mode, stdout and stderr are in different pipes.
-        # We must use the communicate which is the only safe way to read both.
+        # We must use communicate() which is the only safe way to read both.
         out_bytes, err_bytes = proc.communicate()
         # log line by line to preserve pip log indenting
         out = console_to_str(out_bytes)

src/pip/_internal/vcs/bazaar.py

@@ -93,7 +93,9 @@ 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'], show_stdout=False, stdout_only=True, cwd=location
+        )
         for line in urls.splitlines():
             line = line.strip()
             for x in ('checkout of branch: ',
@@ -108,7 +110,7 @@ def get_remote_url(cls, location):
     @classmethod
     def get_revision(cls, location):
         revision = cls.run_command(
-            ['revno'], show_stdout=False, cwd=location,
+            ['revno'], show_stdout=False, stdout_only=True, cwd=location,
         )
         return revision.splitlines()[-1]
 

src/pip/_internal/vcs/git.py

@@ -79,7 +79,9 @@ 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'], show_stdout=False, stdout_only=True
+        )
         if version.startswith(VERSION_PFX):
             version = version[len(VERSION_PFX):].split()[0]
         else:
@@ -102,7 +104,11 @@ 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, ),
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
         )
         ref = output.strip()
 
@@ -135,8 +141,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 = cls.run_command(
+            ['show-ref', rev],
+            cwd=dest,
+            show_stdout=False,
+            stdout_only=True,
+            on_returncode='ignore',
+        )
         refs = {}
         for line in output.strip().splitlines():
             try:
@@ -310,7 +321,10 @@ 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, ),
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
         )
         remotes = stdout.splitlines()
         try:
@@ -346,7 +360,10 @@ 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],
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
         )
         return current_rev.strip()
 
@@ -359,7 +376,10 @@ def get_subdirectory(cls, location):
         # find the repo root
         git_dir = cls.run_command(
             ['rev-parse', '--git-dir'],
-            show_stdout=False, cwd=location).strip()
+            show_stdout=False,
+            stdout_only=True,
+            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, '..'))
@@ -418,6 +438,7 @@ def get_repository_root(cls, location):
                 ['rev-parse', '--show-toplevel'],
                 cwd=location,
                 show_stdout=False,
+                stdout_only=True,
                 on_returncode='raise',
                 log_failed_cmd=False,
             )

src/pip/_internal/vcs/mercurial.py

@@ -92,7 +92,10 @@ 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()
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
+        ).strip()
         if cls._is_local_repository(url):
             url = path_to_url(url)
         return url.strip()
@@ -104,7 +107,10 @@ def get_revision(cls, location):
         """
         current_revision = cls.run_command(
             ['parents', '--template={rev}'],
-            show_stdout=False, cwd=location).strip()
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
+        ).strip()
         return current_revision
 
     @classmethod
@@ -115,7 +121,10 @@ def get_requirement_revision(cls, location):
         """
         current_rev_hash = cls.run_command(
             ['parents', '--template={node}'],
-            show_stdout=False, cwd=location).strip()
+            show_stdout=False,
+            stdout_only=True,
+            cwd=location,
+        ).strip()
         return current_rev_hash
 
     @classmethod
@@ -131,7 +140,8 @@ def get_subdirectory(cls, location):
         """
         # find the repo root
         repo_root = cls.run_command(
-            ['root'], show_stdout=False, cwd=location).strip()
+            ['root'], show_stdout=False, stdout_only=True, 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)
@@ -146,6 +156,7 @@ def get_repository_root(cls, location):
                 ['root'],
                 cwd=location,
                 show_stdout=False,
+                stdout_only=True,
                 on_returncode='raise',
                 log_failed_cmd=False,
             )

src/pip/_internal/vcs/subversion.py

@@ -167,6 +167,7 @@ def _get_svn_url_rev(cls, location):
                 xml = cls.run_command(
                     ['info', '--xml', location],
                     show_stdout=False,
+                    stdout_only=True,
                 )
                 url = _svn_info_xml_url_re.search(xml).group(1)
                 revs = [
@@ -218,7 +219,9 @@ def call_vcs_version(self):
         #   svn, version 1.12.0-SlikSvn (SlikSvn/1.12.0)
         #      compiled May 28 2019, 13:44:56 on x86_64-microsoft-windows6.2
         version_prefix = 'svn, version '
-        version = self.run_command(['--version'], show_stdout=False)
+        version = self.run_command(
+            ['--version'], show_stdout=False, stdout_only=True
+        )
         if not version.startswith(version_prefix):
             return ()
 

src/pip/_internal/vcs/versioncontrol.py

@@ -676,7 +676,8 @@ def run_command(
         command_desc=None,  # type: Optional[str]
         extra_environ=None,  # type: Optional[Mapping[str, Any]]
         spinner=None,  # type: Optional[SpinnerInterface]
-        log_failed_cmd=True  # type: bool
+        log_failed_cmd=True,  # type: bool
+        stdout_only=False,  # type: bool
     ):
         # type: (...) -> Text
         """
@@ -693,7 +694,8 @@ def run_command(
                                    extra_environ=extra_environ,
                                    unset_environ=cls.unset_environ,
                                    spinner=spinner,
-                                   log_failed_cmd=log_failed_cmd)
+                                   log_failed_cmd=log_failed_cmd,
+                                   stdout_only=stdout_only)
         except OSError as e:
             # errno.ENOENT = no such file or directory
             # In other words, the VCS executable isn't available