McSinyx
/
pip
mirror of https://github.com/pypa/pip
1
1
Fork 0
Code Issues Releases Activity

Merge pull request #9331 from sbidoul/7969-revert-sbi 

Revert #7969 and fix VCS stdout/stderr capture
This commit is contained in:
Pradyun Gedam 2021-01-09 21:09:04 +00:00 committed by GitHub
parent e157cf56b2 b3d348d6e7
commit 9b83654de8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 174 additions and 167 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
news/8876.bugfix.rst Normal file
View File

@ -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.

3
src/pip/_internal/cli/base_command.py
View File

@ -22,7 +22,6 @@ from pip._internal.exceptions import (
InstallationError,
NetworkConnectionError,
PreviousBuildDirError,
SubProcessError,
UninstallationError,
)
from pip._internal.utils.deprecation import deprecated
@ -196,7 +195,7 @@ class Command(CommandContextMixIn):
return PREVIOUS_BUILD_DIR_ERROR
except (InstallationError, UninstallationError, BadCommand,
SubProcessError, NetworkConnectionError) as exc:
NetworkConnectionError) as exc:
logger.critical(str(exc))
logger.debug('Exception information:', exc_info=True)

5
src/pip/_internal/exceptions.py
View File

@ -82,11 +82,6 @@ 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"""

75
src/pip/_internal/utils/subprocess.py
View File

@ -115,7 +115,8 @@ def call_subprocess(
extra_environ=None, # type: Optional[Mapping[str, Any]]
unset_environ=None, # type: Optional[Iterable[str]]
spinner=None, # type: Optional[SpinnerInterface]
log_failed_cmd=True # type: Optional[bool]
log_failed_cmd=True, # type: Optional[bool]
stdout_only=False, # type: Optional[bool]
):
# type: (...) -> str
"""
@ -127,6 +128,9 @@ def call_subprocess(
unset_environ: an iterable of environment variable names to unset
prior to calling subprocess.Popen().
log_failed_cmd: if false, failed commands are not logged, only raised.
stdout_only: if true, return only stdout, else return both. When true,
logging of both stdout and stderr occurs when the subprocess has
terminated, else logging occurs as subprocess output is produced.
"""
if extra_ok_returncodes is None:
extra_ok_returncodes = []
@ -177,12 +181,12 @@ def call_subprocess(
proc = subprocess.Popen(
# Convert HiddenText objects to the underlying str.
reveal_command_args(cmd),
stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
stdout=subprocess.PIPE, cwd=cwd, env=env,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT if not stdout_only else subprocess.PIPE,
cwd=cwd,
env=env,
)
assert proc.stdin
assert proc.stdout
proc.stdin.close()
except Exception as exc:
if log_failed_cmd:
subprocess_logger.critical(
@ -190,25 +194,46 @@ def call_subprocess(
)
raise
all_output = []
while True:
# The "line" value is a unicode string in Python 2.
line = console_to_str(proc.stdout.readline())
if not line:
break
line = line.rstrip()
all_output.append(line + '\n')
if not stdout_only:
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())
if not line:
break
line = line.rstrip()
all_output.append(line + '\n')
# Show the line immediately.
log_subprocess(line)
# Update the spinner.
if use_spinner:
assert spinner
spinner.spin()
try:
proc.wait()
finally:
if proc.stdout:
proc.stdout.close()
output = ''.join(all_output)
else:
# In this mode, stdout and stderr are in different pipes.
# 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)
for out_line in out.splitlines():
log_subprocess(out_line)
all_output.append(out)
err = console_to_str(err_bytes)
for err_line in err.splitlines():
log_subprocess(err_line)
all_output.append(err)
output = out
# Show the line immediately.
log_subprocess(line)
# Update the spinner.
if use_spinner:
assert spinner
spinner.spin()
try:
proc.wait()
finally:
if proc.stdout:
proc.stdout.close()
proc_had_error = (
proc.returncode and proc.returncode not in extra_ok_returncodes
)
@ -243,7 +268,7 @@ def call_subprocess(
else:
raise ValueError('Invalid value: on_returncode={!r}'.format(
on_returncode))
return ''.join(all_output)
return output
def runner_with_spinner_message(message):

9
src/pip/_internal/vcs/bazaar.py
View File

@ -44,7 +44,8 @@ class Bazaar(VersionControl):
url, rev_options = self.get_url_rev_options(url)
self.run_command(
make_command('export', location, url, rev_options.to_args())
make_command('export', location, url, rev_options.to_args()),
show_stdout=False,
)
def fetch_new(self, dest, url, rev_options):
@ -82,7 +83,9 @@ class Bazaar(VersionControl):
@classmethod
def get_remote_url(cls, location):
# type: (str) -> str
urls = cls.run_command(['info'], 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: ',
@ -98,7 +101,7 @@ class Bazaar(VersionControl):
def get_revision(cls, location):
# type: (str) -> str
revision = cls.run_command(
['revno'], cwd=location,
['revno'], show_stdout=False, stdout_only=True, cwd=location,
)
return revision.splitlines()[-1]

54
src/pip/_internal/vcs/git.py
View File

@ -9,7 +9,7 @@ import urllib.request
from pip._vendor.packaging.version import parse as parse_version
from pip._internal.exceptions import BadCommand, SubProcessError
from pip._internal.exceptions import BadCommand, InstallationError
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
@ -77,7 +77,9 @@ class Git(VersionControl):
def get_git_version(self):
VERSION_PFX = 'git version '
version = self.run_command(['version'])
version = self.run_command(
['version'], show_stdout=False, stdout_only=True
)
if version.startswith(VERSION_PFX):
version = version[len(VERSION_PFX):].split()[0]
else:
@ -100,7 +102,11 @@ class Git(VersionControl):
# and to suppress the message to stderr.
args = ['symbolic-ref', '-q', 'HEAD']
output = cls.run_command(
args, extra_ok_returncodes=(1, ), cwd=location,
args,
extra_ok_returncodes=(1, ),
show_stdout=False,
stdout_only=True,
cwd=location,
)
ref = output.strip()
@ -119,7 +125,7 @@ class Git(VersionControl):
self.unpack(temp_dir.path, url=url)
self.run_command(
['checkout-index', '-a', '-f', '--prefix', location],
cwd=temp_dir.path
show_stdout=False, cwd=temp_dir.path
)
@classmethod
@ -133,13 +139,13 @@ class Git(VersionControl):
rev: the revision name.
"""
# Pass rev to pre-filter the list.
output = ''
try:
output = cls.run_command(['show-ref', rev], cwd=dest)
except SubProcessError:
pass
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:
@ -314,7 +320,10 @@ class Git(VersionControl):
# 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, ), cwd=location,
extra_ok_returncodes=(1, ),
show_stdout=False,
stdout_only=True,
cwd=location,
)
remotes = stdout.splitlines()
try:
@ -336,9 +345,11 @@ class Git(VersionControl):
"""
try:
cls.run_command(
['rev-parse', '-q', '--verify', "sha^" + rev], cwd=location
['rev-parse', '-q', '--verify', "sha^" + rev],
cwd=location,
log_failed_cmd=False,
)
except SubProcessError:
except InstallationError:
return False
else:
return True
@ -349,7 +360,10 @@ class Git(VersionControl):
if rev is None:
rev = 'HEAD'
current_rev = cls.run_command(
['rev-parse', rev], cwd=location,
['rev-parse', rev],
show_stdout=False,
stdout_only=True,
cwd=location,
)
return current_rev.strip()
@ -362,7 +376,10 @@ class Git(VersionControl):
# find the repo root
git_dir = cls.run_command(
['rev-parse', '--git-dir'],
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, '..'))
@ -420,13 +437,16 @@ class Git(VersionControl):
r = cls.run_command(
['rev-parse', '--show-toplevel'],
cwd=location,
show_stdout=False,
stdout_only=True,
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 SubProcessError:
except InstallationError:
return None
return os.path.normpath(r.rstrip('\r\n'))

28
src/pip/_internal/vcs/mercurial.py
View File

@ -5,7 +5,7 @@ import configparser
import logging
import os
from pip._internal.exceptions import BadCommand, SubProcessError
from pip._internal.exceptions import BadCommand, InstallationError
from pip._internal.utils.misc import display_path
from pip._internal.utils.subprocess import make_command
from pip._internal.utils.temp_dir import TempDirectory
@ -44,7 +44,7 @@ class Mercurial(VersionControl):
self.unpack(temp_dir.path, url=url)
self.run_command(
['archive', location], cwd=temp_dir.path
['archive', location], show_stdout=False, cwd=temp_dir.path
)
def fetch_new(self, dest, url, rev_options):
@ -90,7 +90,10 @@ class Mercurial(VersionControl):
# type: (str) -> str
url = cls.run_command(
['showconfig', 'paths.default'],
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()
@ -102,7 +105,11 @@ class Mercurial(VersionControl):
Return the repository-local changeset revision number, as an integer.
"""
current_revision = cls.run_command(
['parents', '--template={rev}'], cwd=location).strip()
['parents', '--template={rev}'],
show_stdout=False,
stdout_only=True,
cwd=location,
).strip()
return current_revision
@classmethod
@ -113,7 +120,10 @@ class Mercurial(VersionControl):
"""
current_rev_hash = cls.run_command(
['parents', '--template={node}'],
cwd=location).strip()
show_stdout=False,
stdout_only=True,
cwd=location,
).strip()
return current_rev_hash
@classmethod
@ -129,7 +139,8 @@ class Mercurial(VersionControl):
"""
# find the repo root
repo_root = cls.run_command(
['root'], 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)
@ -143,13 +154,16 @@ class Mercurial(VersionControl):
r = cls.run_command(
['root'],
cwd=location,
show_stdout=False,
stdout_only=True,
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 SubProcessError:
except InstallationError:
return None
return os.path.normpath(r.rstrip('\r\n'))

13
src/pip/_internal/vcs/subversion.py
View File

@ -137,7 +137,7 @@ class Subversion(VersionControl):
@classmethod
def _get_svn_url_rev(cls, location):
from pip._internal.exceptions import SubProcessError
from pip._internal.exceptions import InstallationError
entries_path = os.path.join(location, cls.dirname, 'entries')
if os.path.exists(entries_path):
@ -170,12 +170,14 @@ class Subversion(VersionControl):
# are only potentially needed for remote server requests.
xml = cls.run_command(
['info', '--xml', location],
show_stdout=False,
stdout_only=True,
)
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 SubProcessError:
except InstallationError:
url, revs = None, []
if revs:
@ -221,8 +223,9 @@ class Subversion(VersionControl):
# 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'])
version = self.run_command(
['--version'], show_stdout=False, stdout_only=True
)
if not version.startswith(version_prefix):
return ()
@ -304,7 +307,7 @@ class Subversion(VersionControl):
'export', self.get_remote_call_options(),
rev_options.to_args(), url, location,
)
self.run_command(cmd_args)
self.run_command(cmd_args, show_stdout=False)
def fetch_new(self, dest, url, rev_options):
# type: (str, HiddenText, RevOptions) -> None

