Skip to content

Create a custom call_subprocess for vcs commands avoiding merging of stdout and stderr, and introduces a new SubprocessError exception #7969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/7968.bugfix
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 3 additions & 1 deletion src/pip/_internal/cli/base_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
CommandError,
InstallationError,
PreviousBuildDirError,
SubProcessError,
UninstallationError,
)
from pip._internal.utils.deprecation import deprecated
Expand Down Expand Up @@ -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)

Expand Down
5 changes: 5 additions & 0 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""

Expand Down
7 changes: 3 additions & 4 deletions src/pip/_internal/vcs/bazaar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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: ',
Expand All @@ -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]

Expand Down
27 changes: 15 additions & 12 deletions src/pip/_internal/vcs/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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()

Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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()

Expand All @@ -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, '..'))
Expand Down Expand Up @@ -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'))

Expand Down
17 changes: 7 additions & 10 deletions src/pip/_internal/vcs/mercurial.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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'))

Expand Down
10 changes: 5 additions & 5 deletions src/pip/_internal/vcs/subversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 ()

Expand Down Expand Up @@ -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
Expand Down
Loading