Skip to content
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

fix: better error printouts #442

Merged
merged 9 commits into from Mar 2, 2022
Merged
3 changes: 3 additions & 0 deletions pyproject.toml
Expand Up @@ -39,6 +39,9 @@ filterwarnings = [
]

[tool.mypy]
files = "src"
python_version = "3.6"
layday marked this conversation as resolved.
Show resolved Hide resolved
warn_unused_configs = true
strict = true

[[tool.mypy.overrides]]
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Expand Up @@ -63,7 +63,7 @@ test =
setuptools>=56.0.0;python_version >= "3.10"
typing =
importlib-metadata>=4.6.4
mypy==0.910
mypy==0.931
typing-extensions>=3.7.4.3
virtualenv =
virtualenv>=20.0.35
Expand Down
33 changes: 28 additions & 5 deletions src/build/__init__.py
Expand Up @@ -12,6 +12,7 @@
import re
import subprocess
import sys
import textwrap
import types
import warnings
import zipfile
Expand Down Expand Up @@ -85,8 +86,8 @@ def __init__(
self, exception: Exception, description: Optional[str] = None, exc_info: _ExcInfoType = (None, None, None)
) -> None:
super().__init__()
self.exception: Exception = exception
self.exc_info: _ExcInfoType = exc_info
self.exception = exception
self.exc_info = exc_info
self._description = description

def __str__(self) -> str:
Expand All @@ -104,6 +105,27 @@ def __str__(self) -> str:
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'


class FailedProcessError(Exception):
"""
Exception raised when an setup or prepration operation fails.
"""

def __init__(self, exception: subprocess.CalledProcessError, description: str) -> None:
super().__init__()
self.exception = exception
self._description = description

def __str__(self) -> str:
cmd = ' '.join(self.exception.cmd)
description = f"{self._description}\n Command '{cmd}' failed with return code {self.exception.returncode}"
for stream_name in ('stdout', 'stderr'):
stream = getattr(self.exception, stream_name)
if stream:
description += f'\n {stream_name}:\n'
description += textwrap.indent(stream.decode(), ' ')
return description


class TypoWarning(Warning):
"""
Warning raised when a potential typo is found
Expand Down Expand Up @@ -481,11 +503,12 @@ def log(message: str) -> None:

__all__ = (
'__version__',
'BuildBackendException',
'BuildException',
'ConfigSettingsType',
'FailedProcessError',
'ProjectBuilder',
'RunnerType',
'BuildException',
'BuildBackendException',
'TypoWarning',
'check_dependency',
'ProjectBuilder',
)
36 changes: 17 additions & 19 deletions src/build/__main__.py
Expand Up @@ -13,11 +13,11 @@
import traceback
import warnings

from typing import Dict, Iterable, Iterator, List, Optional, Sequence, TextIO, Type, Union
from typing import Dict, Iterator, List, NoReturn, Optional, Sequence, TextIO, Type, Union

import build

from build import BuildBackendException, BuildException, ConfigSettingsType, PathType, ProjectBuilder
from build import BuildBackendException, BuildException, ConfigSettingsType, FailedProcessError, PathType, ProjectBuilder
from build.env import IsolatedEnvBuilder


Expand Down Expand Up @@ -71,7 +71,7 @@ def _setup_cli() -> None:
colorama.init() # fix colors on windows


def _error(msg: str, code: int = 1) -> None: # pragma: no cover
def _error(msg: str, code: int = 1) -> NoReturn: # pragma: no cover
Copy link
Contributor Author

@henryiii henryiii Feb 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the function to serve as a guard below (and is correct).

"""
Print an error message and exit. Will color the output when writing to a TTY.

