Require every `call_subprocess` call-site to pass `command_desc`

This serves as additional context that can be presented in error
messages.
This commit is contained in:
Pradyun Gedam 2021-11-12 13:38:01 +00:00
parent 78ef0b216f
commit 531c991ef9
No known key found for this signature in database
GPG Key ID: FF99710C4332258E
11 changed files with 74 additions and 30 deletions

View File

@ -189,7 +189,8 @@ class BuildEnvironment:
finder: "PackageFinder",
requirements: Iterable[str],
prefix_as_string: str,
message: str,
*,
kind: str,
) -> None:
prefix = self._prefixes[prefix_as_string]
assert not prefix.setup
@ -203,7 +204,7 @@ class BuildEnvironment:
finder,
requirements,
prefix,
message,
kind=kind,
)
@staticmethod
@ -212,7 +213,8 @@ class BuildEnvironment:
finder: "PackageFinder",
requirements: Iterable[str],
prefix: _Prefix,
message: str,
*,
kind: str,
) -> None:
args: List[str] = [
sys.executable,
@ -254,8 +256,13 @@ class BuildEnvironment:
args.append("--")
args.extend(requirements)
extra_environ = {"_PIP_STANDALONE_CERT": where()}
with open_spinner(message) as spinner:
call_subprocess(args, spinner=spinner, extra_environ=extra_environ)
with open_spinner(f"Installing {kind}") as spinner:
call_subprocess(
args,
command_desc=f"pip subprocess to install {kind}",
spinner=spinner,
extra_environ=extra_environ,
)
class NoOpBuildEnvironment(BuildEnvironment):
@ -283,6 +290,7 @@ class NoOpBuildEnvironment(BuildEnvironment):
finder: "PackageFinder",
requirements: Iterable[str],
prefix_as_string: str,
message: str,
*,
kind: str,
) -> None:
raise NotImplementedError()

View File