121
src/pip/_internal/vcs/versioncontrol.py
View File

@ -3,15 +3,12 @@
import logging
import os
import shutil
import subprocess
import sys
import urllib.parse
from pip._vendor import pkg_resources
from pip._internal.exceptions import BadCommand, InstallationError, SubProcessError
from pip._internal.utils.compat import console_to_str
from pip._internal.utils.logging import subprocess_logger
from pip._internal.exceptions import BadCommand, InstallationError
from pip._internal.utils.misc import (
ask_path_exists,
backup_dir,
@ -20,12 +17,7 @@ from pip._internal.utils.misc import (
hide_value,
rmtree,
)
from pip._internal.utils.subprocess import (
format_command_args,
make_command,
make_subprocess_output_error,
reveal_command_args,
)
from pip._internal.utils.subprocess import call_subprocess, make_command
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.urls import get_url_scheme
@ -43,6 +35,7 @@ if MYPY_CHECK_RUNNING:
Union,
)
from pip._internal.cli.spinners import SpinnerInterface
from pip._internal.utils.misc import HiddenText
from pip._internal.utils.subprocess import CommandArgs
@ -83,94 +76,6 @@ def make_vcs_requirement_url(repo_url, rev, project_name, subdir=None):
return req
def call_subprocess(
cmd, # type: Union[List[str], CommandArgs]
cwd=None, # type: Optional[str]
extra_environ=None, # type: Optional[Mapping[str, Any]]
extra_ok_returncodes=None, # type: Optional[Iterable[int]]
log_failed_cmd=True # type: Optional[bool]
):
# type: (...) -> str
"""
Args:
extra_ok_returncodes: an iterable of integer return codes that are
acceptable, in addition to 0. Defaults to None, which means [].
log_failed_cmd: if false, failed commands are not logged,
only raised.
"""
if extra_ok_returncodes is None:
extra_ok_returncodes = []
# log the subprocess output at DEBUG level.
log_subprocess = subprocess_logger.debug
env = os.environ.copy()
if extra_environ:
env.update(extra_environ)
# Whether the subprocess will be visible in the console.
showing_subprocess = True
command_desc = format_command_args(cmd)
try:
proc = subprocess.Popen(
# Convert HiddenText objects to the underlying str.
reveal_command_args(cmd),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=cwd
)
if proc.stdin:
proc.stdin.close()
except Exception as exc:
if log_failed_cmd:
subprocess_logger.critical(
"Error %s while executing command %s", exc, command_desc,
)
raise
all_output = []
while True:
# The "line" value is a unicode string in Python 2.
line = None
if proc.stdout:
line = console_to_str(proc.stdout.readline())
if not line:
break
line = line.rstrip()
all_output.append(line + '\n')
# Show the line immediately.
log_subprocess(line)
try:
proc.wait()
finally:
if proc.stdout:
proc.stdout.close()
if proc.stderr:
proc.stderr.close()
proc_had_error = (
proc.returncode and proc.returncode not in extra_ok_returncodes
)
if proc_had_error:
if not showing_subprocess and log_failed_cmd:
# Then the subprocess streams haven't been logged to the
# console yet.
msg = make_subprocess_output_error(
cmd_args=cmd,
cwd=cwd,
lines=all_output,
exit_status=proc.returncode,
)
subprocess_logger.error(msg)
exc_msg = (
'Command errored out with exit status {}: {} '
'Check the logs for full command output.'
).format(proc.returncode, command_desc)
raise SubProcessError(exc_msg)
return ''.join(all_output)
def find_path_to_setup_from_repo_root(location, repo_root):
# type: (str, str) -> Optional[str]
"""
@ -754,10 +659,15 @@ class VersionControl:
def run_command(
cls,
cmd, # type: Union[List[str], CommandArgs]
show_stdout=True, # type: bool
cwd=None, # type: Optional[str]
extra_environ=None, # type: Optional[Mapping[str, Any]]
on_returncode='raise', # type: str
extra_ok_returncodes=None, # type: Optional[Iterable[int]]
log_failed_cmd=True # type: bool
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
stdout_only=False, # type: bool
):
# type: (...) -> str
"""
@ -767,10 +677,15 @@ class VersionControl:
"""
cmd = make_command(cls.name, *cmd)
try:
return call_subprocess(cmd, cwd,
extra_environ=extra_environ,
return call_subprocess(cmd, show_stdout, cwd,
on_returncode=on_returncode,
extra_ok_returncodes=extra_ok_returncodes,
log_failed_cmd=log_failed_cmd)
command_desc=command_desc,
extra_environ=extra_environ,
unset_environ=cls.unset_environ,
spinner=spinner,
log_failed_cmd=log_failed_cmd,
stdout_only=stdout_only)
except FileNotFoundError:
# errno.ENOENT = no such file or directory
# In other words, the VCS executable isn't available

30
tests/unit/test_utils_subprocess.py
View File

@ -13,6 +13,7 @@ from pip._internal.utils.subprocess import (
format_command_args,
make_command,
make_subprocess_output_error,
subprocess_logger,
)
@ -153,6 +154,35 @@ def test_make_subprocess_output_error__non_ascii_line():
assert actual == expected, f'actual: {actual}'
@pytest.mark.parametrize(
('stdout_only', 'expected'),
[
(True, ("out\n", "out\r\n")),
(False, ("out\nerr\n", "out\r\nerr\r\n", "err\nout\n", "err\r\nout\r\n")),
],
)
def test_call_subprocess_stdout_only(capfd, monkeypatch, stdout_only, expected):
log = []
monkeypatch.setattr(subprocess_logger, "debug", lambda *args: log.append(args[0]))
out = call_subprocess(
[
sys.executable,
"-c",
"import sys; "
"sys.stdout.write('out\\n'); "
"sys.stderr.write('err\\n')"
],
stdout_only=stdout_only,
)
assert out in expected
captured = capfd.readouterr()
assert captured.err == ""
assert (
log == ["Running command %s", "out", "err"]
or log == ["Running command %s", "err", "out"]
)
class FakeSpinner(SpinnerInterface):
def __init__(self):