Expand Down Expand Up @@ -146,23 +146,24 @@ def _build(
def _handle_build_error() -> Iterator[None]:
try:
yield
except BuildException as e:
except (BuildException, FailedProcessError) as e:
_error(str(e))
except BuildBackendException as e:
if isinstance(e.exception, subprocess.CalledProcessError):
print()
_error(str(e))

if e.exc_info:
tb_lines = traceback.format_exception(
e.exc_info[0],
e.exc_info[1],
e.exc_info[2],
limit=-1,
)
tb = ''.join(tb_lines)
else:
if e.exc_info:
tb_lines = traceback.format_exception(
e.exc_info[0],
e.exc_info[1],
e.exc_info[2],
limit=-1,
)
tb = ''.join(tb_lines)
else:
tb = traceback.format_exc(-1)
print('\n{dim}{}{reset}\n'.format(tb.strip('\n'), **_STYLES))
tb = traceback.format_exc(-1)
print('\n{dim}{}{reset}\n'.format(tb.strip('\n'), **_STYLES))
_error(str(e))


Expand Down Expand Up @@ -250,9 +251,6 @@ def main_parser() -> argparse.ArgumentParser:
"""
Construct the main parser.
"""
# mypy does not recognize module.__path__
# https://github.com/python/mypy/issues/1422
paths: Iterable[str] = build.__path__ # type: ignore
parser = argparse.ArgumentParser(
description=textwrap.indent(
textwrap.dedent(
Expand Down Expand Up @@ -285,7 +283,7 @@ def main_parser() -> argparse.ArgumentParser:
'--version',
'-V',
action='version',
version=f"build {build.__version__} ({','.join(paths)})",
version=f"build {build.__version__} ({','.join(build.__path__)})",
)
parser.add_argument(
'--sdist',
Expand Down
15 changes: 10 additions & 5 deletions src/build/env.py
Expand Up @@ -74,7 +74,7 @@ def _should_use_virtualenv() -> bool:
def _subprocess(cmd: List[str]) -> None:
"""Invoke subprocess and output stdout and stderr if it fails."""
try:
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
print(e.output.decode(), end='', file=sys.stderr)
raise e
Expand Down Expand Up @@ -219,7 +219,7 @@ def install(self, requirements: Iterable[str]) -> None:

def _create_isolated_env_virtualenv(path: str) -> Tuple[str, str]:
"""
On Python 2 we use the virtualenv package to provision a virtual environment.
We optionally can use the virtualenv package to provision a virtual environment.

:param path: The path where to create the isolated build environment
:return: The Python executable and script folder
Expand All @@ -235,7 +235,7 @@ def _create_isolated_env_virtualenv(path: str) -> Tuple[str, str]:
def _fs_supports_symlink() -> bool:
"""Return True if symlinks are supported"""
# Using definition used by venv.main()
if os.name != 'nt':
if not sys.platform.startswith('win'):
return True

# Windows may support symlinks (setting in Windows 10)
Expand All @@ -260,7 +260,12 @@ def _create_isolated_env_venv(path: str) -> Tuple[str, str]:

import packaging.version

venv.EnvBuilder(with_pip=True, symlinks=_fs_supports_symlink()).create(path)
simlinks = _fs_supports_symlink()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here to make --showlocals more useful in pytest.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, should be symlinks.

try:
venv.EnvBuilder(with_pip=True, symlinks=simlinks).create(path)
except subprocess.CalledProcessError as exc:
raise build.FailedProcessError(exc, 'Failed to create venv. Maybe try installing virtualenv.') from None

executable, script_dir, purelib = _find_executable_and_scripts(path)

# Get the version of pip in the environment
Expand Down Expand Up @@ -304,7 +309,7 @@ def _find_executable_and_scripts(path: str) -> Tuple[str, str, str]:
paths = sysconfig.get_paths(scheme='posix_prefix', vars=config_vars)
else:
paths = sysconfig.get_paths(vars=config_vars)
executable = os.path.join(paths['scripts'], 'python.exe' if os.name == 'nt' else 'python')
executable = os.path.join(paths['scripts'], 'python.exe' if sys.platform.startswith('win') else 'python')
if not os.path.exists(executable):
raise RuntimeError(f'Virtual environment creation failed, executable {executable} missing')

Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Expand Up @@ -66,7 +66,7 @@ def pytest_addoption(parser):
parser.addoption('--only-integration', action='store_true', help='only run the integration tests')


PYPY3_WIN_VENV_BAD = platform.python_implementation() == 'PyPy' and os.name == 'nt'
PYPY3_WIN_VENV_BAD = platform.python_implementation() == 'PyPy' and sys.platform.startswith('win')
PYPY3_WIN_M = 'https://foss.heptapod.net/pypy/pypy/-/issues/3323 and https://foss.heptapod.net/pypy/pypy/-/issues/3321'


Expand Down
11 changes: 6 additions & 5 deletions tests/test_env.py
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: MIT
import collections
import inspect
import logging
import os
import platform
import shutil
import subprocess
Expand Down Expand Up @@ -103,8 +103,9 @@ def test_isolated_env_log(mocker, caplog, package_test_flit):
caplog.set_level(logging.DEBUG)

builder = build.env.IsolatedEnvBuilder()
frameinfo = inspect.getframeinfo(inspect.currentframe())
builder.log('something') # line number 106
with builder as env: # line number 107
with builder as env:
env.install(['something'])

assert [(record.levelname, record.message) for record in caplog.records] == [
Expand All @@ -113,7 +114,7 @@ def test_isolated_env_log(mocker, caplog, package_test_flit):
('INFO', 'Installing packages in isolated environment... (something)'),
]
if sys.version_info >= (3, 8): # stacklevel
assert [(record.lineno) for record in caplog.records] == [106, 107, 198]
assert [(record.lineno) for record in caplog.records] == [frameinfo.lineno + 1, 107, 198]


@pytest.mark.isolated
Expand Down Expand Up @@ -151,8 +152,8 @@ def test_pip_needs_upgrade_mac_os_11(mocker, pip_version, arch):


@pytest.mark.isolated
@pytest.mark.skipif(IS_PYPY3 and os.name == 'nt', reason='Isolated tests not supported on PyPy3 + Windows')
@pytest.mark.parametrize('has_symlink', [True, False] if os.name == 'nt' else [True])
@pytest.mark.skipif(IS_PYPY3 and sys.platform.startswith('win'), reason='Isolated tests not supported on PyPy3 + Windows')
@pytest.mark.parametrize('has_symlink', [True, False] if sys.platform.startswith('win') else [True])
def test_venv_symlink(mocker, has_symlink):
if has_symlink:
mocker.patch('os.symlink')
Expand Down
4 changes: 2 additions & 2 deletions tests/test_integration.py
Expand Up @@ -16,7 +16,7 @@
import build.__main__


IS_WINDOWS = os.name == 'nt'
IS_WINDOWS = sys.platform.startswith('win')
IS_PYPY3 = platform.python_implementation() == 'PyPy'


Expand Down Expand Up @@ -102,7 +102,7 @@ def test_build(monkeypatch, project, args, call, tmp_path):
monkeypatch.setenv('SETUPTOOLS_SCM_PRETEND_VERSION', '0+dummy') # for the projects that use setuptools_scm

if call and call[0] == 'pyproject-build':
exe_name = f"pyproject-build{'.exe' if os.name == 'nt' else ''}"
exe_name = f"pyproject-build{'.exe' if sys.platform.startswith('win') else ''}"
exe = os.path.join(os.path.dirname(sys.executable), exe_name)
if os.path.exists(exe):
call[0] = exe
Expand Down
30 changes: 30 additions & 0 deletions tests/test_main.py
Expand Up @@ -5,7 +5,9 @@
import io
import os
import re
import subprocess
import sys
import venv

import pytest

Expand Down Expand Up @@ -399,3 +401,31 @@ def test_colors_conflict(monkeypatch, main_reload_styles):
importlib.reload(build.__main__)

assert build.__main__._STYLES == build.__main__._NO_COLORS


def raise_called_process_err(*args, **kwargs):
raise subprocess.CalledProcessError(1, ['test', 'args'], b'stdoutput', b'stderror')


def test_venv_fail(monkeypatch, package_test_flit, tmp_dir, capsys):
monkeypatch.setattr(venv.EnvBuilder, 'create', raise_called_process_err)
monkeypatch.setenv('NO_COLOR', '')

with pytest.raises(SystemExit):
build.__main__.main([package_test_flit, '-o', tmp_dir])

stdout, stderr = capsys.readouterr()

assert (
stdout
== '''\
* Creating venv isolated environment...
ERROR Failed to create venv. Maybe try installing virtualenv.
Command 'test args' failed with return code 1
stdout:
stdoutput
stderr:
stderror
'''
)
assert stderr == ''
2 changes: 1 addition & 1 deletion tests/test_projectbuilder.py
Expand Up @@ -173,7 +173,7 @@ def test_init(mocker, package_test_flit, package_legacy, test_no_permission, pac
)

# PermissionError
if os.name != 'nt': # can't correctly set the permissions required for this
if not sys.platform.startswith('win'): # can't correctly set the permissions required for this
with pytest.raises(build.BuildException):
build.ProjectBuilder(test_no_permission)

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Expand Up @@ -52,7 +52,7 @@ commands =
description = run type check on code base
extras = typing
commands =
mypy --show-error-codes --python-version 3.6 src/build
mypy --show-error-codes src/build

[testenv:{py311, py310, py39, py38, py37, py36, pypy3}-min]
description = check minimum versions required of all dependencies
Expand Down