@ -54,7 +54,7 @@ class SourceDistribution(AbstractDistribution):
self.req.build_env = BuildEnvironment()
self.req.build_env.install_requirements(
finder, pyproject_requires, "overlay", "Installing build dependencies"
finder, pyproject_requires, "overlay", kind="build dependencies"
)
conflicting, missing = self.req.build_env.check_requirements(
self.req.requirements_to_check
@ -106,7 +106,7 @@ class SourceDistribution(AbstractDistribution):
if conflicting:
self._raise_conflicts("the backend dependencies", conflicting)
self.req.build_env.install_requirements(
finder, missing, "normal", "Installing backend dependencies"
finder, missing, "normal", kind="backend dependencies"
)
def _raise_conflicts(

View File

@ -86,6 +86,7 @@ def build_wheel_legacy(
try:
output = call_subprocess(
wheel_args,
command_desc="python setup.py bdist_wheel",
cwd=source_dir,
spinner=spinner,
)

View File

@ -42,5 +42,6 @@ def install_editable(
with build_env:
call_subprocess(
args,
command_desc="python setup.py develop",
cwd=unpacked_source_directory,
)

View File

@ -110,12 +110,13 @@ def call_subprocess(
cwd: Optional[str] = None,
on_returncode: 'Literal["raise", "warn", "ignore"]' = "raise",
extra_ok_returncodes: Optional[Iterable[int]] = None,
command_desc: Optional[str] = None,
extra_environ: Optional[Mapping[str, Any]] = None,
unset_environ: Optional[Iterable[str]] = None,
spinner: Optional[SpinnerInterface] = None,
log_failed_cmd: Optional[bool] = True,
stdout_only: Optional[bool] = False,
*,
command_desc: str,
) -> str:
"""
Args:
@ -166,9 +167,6 @@ def call_subprocess(
# and we have a spinner.
use_spinner = not showing_subprocess and spinner is not None
if command_desc is None:
command_desc = format_command_args(cmd)
log_subprocess("Running command %s", command_desc)
env = os.environ.copy()
if extra_environ:
@ -281,6 +279,7 @@ def runner_with_spinner_message(message: str) -> Callable[..., None]:
with open_spinner(message) as spinner:
call_subprocess(
cmd,
command_desc=message,
cwd=cwd,
extra_environ=extra_environ,
spinner=spinner,

View File

@ -91,7 +91,12 @@ class Git(VersionControl):
return not is_tag_or_branch
def get_git_version(self) -> Tuple[int, ...]:
version = self.run_command(["version"], show_stdout=False, stdout_only=True)
version = self.run_command(
["version"],
command_desc="git version",
show_stdout=False,
stdout_only=True,
)
match = GIT_VERSION_REGEX.match(version)
if not match:
logger.warning("Can't parse git version: %s", version)

View File

@ -31,7 +31,12 @@ from pip._internal.utils.misc import (
is_installable_dir,
rmtree,
)
from pip._internal.utils.subprocess import CommandArgs, call_subprocess, make_command
from pip._internal.utils.subprocess import (
CommandArgs,
call_subprocess,
format_command_args,
make_command,
)
from pip._internal.utils.urls import get_url_scheme
if TYPE_CHECKING:
@ -634,6 +639,8 @@ class VersionControl:
command name, and checks that the VCS is available
"""
cmd = make_command(cls.name, *cmd)
if command_desc is None:
command_desc = format_command_args(cmd)
try:
return call_subprocess(
cmd,

View File

@ -310,7 +310,9 @@ def _clean_one_legacy(req: InstallRequirement, global_options: List[str]) -> boo
logger.info("Running setup.py clean for %s", req.name)
try:
call_subprocess(clean_args, cwd=req.source_dir)
call_subprocess(
clean_args, command_desc="python setup.py clean", cwd=req.source_dir
)
return True
except Exception:
logger.error("Failed cleaning build dir for %s", req.name)

View File

@ -81,7 +81,7 @@ def test_build_env_allow_empty_requirements_install() -> None:
build_env = BuildEnvironment()
for prefix in ("normal", "overlay"):
build_env.install_requirements(
finder, [], prefix, "Installing build dependencies"
finder, [], prefix, kind="Installing build dependencies"
)
@ -92,15 +92,15 @@ def test_build_env_allow_only_one_install(script: PipTestEnvironment) -> None:
build_env = BuildEnvironment()
for prefix in ("normal", "overlay"):
build_env.install_requirements(
finder, ["foo"], prefix, f"installing foo in {prefix}"
finder, ["foo"], prefix, kind=f"installing foo in {prefix}"
)
with pytest.raises(AssertionError):
build_env.install_requirements(
finder, ["bar"], prefix, f"installing bar in {prefix}"
finder, ["bar"], prefix, kind=f"installing bar in {prefix}"
)
with pytest.raises(AssertionError):
build_env.install_requirements(
finder, [], prefix, f"installing in {prefix}"
finder, [], prefix, kind=f"installing in {prefix}"
)
@ -131,7 +131,7 @@ def test_build_env_requirements_check(script: PipTestEnvironment) -> None:
script,
"""
build_env.install_requirements(finder, ['foo', 'bar==3.0'], 'normal',
'installing foo in normal')
kind='installing foo in normal')
r = build_env.check_requirements(['foo', 'bar', 'other'])
assert r == (set(), {'other'}), repr(r)
@ -148,9 +148,9 @@ def test_build_env_requirements_check(script: PipTestEnvironment) -> None:
script,
"""
build_env.install_requirements(finder, ['foo', 'bar==3.0'], 'normal',
'installing foo in normal')
kind='installing foo in normal')
build_env.install_requirements(finder, ['bar==1.0'], 'overlay',
'installing foo in overlay')
kind='installing foo in overlay')
r = build_env.check_requirements(['foo', 'bar', 'other'])
assert r == (set(), {'other'}), repr(r)
@ -172,9 +172,9 @@ def test_build_env_overlay_prefix_has_priority(script: PipTestEnvironment) -> No
script,
"""
build_env.install_requirements(finder, ['pkg==2.0'], 'overlay',
'installing pkg==2.0 in overlay')
kind='installing pkg==2.0 in overlay')
build_env.install_requirements(finder, ['pkg==4.3'], 'normal',
'installing pkg==4.3 in normal')
kind='installing pkg==4.3 in normal')
""",
"""
print(__import__('pkg').__version__)

View File

@ -36,7 +36,7 @@ def test_backend(tmpdir: Path, data: TestData) -> None:
req.load_pyproject_toml()
env = BuildEnvironment()
finder = make_test_finder(find_links=[data.backends])
env.install_requirements(finder, ["dummy_backend"], "normal", "Installing")
env.install_requirements(finder, ["dummy_backend"], "normal", kind="Installing")
conflicting, missing = env.check_requirements(["dummy_backend"])
assert not conflicting and not missing
assert hasattr(req.pep517_backend, "build_wheel")
@ -83,7 +83,7 @@ def test_backend_path_and_dep(tmpdir: Path, data: TestData) -> None:
req.load_pyproject_toml()
env = BuildEnvironment()
finder = make_test_finder(find_links=[data.backends])
env.install_requirements(finder, ["dummy_backend"], "normal", "Installing")
env.install_requirements(finder, ["dummy_backend"], "normal", kind="Installing")
assert hasattr(req.pep517_backend, "build_wheel")
with env:

View File

@ -163,6 +163,7 @@ def test_call_subprocess_stdout_only(
"-c",
"import sys; sys.stdout.write('out\\n'); sys.stderr.write('err\\n')",
],
command_desc="test stdout_only",
stdout_only=stdout_only,
)
assert out in expected
@ -271,7 +272,11 @@ class TestCallSubprocess:
"""
log_level = DEBUG
args, spinner = self.prepare_call(caplog, log_level)
result = call_subprocess(args, spinner=spinner)
result = call_subprocess(
args,
command_desc="test debug logging",
spinner=spinner,
)
expected = (
["Hello", "world"],
@ -301,7 +306,11 @@ class TestCallSubprocess:
"""
log_level = INFO
args, spinner = self.prepare_call(caplog, log_level)
result = call_subprocess(args, spinner=spinner)
result = call_subprocess(
args,
command_desc="test info logging",
spinner=spinner,
)
expected: Tuple[List[str], List[Tuple[str, int, str]]] = (
["Hello", "world"],
@ -331,7 +340,11 @@ class TestCallSubprocess:
args, spinner = self.prepare_call(caplog, log_level, command=command)
with pytest.raises(InstallationSubprocessError) as exc:
call_subprocess(args, spinner=spinner)
call_subprocess(
args,
command_desc="test info logging with subprocess error",
spinner=spinner,
)
result = None
exc_message = str(exc.value)
assert exc_message.startswith("Command errored out with exit status 1: ")
@ -390,7 +403,12 @@ class TestCallSubprocess:
"""
log_level = INFO
args, spinner = self.prepare_call(caplog, log_level)
result = call_subprocess(args, spinner=spinner, show_stdout=True)
result = call_subprocess(
args,
command_desc="test info logging with show_stdout",
spinner=spinner,
show_stdout=True,
)
expected = (
["Hello", "world"],
@ -456,6 +474,7 @@ class TestCallSubprocess:
try:
call_subprocess(
args,
command_desc="spinner go spinny",
show_stdout=show_stdout,
extra_ok_returncodes=extra_ok_returncodes,
spinner=spinner,
@ -474,6 +493,7 @@ class TestCallSubprocess:
call_subprocess(
[sys.executable, "-c", "input()"],
show_stdout=True,
command_desc="stdin reader",
)
@ -487,6 +507,7 @@ def test_unicode_decode_error(caplog: pytest.LogCaptureFixture) -> None:
"-c",
"import sys; sys.stdout.buffer.write(b'\\xff')",
],
command_desc="invalid decode output",
show_stdout=True,
)