From a1489f3c5bbeff027ed26a76ce24a7de9cc218a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 17 Oct 2020 13:08:11 +0100 Subject: [PATCH 1/5] Refactor to simplify the env package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create the isolated build environment upfront to avoid executable, pip executable or path not yet set. Remove unused code segments. Signed-off-by: Bernát Gábor --- src/build/__main__.py | 4 +- src/build/env.py | 281 +++++++++++++++++------------------------- tests/test_env.py | 52 ++++---- 3 files changed, 145 insertions(+), 192 deletions(-) diff --git a/src/build/__main__.py b/src/build/__main__.py index 2d0aafe8..43a499fe 100644 --- a/src/build/__main__.py +++ b/src/build/__main__.py @@ -9,7 +9,7 @@ from typing import List, Optional, TextIO, Type, Union from build import BuildBackendException, BuildException, ConfigSettings, ProjectBuilder -from build.env import IsolatedEnvironment +from build.env import isolated_env __all__ = ['build', 'main', 'main_parser'] @@ -41,7 +41,7 @@ def _error(msg, code=1): # type: (str, int) -> None # pragma: no cover def _build_in_isolated_env(builder, outdir, distributions): # type: (ProjectBuilder, str, List[str]) -> None - with IsolatedEnvironment.for_current() as env: + with isolated_env() as env: env.install(builder.build_dependencies) for distribution in distributions: builder.build(distribution, outdir) diff --git a/src/build/env.py b/src/build/env.py index 760957ea..9830617f 100644 --- a/src/build/env.py +++ b/src/build/env.py @@ -1,3 +1,7 @@ +""" +Creates and manages isolated build environments. +""" +import contextlib import os import platform import shutil @@ -5,217 +9,150 @@ import sys import sysconfig import tempfile -import types -import typing -from typing import Dict, Iterable, List, Optional, Sequence, Type - -if sys.version_info[0] == 2: - FileExistsError = OSError +from typing import Generator, Iterable, Tuple try: import pip except ImportError: # pragma: no cover pip = None # pragma: no cover -_HAS_SYMLINK = None # type: Optional[bool] - - -def _fs_supports_symlink(): # type: () -> bool - if not hasattr(os, 'symlink'): - return False - - if sys.platform.startswith(('aix', 'darwin', 'freebsd', 'linux')): - return True - else: - with tempfile.NamedTemporaryFile(prefix='TmP') as tmp_file: - temp_dir = os.path.dirname(tmp_file.name) - dest = os.path.join(temp_dir, '{}-{}'.format(tmp_file.name, 'b')) - try: - os.symlink(tmp_file.name, dest) - return True - except (OSError, NotImplementedError): - return False - - -def fs_supports_symlink(): # type: () -> bool - global _HAS_SYMLINK - if _HAS_SYMLINK is None: - _HAS_SYMLINK = _fs_supports_symlink() - return _HAS_SYMLINK - -class IsolatedEnvironment(object): +@contextlib.contextmanager +def isolated_env(): # type: () -> Generator[IsolatedEnvironment, None, None] """ - Isolated build environment context manager - - Non-standard paths injected directly to sys.path still be passed to the environment. + Create an isolated build environment. """ + path = tempfile.mkdtemp(prefix='build-env-') + try: + executable, pip_executable = _create_isolated_env(path) + env = IsolatedEnvironment(path, executable, pip_executable) + with _patch_sys_executable(executable): + yield env + finally: + if os.path.exists(path): # in case the user already deleted skip remove + shutil.rmtree(path) + + +@contextlib.contextmanager +def _patch_sys_executable(executable): # type: (str) -> Generator[None, None, None] + """ + Address https://github.com/pypa/pep517/pull/93, that always uses sys.executable to run pep517 hooks in. - _MANIPULATE_PATHS = ('purelib', 'platlib') - - def __init__(self, remove_paths, _executable=sys.executable): # type: (Sequence[str], str) -> None - """ - :param remove_paths: Import paths that should be removed from the environment - """ - self._env = {} # type: Dict[str, Optional[str]] - self._env_vars = {} # type: Dict[str, str] - self._path = None # type: Optional[str] - self._remove_paths = [] # type: List[str] - self._executable = _executable - self._old_executable = None # type: Optional[str] - self._pip_executable = None # type: Optional[str] - - # normalize paths so that we can compare them -- required on case insensitive systems - for path in remove_paths: - self._remove_paths.append(os.path.normcase(path)) + :param executable: the new sys.executable + """ - @property - def path(self): # type: () -> str - if not self._path: - raise RuntimeError("{} context environment hasn't been entered yet".format(self.__class__.__name__)) + old_executable = sys.executable + sys.executable = executable + try: + yield + finally: + sys.executable = old_executable - return self._path - @property - def executable(self): # type: () -> str - return self._executable +if sys.version_info[0] == 2: # noqa: C901 # disable if too complex - @classmethod - def for_current(cls): # type: () -> IsolatedEnvironment - """ - Creates an isolated environment for the current interpreter + def _create_isolated_env(path): # type: (str) -> Tuple[str, str] """ - remove_paths = os.environ.get('PYTHONPATH', '').split(os.pathsep) - # also remove the pth file adding this project (when installed via --develop install) - for path in sys.path: - if os.path.exists(os.path.join(path, 'build.egg-info')): - remove_paths.append(path) # pragma: no cover + On Python 2 we use the virtualenv package to provision a virtual environment. - return cls(remove_paths) - - def _replace_env(self, key, new): # type: (str, Optional[str]) -> None - """ - Replaces an environment variable + :param path: the folder where to create the isolated build environment + :return: the isolated build environment executable, and the pip to use to install packages into it """ - if not new: # pragma: no cover - return + from virtualenv import cli_run - self._env[key] = os.environ.get(key, None) - os.environ[key] = new + cmd = [str(path), '--no-setuptools', '--no-wheel', '--activators', ''] + if pip is not None: + cmd.append('--no-pip') + result = cli_run(cmd, setup_logging=False) + executable = str(result.creator.exe) + pip_executable = executable if pip is None else sys.executable + return executable, pip_executable - def _pop_env(self, key): # type: (str) -> None - """ - Removes an environment variable - """ - self._env[key] = os.environ.pop(key, None) - def _restore_env(self): # type: () -> None - """ - Restores the initial environment variables - """ - for key, val in self._env.items(): - if val is None: - os.environ.pop(key, None) - else: - os.environ[key] = val +else: - def _get_env_path(self, path): # type: (str) -> Optional[str] - """ - Returns sysconfig path from our environment + def _create_isolated_env(path): # type: (str) -> Tuple[str, str] """ - return sysconfig.get_path(path, vars=self._env_vars) + On Python 3 we use the venv package from the standard library, and if host python has no pip the ensurepip + package to provision one into the created virtual environment. - def _create_env_venv(self): # type: () -> None + :param path: the folder where to create the isolated build environment + :return: the isolated build environment executable, and the pip to use to install packages into it + """ import venv - venv.EnvBuilder(with_pip=False).create(self.path) - - env_scripts = self._get_env_path('scripts') - if not env_scripts: - raise RuntimeError("Couldn't get environment scripts path") - exe = 'pypy3' if platform.python_implementation() == 'PyPy' else 'python' - if os.name == 'nt': - exe = '{}.exe'.format(exe) - - self._executable = os.path.join(self.path, env_scripts, exe) - if not os.path.exists(self._executable): - raise RuntimeError('Virtual environment creation failed, executable {} missing'.format(self._executable)) + venv.EnvBuilder(with_pip=False).create(path) + executable = _find_executable(path) # Scenario 1: pip is available (either installed or via pth file) within the python executable alongside # this projects environment: in this case we should be able to import it if pip is not None: - self._pip_executable = sys.executable - return - # Scenario 2: this project is installed into a virtual environment that has no pip, but the matching system has - # Scenario 3: there's a pip executable on PATH - # Scenario 4: no pip can be found, we might be able to provision one into the isolated build env via ensurepip - cmd = [self.executable, '-Im', 'ensurepip', '--upgrade', '--default-pip'] - try: - subprocess.check_call(cmd, cwd=self.path) - except subprocess.CalledProcessError: # pragma: no cover - pass # pragma: no cover - # avoid the setuptools from ensurepip to break the isolation - subprocess.check_call([self.executable, '-Im', 'pip', 'uninstall', 'setuptools', '-y']) - self._pip_executable = self.executable - - def _create_env_virtualenv(self): # type: () -> None - from virtualenv import cli_run - - cmd = [str(self.path), '--no-setuptools', '--no-wheel', '--activators', ''] - if pip is not None: - cmd.append('--no-pip') - result = cli_run(cmd, setup_logging=False) - self._executable = str(result.creator.exe) - self._pip_executable = self._executable - if pip is not None: - self._pip_executable = sys.executable - - def _create_isolated_env(self): # type: () -> None - if sys.version_info[0] == 2: - self._create_env_virtualenv() - + pip_executable = sys.executable else: - self._create_env_venv() + # Scenario 2: this project is installed into a virtual environment that has no pip, but the system has + # Scenario 3: there's a pip executable on PATH + # Scenario 4: no pip can be found, we might be able to provision one into the build env via ensurepip + cmd = [executable, '-Im', 'ensurepip', '--upgrade', '--default-pip'] + try: + subprocess.check_call(cmd, cwd=path) + except subprocess.CalledProcessError: # pragma: no cover + pass # pragma: no cover + # avoid the setuptools from ensurepip to break the isolation + subprocess.check_call([executable, '-Im', 'pip', 'uninstall', 'setuptools', '-y']) + pip_executable = executable + return executable, pip_executable - def __enter__(self): # type: () -> IsolatedEnvironment + def _find_executable(path): # type: (str) -> str """ - Set up the environment + Detect the executable within a virtual environment. - The environment path should be empty + :param path: the location of the virtual environment + :return: the python executable """ - self._path = tempfile.mkdtemp(prefix='build-env-') - self._env_vars = { - 'base': self.path, - 'platbase': self.path, - } + config_vars = sysconfig.get_config_vars() + config_vars['base'] = path + env_scripts = sysconfig.get_path('scripts', vars=config_vars) + if not env_scripts: + raise RuntimeError("Couldn't get environment scripts path") + exe = 'pypy3' if platform.python_implementation() == 'PyPy' else 'python' + if os.name == 'nt': + exe = '{}.exe'.format(exe) + executable = os.path.join(path, env_scripts, exe) + if not os.path.exists(executable): + raise RuntimeError('Virtual environment creation failed, executable {} missing'.format(executable)) + return executable - self._create_isolated_env() - self._pop_env('PIP_REQUIRE_VIRTUALENV') - # address https://github.com/pypa/pep517/pull/93 - self._old_executable, sys.executable = sys.executable, self._executable +class IsolatedEnvironment(object): + """ + Isolated build environment context manager - return self + Non-standard paths injected directly to sys.path still be passed to the environment. + """ - def __exit__(self, typ, value, traceback): - # type: (Optional[Type[BaseException]], Optional[BaseException], Optional[types.TracebackType]) -> None - """ - Restores the everything to the original state - """ - if self._old_executable is not None: - sys.executable = self._old_executable - if self.path and os.path.isdir(self.path): - shutil.rmtree(self.path) + def __init__(self, path, executable, pip_executable): # type: (str, str, str) -> None + self._path = path + self._pip_executable = pip_executable + self._executable = executable - self._restore_env() + @property + def path(self): # type: () -> str + """:return: the location of the isolated build environment""" + return self._path + + @property + def executable(self): # type: () -> str + """:return: the python executable of the isolated build environment""" + return self._executable def install(self, requirements): # type: (Iterable[str]) -> None """ Installs the specified PEP 508 requirements on the environment - Passing non PEP 508 strings will result in undefined behavior, you - *should not* rely on it. It is merely an implementation detail, it may - change any time without warning. + :param requirements: PEP-508 requirement specification to install + + :note: Passing non PEP 508 strings will result in undefined behavior, you *should not* rely on it. It is \ + merely an implementation detail, it may change any time without warning. """ if not requirements: return @@ -224,10 +161,10 @@ def install(self, requirements): # type: (Iterable[str]) -> None req_file.write(os.linesep.join(requirements)) req_file.close() cmd = [ - typing.cast(str, self._pip_executable), + self._pip_executable, # on python2 if isolation is achieved via environment variables, we need to ignore those while calling # host python (otherwise pip would not be available within it) - '-{}m'.format('E' if self._pip_executable == self._executable and sys.version_info[0] == 2 else ''), + '-{}m'.format('E' if self._pip_executable == self.executable and sys.version_info[0] == 2 else ''), 'pip', 'install', '--prefix', @@ -239,3 +176,9 @@ def install(self, requirements): # type: (Iterable[str]) -> None ] subprocess.check_call(cmd) os.unlink(req_file.name) + + +__all__ = ( + 'IsolatedEnvironment', + 'isolated_env', +) diff --git a/tests/test_env.py b/tests/test_env.py index 969eff47..4056dfb0 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -1,8 +1,7 @@ # SPDX-License-Identifier: MIT import json -import os -import os.path import platform +import shutil import subprocess import sys @@ -14,23 +13,15 @@ @pytest.mark.isolated def test_isolation(): subprocess.check_call([sys.executable, '-c', 'import build.env']) - with build.env.IsolatedEnvironment.for_current() as env: + with build.env.isolated_env() as env: with pytest.raises(subprocess.CalledProcessError): debug = 'import sys; import os; print(os.linesep.join(sys.path));' subprocess.check_call([env.executable, '-c', '{} import build.env'.format(debug)]) -@pytest.mark.isolated -def test_isolated_environment_setup_require_virtualenv(mocker): - mocker.patch.dict(os.environ, {'PIP_REQUIRE_VIRTUALENV': 'true'}) - with build.env.IsolatedEnvironment.for_current(): - assert 'PIP_REQUIRE_VIRTUALENV' not in os.environ - assert os.environ['PIP_REQUIRE_VIRTUALENV'] == 'true' - - @pytest.mark.isolated def test_isolated_environment_install(mocker): - with build.env.IsolatedEnvironment.for_current() as env: + with build.env.isolated_env() as env: mocker.patch('subprocess.check_call') env.install([]) @@ -53,19 +44,12 @@ def test_isolated_environment_install(mocker): ] -def test_uninitialised_isolated_environment(): - env = build.env.IsolatedEnvironment.for_current() - - with pytest.raises(RuntimeError): - env.path - - @pytest.mark.isolated def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): mocker.patch.object(build.env, 'pip', None) expected = {'pip', 'greenlet', 'readline', 'cffi'} if platform.python_implementation() == 'PyPy' else {'pip'} - with build.env.IsolatedEnvironment.for_current() as isolated_env: + with build.env.isolated_env() as isolated_env: cmd = [isolated_env.executable, '-m', 'pip', 'list', '--format', 'json'] packages = {p['name'] for p in json.loads(subprocess.check_output(cmd, universal_newlines=True))} assert packages == expected @@ -80,9 +64,35 @@ def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): @pytest.mark.isolated def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): - with build.env.IsolatedEnvironment.for_current() as isolated_env: + with build.env.isolated_env() as isolated_env: pass assert isolated_env._pip_executable == sys.executable out, err = capfd.readouterr() assert not out assert not err + + +@pytest.mark.skipif(sys.version_info[0] == 2, reason='venv module used on Python 3 only') +def test_fail_to_get_script_path(mocker): + get_path = mocker.patch('sysconfig.get_path', return_value=None) + with pytest.raises(RuntimeError, match="Couldn't get environment scripts path"): + with build.env.isolated_env(): + pass + assert get_path.call_count == 1 + + +@pytest.mark.skipif(sys.version_info[0] == 2, reason='venv module used on Python 3 only') +def test_executable_missing_post_creation(mocker): + import sysconfig + + original_get_path = sysconfig.get_path + + def _get_path(name, vars): # noqa + shutil.rmtree(vars['base']) + return original_get_path(name, vars=vars) + + get_path = mocker.patch('sysconfig.get_path', side_effect=_get_path) + with pytest.raises(RuntimeError, match='Virtual environment creation failed, executable .* missing'): + with build.env.isolated_env(): + pass + assert get_path.call_count == 1 From 6404e054a3bbec46f53183c83338c6c175e175f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Mon, 19 Oct 2020 06:26:35 +0100 Subject: [PATCH 2/5] PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- setup.cfg | 2 +- src/build/__init__.py | 8 +- src/build/__main__.py | 10 +- src/build/env.py | 173 ++++++++++++++++++----------------- tests/test_env.py | 20 ++-- tests/test_main.py | 59 +++++++----- tests/test_projectbuilder.py | 10 +- 7 files changed, 153 insertions(+), 129 deletions(-) diff --git a/setup.cfg b/setup.cfg index 7a4c89c5..0a7c20aa 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,7 +26,7 @@ project_urls = packages = find: install_requires = packaging - pep517 + pep517>=0.9.1 toml importlib-metadata;python_version < "3.8" typing;python_version<"3" diff --git a/src/build/__init__.py b/src/build/__init__.py index 885a926c..6d546112 100644 --- a/src/build/__init__.py +++ b/src/build/__init__.py @@ -10,14 +10,12 @@ import os import sys import warnings - from typing import Dict, Iterator, List, Mapping, Optional, Set, Union import pep517.wrappers import toml import toml.decoder - if sys.version_info < (3,): FileNotFoundError = IOError PermissionError = OSError @@ -118,7 +116,8 @@ def _working_directory(path): # type: (str) -> Iterator[None] class ProjectBuilder(object): - def __init__(self, srcdir='.', config_settings=None): # type: (str, Optional[ConfigSettings]) -> None + def __init__(self, srcdir='.', config_settings=None, executable=sys.executable): + # type: (str, Optional[ConfigSettings], str) -> None """ :param srcdir: Source directory """ @@ -152,7 +151,7 @@ def __init__(self, srcdir='.', config_settings=None): # type: (str, Optional[Co self._backend = self._build_system['build-backend'] self.hook = pep517.wrappers.Pep517HookCaller( - self.srcdir, self._backend, backend_path=self._build_system.get('backend-path') + self.srcdir, self._backend, backend_path=self._build_system.get('backend-path'), python_executable=executable ) @property @@ -190,6 +189,7 @@ def build(self, distribution, outdir): # type: (str, str) -> None :param distribution: Distribution to build (sdist or wheel) :param outdir: Output directory + :param executable: executable """ build = getattr(self.hook, 'build_{}'.format(distribution)) outdir = os.path.abspath(outdir) diff --git a/src/build/__main__.py b/src/build/__main__.py index 43a499fe..b8908751 100644 --- a/src/build/__main__.py +++ b/src/build/__main__.py @@ -9,8 +9,7 @@ from typing import List, Optional, TextIO, Type, Union from build import BuildBackendException, BuildException, ConfigSettings, ProjectBuilder -from build.env import isolated_env - +from build.env import IsolatedEnvironment __all__ = ['build', 'main', 'main_parser'] @@ -40,8 +39,10 @@ def _error(msg, code=1): # type: (str, int) -> None # pragma: no cover exit(code) -def _build_in_isolated_env(builder, outdir, distributions): # type: (ProjectBuilder, str, List[str]) -> None - with isolated_env() as env: +def _build_in_isolated_env(builder, outdir, distributions): + # type: (ProjectBuilder, str, List[str]) -> None + with IsolatedEnvironment.for_current() as env: + builder.hook.python_executable = env.executable env.install(builder.build_dependencies) for distribution in distributions: builder.build(distribution, outdir) @@ -75,7 +76,6 @@ def build(srcdir, outdir, distributions, config_settings=None, isolation=True, s try: builder = ProjectBuilder(srcdir, config_settings) - if isolation: _build_in_isolated_env(builder, outdir, distributions) else: diff --git a/src/build/env.py b/src/build/env.py index 9830617f..e3dc744a 100644 --- a/src/build/env.py +++ b/src/build/env.py @@ -1,7 +1,6 @@ """ Creates and manages isolated build environments. """ -import contextlib import os import platform import shutil @@ -9,7 +8,8 @@ import sys import sysconfig import tempfile -from typing import Generator, Iterable, Tuple +from types import TracebackType +from typing import Iterable, Optional, Tuple, Type try: import pip @@ -17,36 +17,99 @@ pip = None # pragma: no cover -@contextlib.contextmanager -def isolated_env(): # type: () -> Generator[IsolatedEnvironment, None, None] +class IsolatedEnvironment(object): """ - Create an isolated build environment. + Isolated build environment context manager + + Non-standard paths injected directly to sys.path still be passed to the environment. """ - path = tempfile.mkdtemp(prefix='build-env-') - try: + + def __init__(self, path, executable, install_executable): + # type: (str, str, str) -> None + """ + Define an isolated build environment. + + :param path: the path where the environment exists + :param executable: the python executable within the environment + :param install_executable: an executable that allows installing packages within the environment + """ + self._path = path + self._install_executable = install_executable + self._executable = executable + + @classmethod + def for_current(cls): # type: () -> IsolatedEnvironment + """ + Create an isolated build environment into a temporary folder that matches the current interpreter. + + :return: the isolated build environment + """ + path = tempfile.mkdtemp(prefix='build-env-') executable, pip_executable = _create_isolated_env(path) - env = IsolatedEnvironment(path, executable, pip_executable) - with _patch_sys_executable(executable): - yield env - finally: - if os.path.exists(path): # in case the user already deleted skip remove - shutil.rmtree(path) + return cls(path=path, executable=executable, install_executable=pip_executable) + def __enter__(self): # type: () -> IsolatedEnvironment + """Enable the isolated build environment""" + return self -@contextlib.contextmanager -def _patch_sys_executable(executable): # type: (str) -> Generator[None, None, None] - """ - Address https://github.com/pypa/pep517/pull/93, that always uses sys.executable to run pep517 hooks in. + def __exit__(self, exc_type, exc_val, exc_tb): + # type: (Optional[Type[BaseException]], Optional[BaseException], Optional[TracebackType]) -> None + """ + Exit from the isolated build environment. - :param executable: the new sys.executable - """ + :param exc_type: the type of exception raised (if any) + :param exc_val: the value of exception raised (if any) + :param exc_tb: the traceback of exception raised (if any) + """ + self.close() + + def close(self): # type: () -> None + """Close the isolated cleanup environment.""" + if os.path.exists(self._path): # in case the user already deleted skip remove + shutil.rmtree(self._path) + + @property + def path(self): # type: () -> str + """:return: the location of the isolated build environment""" + return self._path + + @property + def executable(self): # type: () -> str + """:return: the python executable of the isolated build environment""" + return self._executable - old_executable = sys.executable - sys.executable = executable - try: - yield - finally: - sys.executable = old_executable + def install(self, requirements): # type: (Iterable[str]) -> None + """ + Installs the specified PEP 508 requirements on the environment + + :param requirements: PEP-508 requirement specification to install + + :note: Passing non PEP 508 strings will result in undefined behavior, you *should not* rely on it. It is \ + merely an implementation detail, it may change any time without warning. + """ + if not requirements: + return + + with tempfile.NamedTemporaryFile('w+', prefix='build-reqs-', suffix='.txt', delete=False) as req_file: + req_file.write(os.linesep.join(requirements)) + try: + cmd = [ + self._install_executable, + # on python2 if isolation is achieved via environment variables, we need to ignore those while calling + # host python (otherwise pip would not be available within it) + '-{}m'.format('E' if self._install_executable == self.executable and sys.version_info[0] == 2 else ''), + 'pip', + 'install', + '--prefix', + self.path, + '--ignore-installed', + '--no-warn-script-location', + '-r', + os.path.abspath(req_file.name), + ] + subprocess.check_call(cmd) + finally: + os.unlink(req_file.name) if sys.version_info[0] == 2: # noqa: C901 # disable if too complex @@ -123,62 +186,4 @@ def _find_executable(path): # type: (str) -> str return executable -class IsolatedEnvironment(object): - """ - Isolated build environment context manager - - Non-standard paths injected directly to sys.path still be passed to the environment. - """ - - def __init__(self, path, executable, pip_executable): # type: (str, str, str) -> None - self._path = path - self._pip_executable = pip_executable - self._executable = executable - - @property - def path(self): # type: () -> str - """:return: the location of the isolated build environment""" - return self._path - - @property - def executable(self): # type: () -> str - """:return: the python executable of the isolated build environment""" - return self._executable - - def install(self, requirements): # type: (Iterable[str]) -> None - """ - Installs the specified PEP 508 requirements on the environment - - :param requirements: PEP-508 requirement specification to install - - :note: Passing non PEP 508 strings will result in undefined behavior, you *should not* rely on it. It is \ - merely an implementation detail, it may change any time without warning. - """ - if not requirements: - return - - with tempfile.NamedTemporaryFile('w+', prefix='build-reqs-', suffix='.txt', delete=False) as req_file: - req_file.write(os.linesep.join(requirements)) - req_file.close() - cmd = [ - self._pip_executable, - # on python2 if isolation is achieved via environment variables, we need to ignore those while calling - # host python (otherwise pip would not be available within it) - '-{}m'.format('E' if self._pip_executable == self.executable and sys.version_info[0] == 2 else ''), - 'pip', - 'install', - '--prefix', - self.path, - '--ignore-installed', - '--no-warn-script-location', - '-r', - os.path.abspath(req_file.name), - ] - subprocess.check_call(cmd) - os.unlink(req_file.name) - - -__all__ = ( - 'IsolatedEnvironment', - 'isolated_env', -) +__all__ = ('IsolatedEnvironment',) diff --git a/tests/test_env.py b/tests/test_env.py index 4056dfb0..542368f5 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -13,7 +13,7 @@ @pytest.mark.isolated def test_isolation(): subprocess.check_call([sys.executable, '-c', 'import build.env']) - with build.env.isolated_env() as env: + with build.env.IsolatedEnvironment.for_current() as env: with pytest.raises(subprocess.CalledProcessError): debug = 'import sys; import os; print(os.linesep.join(sys.path));' subprocess.check_call([env.executable, '-c', '{} import build.env'.format(debug)]) @@ -21,7 +21,7 @@ def test_isolation(): @pytest.mark.isolated def test_isolated_environment_install(mocker): - with build.env.isolated_env() as env: + with build.env.IsolatedEnvironment.for_current() as env: mocker.patch('subprocess.check_call') env.install([]) @@ -32,8 +32,8 @@ def test_isolated_environment_install(mocker): subprocess.check_call.assert_called() args = subprocess.check_call.call_args[0][0][:-1] assert args == [ - env._pip_executable, - '-{}m'.format('E' if env._pip_executable == env._executable and sys.version_info[0] == 2 else ''), + env._install_executable, + '-{}m'.format('E' if env._install_executable == env._executable and sys.version_info[0] == 2 else ''), 'pip', 'install', '--prefix', @@ -49,11 +49,11 @@ def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): mocker.patch.object(build.env, 'pip', None) expected = {'pip', 'greenlet', 'readline', 'cffi'} if platform.python_implementation() == 'PyPy' else {'pip'} - with build.env.isolated_env() as isolated_env: + with build.env.IsolatedEnvironment.for_current() as isolated_env: cmd = [isolated_env.executable, '-m', 'pip', 'list', '--format', 'json'] packages = {p['name'] for p in json.loads(subprocess.check_output(cmd, universal_newlines=True))} assert packages == expected - assert isolated_env._pip_executable == isolated_env.executable + assert isolated_env._install_executable == isolated_env.executable out, err = capfd.readouterr() if sys.version_info[0] == 3: assert out # ensurepip prints onto the stdout @@ -64,9 +64,9 @@ def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): @pytest.mark.isolated def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): - with build.env.isolated_env() as isolated_env: + with build.env.IsolatedEnvironment.for_current() as isolated_env: pass - assert isolated_env._pip_executable == sys.executable + assert isolated_env._install_executable == sys.executable out, err = capfd.readouterr() assert not out assert not err @@ -76,7 +76,7 @@ def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): def test_fail_to_get_script_path(mocker): get_path = mocker.patch('sysconfig.get_path', return_value=None) with pytest.raises(RuntimeError, match="Couldn't get environment scripts path"): - with build.env.isolated_env(): + with build.env.IsolatedEnvironment.for_current(): pass assert get_path.call_count == 1 @@ -93,6 +93,6 @@ def _get_path(name, vars): # noqa get_path = mocker.patch('sysconfig.get_path', side_effect=_get_path) with pytest.raises(RuntimeError, match='Virtual environment creation failed, executable .* missing'): - with build.env.isolated_env(): + with build.env.IsolatedEnvironment.for_current(): pass assert get_path.call_count == 1 diff --git a/tests/test_main.py b/tests/test_main.py index bc93a51a..af4d2e07 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -87,39 +87,56 @@ def test_prog(): @pytest.mark.isolated -def test_build(mocker, test_flit_path): - mocker.patch('importlib.import_module', autospec=True) - mocker.patch('build.ProjectBuilder.check_dependencies') - mocker.patch('build.ProjectBuilder.build') +def test_build_isolated(mocker, test_flit_path): + build_cmd = mocker.patch('build.ProjectBuilder.build') mocker.patch('build.__main__._error') - mocker.patch('build.env.IsolatedEnvironment.install') - - build.ProjectBuilder.check_dependencies.side_effect = [[], ['something'], [], []] - build.env.IsolatedEnvironment._path = mocker.Mock() + install = mocker.patch('build.env.IsolatedEnvironment.install') - # isolation=True build.__main__.build(test_flit_path, '.', ['sdist']) - build.ProjectBuilder.build.assert_called_with('sdist', '.') + build_cmd.assert_called_with('sdist', '.') + install.assert_called_with({'flit_core >=2,<3'}) + + +def test_build_no_isolation_check_deps_empty(mocker, test_flit_path): # check_dependencies = [] + build_cmd = mocker.patch('build.ProjectBuilder.build') + mocker.patch('build.ProjectBuilder.check_dependencies', return_value=[]) + build.__main__.build(test_flit_path, '.', ['sdist'], isolation=False) - build.ProjectBuilder.build.assert_called_with('sdist', '.') - build.env.IsolatedEnvironment.install.assert_called_with({'flit_core >=2,<3'}) + build_cmd.assert_called_with('sdist', '.') + + +def test_build_no_isolation_with_check_deps(mocker, test_flit_path): # check_dependencies = ['something'] + error = mocker.patch('build.__main__._error') + build_cmd = mocker.patch('build.ProjectBuilder.build') + mocker.patch('build.ProjectBuilder.check_dependencies', return_value=['something']) + build.__main__.build(test_flit_path, '.', ['sdist'], isolation=False) - build.ProjectBuilder.build.assert_called_with('sdist', '.') - build.__main__._error.assert_called_with('Missing dependencies:\n\tsomething') - build.ProjectBuilder.build.side_effect = [build.BuildException, build.BuildBackendException] - build.__main__._error.reset_mock() + build_cmd.assert_called_with('sdist', '.') + error.assert_called_with('Missing dependencies:\n\tsomething') + + +@pytest.mark.isolated +def test_build_raises_build_exception(mocker, test_flit_path): + error = mocker.patch('build.__main__._error') + mocker.patch('build.ProjectBuilder.build', side_effect=build.BuildException) + mocker.patch('build.env.IsolatedEnvironment.install') - # BuildException build.__main__.build(test_flit_path, '.', ['sdist']) - build.__main__._error.assert_called_with('') - build.__main__._error.reset_mock() + error.assert_called_with('') + + +@pytest.mark.isolated +def test_build_raises_build_backend_exception(mocker, test_flit_path): + error = mocker.patch('build.__main__._error') + mocker.patch('build.ProjectBuilder.build', side_effect=build.BuildBackendException) + mocker.patch('build.env.IsolatedEnvironment.install') - # BuildBackendException build.__main__.build(test_flit_path, '.', ['sdist']) - build.__main__._error.assert_called_with('') + + error.assert_called_with('') diff --git a/tests/test_projectbuilder.py b/tests/test_projectbuilder.py index 640c25d0..0c0a0363 100644 --- a/tests/test_projectbuilder.py +++ b/tests/test_projectbuilder.py @@ -88,16 +88,18 @@ def test_init(mocker, test_flit_path, legacy_path, test_no_permission, test_bad_ 'setuptools.build_meta:__legacy__': None, } mocker.patch('importlib.import_module', modules.get) - mocker.patch('pep517.wrappers.Pep517HookCaller') + caller = mocker.patch('pep517.wrappers.Pep517HookCaller') # correct flit pyproject.toml build.ProjectBuilder(test_flit_path) - pep517.wrappers.Pep517HookCaller.assert_called_with(test_flit_path, 'flit_core.buildapi', backend_path=None) - pep517.wrappers.Pep517HookCaller.reset_mock() + caller.assert_called_with(test_flit_path, 'flit_core.buildapi', backend_path=None, python_executable=sys.executable) + caller.reset_mock() # FileNotFoundError build.ProjectBuilder(legacy_path) - pep517.wrappers.Pep517HookCaller.assert_called_with(legacy_path, 'setuptools.build_meta:__legacy__', backend_path=None) + caller.assert_called_with( + legacy_path, 'setuptools.build_meta:__legacy__', backend_path=None, python_executable=sys.executable + ) # PermissionError if sys.version_info[0] != 2 and os.name != 'nt': # can't correctly set the permissions required for this From d444d0cd7dd6c0ebbfac5016e89a1d60262fa26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Mon, 19 Oct 2020 14:00:17 +0100 Subject: [PATCH 3/5] executable -> python_executable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- src/build/env.py | 10 +++++----- tests/test_env.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/build/env.py b/src/build/env.py index e3dc744a..7a3f3798 100644 --- a/src/build/env.py +++ b/src/build/env.py @@ -24,18 +24,18 @@ class IsolatedEnvironment(object): Non-standard paths injected directly to sys.path still be passed to the environment. """ - def __init__(self, path, executable, install_executable): + def __init__(self, path, python_executable, install_executable): # type: (str, str, str) -> None """ Define an isolated build environment. :param path: the path where the environment exists - :param executable: the python executable within the environment + :param python_executable: the python executable within the environment :param install_executable: an executable that allows installing packages within the environment """ self._path = path self._install_executable = install_executable - self._executable = executable + self._python_executable = python_executable @classmethod def for_current(cls): # type: () -> IsolatedEnvironment @@ -46,7 +46,7 @@ def for_current(cls): # type: () -> IsolatedEnvironment """ path = tempfile.mkdtemp(prefix='build-env-') executable, pip_executable = _create_isolated_env(path) - return cls(path=path, executable=executable, install_executable=pip_executable) + return cls(path=path, python_executable=executable, install_executable=pip_executable) def __enter__(self): # type: () -> IsolatedEnvironment """Enable the isolated build environment""" @@ -76,7 +76,7 @@ def path(self): # type: () -> str @property def executable(self): # type: () -> str """:return: the python executable of the isolated build environment""" - return self._executable + return self._python_executable def install(self, requirements): # type: (Iterable[str]) -> None """ diff --git a/tests/test_env.py b/tests/test_env.py index 542368f5..a0debb1d 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -33,7 +33,7 @@ def test_isolated_environment_install(mocker): args = subprocess.check_call.call_args[0][0][:-1] assert args == [ env._install_executable, - '-{}m'.format('E' if env._install_executable == env._executable and sys.version_info[0] == 2 else ''), + '-{}m'.format('E' if env._install_executable == env._python_executable and sys.version_info[0] == 2 else ''), 'pip', 'install', '--prefix', From 0641c0f41208dd86a6fb67ef57e603543a876faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 20 Oct 2020 08:15:50 +0100 Subject: [PATCH 4/5] More PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- setup.cfg | 6 +++ src/build/__init__.py | 16 +++++--- src/build/__main__.py | 4 +- src/build/env.py | 93 ++++++++++++++++++++++++++++--------------- tests/test_env.py | 20 +++++----- tests/test_main.py | 6 +-- 6 files changed, 92 insertions(+), 53 deletions(-) diff --git a/setup.cfg b/setup.cfg index 0a7c20aa..71d17c44 100644 --- a/setup.cfg +++ b/setup.cfg @@ -78,6 +78,12 @@ omit = *bin/pyproject-build *bin\pyproject-build.exe +[coverage:report] +exclude_lines = + \#\s*pragma: no cover + ^\s*raise NotImplementedError\b + ^if __name__ == ['"]__main__['"]:$ + [coverage:paths] source = src diff --git a/src/build/__init__.py b/src/build/__init__.py index 6d546112..bfa8e782 100644 --- a/src/build/__init__.py +++ b/src/build/__init__.py @@ -116,11 +116,15 @@ def _working_directory(path): # type: (str) -> Iterator[None] class ProjectBuilder(object): - def __init__(self, srcdir='.', config_settings=None, executable=sys.executable): - # type: (str, Optional[ConfigSettings], str) -> None + def __init__(self, srcdir='.', config_settings=None, python_executable=sys.executable): """ - :param srcdir: Source directory + Create a project builder. + + :param srcdir: the source directory + :param config_settings: config settings for the build backend + :param python_executable: the python executable where the backend lives """ + # type: (str, Optional[ConfigSettings], str) -> None self.srcdir = os.path.abspath(srcdir) self.config_settings = config_settings if config_settings else {} @@ -151,7 +155,10 @@ def __init__(self, srcdir='.', config_settings=None, executable=sys.executable): self._backend = self._build_system['build-backend'] self.hook = pep517.wrappers.Pep517HookCaller( - self.srcdir, self._backend, backend_path=self._build_system.get('backend-path'), python_executable=executable + self.srcdir, + self._backend, + backend_path=self._build_system.get('backend-path'), + python_executable=python_executable, ) @property @@ -189,7 +196,6 @@ def build(self, distribution, outdir): # type: (str, str) -> None :param distribution: Distribution to build (sdist or wheel) :param outdir: Output directory - :param executable: executable """ build = getattr(self.hook, 'build_{}'.format(distribution)) outdir = os.path.abspath(outdir) diff --git a/src/build/__main__.py b/src/build/__main__.py index b8908751..a353f9da 100644 --- a/src/build/__main__.py +++ b/src/build/__main__.py @@ -9,7 +9,7 @@ from typing import List, Optional, TextIO, Type, Union from build import BuildBackendException, BuildException, ConfigSettings, ProjectBuilder -from build.env import IsolatedEnvironment +from build.env import IsolatedEnvBuilder __all__ = ['build', 'main', 'main_parser'] @@ -41,7 +41,7 @@ def _error(msg, code=1): # type: (str, int) -> None # pragma: no cover def _build_in_isolated_env(builder, outdir, distributions): # type: (ProjectBuilder, str, List[str]) -> None - with IsolatedEnvironment.for_current() as env: + with IsolatedEnvBuilder() as env: builder.hook.python_executable = env.executable env.install(builder.build_dependencies) for distribution in distributions: diff --git a/src/build/env.py b/src/build/env.py index 7a3f3798..9d5bc805 100644 --- a/src/build/env.py +++ b/src/build/env.py @@ -1,6 +1,7 @@ """ Creates and manages isolated build environments. """ +import abc import os import platform import shutil @@ -17,57 +18,80 @@ pip = None # pragma: no cover -class IsolatedEnvironment(object): - """ - Isolated build environment context manager +ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()}) # Python 2/3 compatible ABC - Non-standard paths injected directly to sys.path still be passed to the environment. - """ - def __init__(self, path, python_executable, install_executable): - # type: (str, str, str) -> None +class IsolatedEnvironment(ABC): + """Abstract base of isolated build environments, as required by the build project.""" + + @property + @abc.abstractmethod + def executable(self): # type: () -> str + """Return the executable of the isolated build environment.""" + raise NotImplementedError + + @abc.abstractmethod + def install(self, requirements): # type: (Iterable[str]) -> None """ - Define an isolated build environment. + Install PEP-508 requirements into the isolated build environment. - :param path: the path where the environment exists - :param python_executable: the python executable within the environment - :param install_executable: an executable that allows installing packages within the environment + :param requirements: PEP-508 requirements """ - self._path = path - self._install_executable = install_executable - self._python_executable = python_executable + raise NotImplementedError - @classmethod - def for_current(cls): # type: () -> IsolatedEnvironment + +class IsolatedEnvBuilder(object): + def __init__(self): + """Builder object for isolated environment.""" + self._path = None # type: Optional[str] + + def __enter__(self): # type: () -> IsolatedEnvironment """ - Create an isolated build environment into a temporary folder that matches the current interpreter. + Creates an isolated build environment. :return: the isolated build environment """ - path = tempfile.mkdtemp(prefix='build-env-') - executable, pip_executable = _create_isolated_env(path) - return cls(path=path, python_executable=executable, install_executable=pip_executable) - - def __enter__(self): # type: () -> IsolatedEnvironment - """Enable the isolated build environment""" - return self + self._path = tempfile.mkdtemp(prefix='build-env-') + try: + executable, pip_executable = _create_isolated_env(self._path) + return _IsolatedEnvVenvPip(path=self._path, python_executable=executable, pip_executable=pip_executable) + except Exception: # cleanup if creation fails + self.__exit__(*sys.exc_info()) + raise def __exit__(self, exc_type, exc_val, exc_tb): # type: (Optional[Type[BaseException]], Optional[BaseException], Optional[TracebackType]) -> None """ - Exit from the isolated build environment. + Delete the created isolated build environment. :param exc_type: the type of exception raised (if any) :param exc_val: the value of exception raised (if any) :param exc_tb: the traceback of exception raised (if any) """ - self.close() - - def close(self): # type: () -> None - """Close the isolated cleanup environment.""" - if os.path.exists(self._path): # in case the user already deleted skip remove + if self._path is not None and os.path.exists(self._path): # in case the user already deleted skip remove shutil.rmtree(self._path) + +class _IsolatedEnvVenvPip(IsolatedEnvironment): + """ + Isolated build environment context manager + + Non-standard paths injected directly to sys.path still be passed to the environment. + """ + + def __init__(self, path, python_executable, pip_executable): + # type: (str, str, str) -> None + """ + Define an isolated build environment. + + :param path: the path where the environment exists + :param python_executable: the python executable within the environment + :param pip_executable: an executable that allows installing packages within the environment + """ + self._path = path + self._pip_executable = pip_executable + self._python_executable = python_executable + @property def path(self): # type: () -> str """:return: the location of the isolated build environment""" @@ -94,10 +118,10 @@ def install(self, requirements): # type: (Iterable[str]) -> None req_file.write(os.linesep.join(requirements)) try: cmd = [ - self._install_executable, + self._pip_executable, # on python2 if isolation is achieved via environment variables, we need to ignore those while calling # host python (otherwise pip would not be available within it) - '-{}m'.format('E' if self._install_executable == self.executable and sys.version_info[0] == 2 else ''), + '-{}m'.format('E' if self._pip_executable == self.executable and sys.version_info[0] == 2 else ''), 'pip', 'install', '--prefix', @@ -186,4 +210,7 @@ def _find_executable(path): # type: (str) -> str return executable -__all__ = ('IsolatedEnvironment',) +__all__ = ( + 'IsolatedEnvBuilder', + 'IsolatedEnvironment', +) diff --git a/tests/test_env.py b/tests/test_env.py index a0debb1d..1f4b4e66 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -13,7 +13,7 @@ @pytest.mark.isolated def test_isolation(): subprocess.check_call([sys.executable, '-c', 'import build.env']) - with build.env.IsolatedEnvironment.for_current() as env: + with build.env.IsolatedEnvBuilder() as env: with pytest.raises(subprocess.CalledProcessError): debug = 'import sys; import os; print(os.linesep.join(sys.path));' subprocess.check_call([env.executable, '-c', '{} import build.env'.format(debug)]) @@ -21,7 +21,7 @@ def test_isolation(): @pytest.mark.isolated def test_isolated_environment_install(mocker): - with build.env.IsolatedEnvironment.for_current() as env: + with build.env.IsolatedEnvBuilder() as env: mocker.patch('subprocess.check_call') env.install([]) @@ -32,8 +32,8 @@ def test_isolated_environment_install(mocker): subprocess.check_call.assert_called() args = subprocess.check_call.call_args[0][0][:-1] assert args == [ - env._install_executable, - '-{}m'.format('E' if env._install_executable == env._python_executable and sys.version_info[0] == 2 else ''), + env._pip_executable, + '-{}m'.format('E' if env._pip_executable == env._python_executable and sys.version_info[0] == 2 else ''), 'pip', 'install', '--prefix', @@ -49,11 +49,11 @@ def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): mocker.patch.object(build.env, 'pip', None) expected = {'pip', 'greenlet', 'readline', 'cffi'} if platform.python_implementation() == 'PyPy' else {'pip'} - with build.env.IsolatedEnvironment.for_current() as isolated_env: + with build.env.IsolatedEnvBuilder() as isolated_env: cmd = [isolated_env.executable, '-m', 'pip', 'list', '--format', 'json'] packages = {p['name'] for p in json.loads(subprocess.check_output(cmd, universal_newlines=True))} assert packages == expected - assert isolated_env._install_executable == isolated_env.executable + assert isolated_env._pip_executable == isolated_env.executable out, err = capfd.readouterr() if sys.version_info[0] == 3: assert out # ensurepip prints onto the stdout @@ -64,9 +64,9 @@ def test_create_isolated_build_host_with_no_pip(tmp_path, capfd, mocker): @pytest.mark.isolated def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): - with build.env.IsolatedEnvironment.for_current() as isolated_env: + with build.env.IsolatedEnvBuilder() as isolated_env: pass - assert isolated_env._install_executable == sys.executable + assert isolated_env._pip_executable == sys.executable out, err = capfd.readouterr() assert not out assert not err @@ -76,7 +76,7 @@ def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): def test_fail_to_get_script_path(mocker): get_path = mocker.patch('sysconfig.get_path', return_value=None) with pytest.raises(RuntimeError, match="Couldn't get environment scripts path"): - with build.env.IsolatedEnvironment.for_current(): + with build.env.IsolatedEnvBuilder(): pass assert get_path.call_count == 1 @@ -93,6 +93,6 @@ def _get_path(name, vars): # noqa get_path = mocker.patch('sysconfig.get_path', side_effect=_get_path) with pytest.raises(RuntimeError, match='Virtual environment creation failed, executable .* missing'): - with build.env.IsolatedEnvironment.for_current(): + with build.env.IsolatedEnvBuilder(): pass assert get_path.call_count == 1 diff --git a/tests/test_main.py b/tests/test_main.py index af4d2e07..bca03a1a 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -90,7 +90,7 @@ def test_prog(): def test_build_isolated(mocker, test_flit_path): build_cmd = mocker.patch('build.ProjectBuilder.build') mocker.patch('build.__main__._error') - install = mocker.patch('build.env.IsolatedEnvironment.install') + install = mocker.patch('build.env._IsolatedEnvVenvPip.install') build.__main__.build(test_flit_path, '.', ['sdist']) @@ -124,7 +124,7 @@ def test_build_no_isolation_with_check_deps(mocker, test_flit_path): def test_build_raises_build_exception(mocker, test_flit_path): error = mocker.patch('build.__main__._error') mocker.patch('build.ProjectBuilder.build', side_effect=build.BuildException) - mocker.patch('build.env.IsolatedEnvironment.install') + mocker.patch('build.env._IsolatedEnvVenvPip.install') build.__main__.build(test_flit_path, '.', ['sdist']) @@ -135,7 +135,7 @@ def test_build_raises_build_exception(mocker, test_flit_path): def test_build_raises_build_backend_exception(mocker, test_flit_path): error = mocker.patch('build.__main__._error') mocker.patch('build.ProjectBuilder.build', side_effect=build.BuildBackendException) - mocker.patch('build.env.IsolatedEnvironment.install') + mocker.patch('build.env._IsolatedEnvVenvPip.install') build.__main__.build(test_flit_path, '.', ['sdist']) From 9253f6d0a29b5b01896887801ccf2f08af8962c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 20 Oct 2020 10:02:02 +0100 Subject: [PATCH 5/5] Make the typing gods happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- setup.cfg | 4 +++- src/build/__init__.py | 2 +- src/build/_compat.py | 42 ++++++++++++++++++++++++++++++++++++++++++ src/build/env.py | 21 ++++++++++----------- tests/test_env.py | 29 ++++++++++++++++++++++++++++- tox.ini | 3 +-- 6 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 src/build/_compat.py diff --git a/setup.cfg b/setup.cfg index 71d17c44..02cfdb25 100644 --- a/setup.cfg +++ b/setup.cfg @@ -50,6 +50,9 @@ test = pytest pytest-cov pytest-mock +typing = + mypy==0.790 + typing-extensions>=3.7.4.3 [options.packages.find] where = src @@ -82,7 +85,6 @@ omit = exclude_lines = \#\s*pragma: no cover ^\s*raise NotImplementedError\b - ^if __name__ == ['"]__main__['"]:$ [coverage:paths] source = diff --git a/src/build/__init__.py b/src/build/__init__.py index bfa8e782..d6d34556 100644 --- a/src/build/__init__.py +++ b/src/build/__init__.py @@ -117,6 +117,7 @@ def _working_directory(path): # type: (str) -> Iterator[None] class ProjectBuilder(object): def __init__(self, srcdir='.', config_settings=None, python_executable=sys.executable): + # type: (str, Optional[ConfigSettings], str) -> None """ Create a project builder. @@ -124,7 +125,6 @@ def __init__(self, srcdir='.', config_settings=None, python_executable=sys.execu :param config_settings: config settings for the build backend :param python_executable: the python executable where the backend lives """ - # type: (str, Optional[ConfigSettings], str) -> None self.srcdir = os.path.abspath(srcdir) self.config_settings = config_settings if config_settings else {} diff --git a/src/build/_compat.py b/src/build/_compat.py new file mode 100644 index 00000000..3a5da79b --- /dev/null +++ b/src/build/_compat.py @@ -0,0 +1,42 @@ +import abc +import sys +from typing import Callable, TypeVar + +_T = TypeVar('_T') + + +def add_metaclass(metaclass): # type: (type) -> Callable[[_T], _T] + """Class decorator for creating a class with a metaclass (borrowed from six code).""" + + def wrapper(cls): # type: (_T) -> _T + orig_vars = cls.__dict__.copy() + slots = orig_vars.get('__slots__') + if slots is not None: + if isinstance(slots, str): # pragma: no cover + slots = [slots] # pragma: no cover + for slots_var in slots: # pragma: no cover + orig_vars.pop(slots_var) # pragma: no cover + orig_vars.pop('__dict__', None) + orig_vars.pop('__weakref__', None) + if hasattr(cls, '__qualname__'): + orig_vars['__qualname__'] = cls.__qualname__ # type: ignore + return metaclass(cls.__name__, cls.__bases__, orig_vars) # type: ignore + + return wrapper + + +if sys.version_info[0] == 2: + abstractproperty = abc.abstractproperty +else: + from typing import Any, cast + + F = TypeVar('F', bound=Callable[..., Any]) + + def abstractproperty(func): # type: (F) -> F + return cast(F, property(abc.abstractmethod(func))) + + +__all__ = ( + 'abstractproperty', + 'add_metaclass', +) diff --git a/src/build/env.py b/src/build/env.py index 9d5bc805..0c1be7ec 100644 --- a/src/build/env.py +++ b/src/build/env.py @@ -12,20 +12,19 @@ from types import TracebackType from typing import Iterable, Optional, Tuple, Type +from ._compat import abstractproperty, add_metaclass + try: import pip except ImportError: # pragma: no cover pip = None # pragma: no cover -ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()}) # Python 2/3 compatible ABC - - -class IsolatedEnvironment(ABC): +@add_metaclass(abc.ABCMeta) +class IsolatedEnv(object): """Abstract base of isolated build environments, as required by the build project.""" - @property - @abc.abstractmethod + @abstractproperty def executable(self): # type: () -> str """Return the executable of the isolated build environment.""" raise NotImplementedError @@ -41,11 +40,11 @@ def install(self, requirements): # type: (Iterable[str]) -> None class IsolatedEnvBuilder(object): - def __init__(self): + def __init__(self): # type: () -> None """Builder object for isolated environment.""" self._path = None # type: Optional[str] - def __enter__(self): # type: () -> IsolatedEnvironment + def __enter__(self): # type: () -> IsolatedEnv """ Creates an isolated build environment. @@ -55,7 +54,7 @@ def __enter__(self): # type: () -> IsolatedEnvironment try: executable, pip_executable = _create_isolated_env(self._path) return _IsolatedEnvVenvPip(path=self._path, python_executable=executable, pip_executable=pip_executable) - except Exception: # cleanup if creation fails + except Exception: # cleanup folder if creation fails self.__exit__(*sys.exc_info()) raise @@ -72,7 +71,7 @@ def __exit__(self, exc_type, exc_val, exc_tb): shutil.rmtree(self._path) -class _IsolatedEnvVenvPip(IsolatedEnvironment): +class _IsolatedEnvVenvPip(IsolatedEnv): """ Isolated build environment context manager @@ -212,5 +211,5 @@ def _find_executable(path): # type: (str) -> str __all__ = ( 'IsolatedEnvBuilder', - 'IsolatedEnvironment', + 'IsolatedEnv', ) diff --git a/tests/test_env.py b/tests/test_env.py index 1f4b4e66..9a588acb 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: MIT import json +import os import platform import shutil import subprocess @@ -76,8 +77,10 @@ def test_create_isolated_build_has_with_pip(tmp_path, capfd, mocker): def test_fail_to_get_script_path(mocker): get_path = mocker.patch('sysconfig.get_path', return_value=None) with pytest.raises(RuntimeError, match="Couldn't get environment scripts path"): - with build.env.IsolatedEnvBuilder(): + env = build.env.IsolatedEnvBuilder() + with env: pass + assert not os.path.exists(env._path) assert get_path.call_count == 1 @@ -96,3 +99,27 @@ def _get_path(name, vars): # noqa with build.env.IsolatedEnvBuilder(): pass assert get_path.call_count == 1 + + +def test_isolated_env_abstract(): + with pytest.raises(TypeError): + build.env.IsolatedEnv() + + +def test_isolated_env_has_executable_still_abstract(): + class Env(build.env.IsolatedEnv): # noqa + @property + def executable(self): + raise NotImplementedError + + with pytest.raises(TypeError): + Env() + + +def test_isolated_env_has_install_still_abstract(): + class Env(build.env.IsolatedEnv): # noqa + def install(self, requirements): + raise NotImplementedError + + with pytest.raises(TypeError): + Env() diff --git a/tox.ini b/tox.ini index e32bb0dc..31dff1b3 100644 --- a/tox.ini +++ b/tox.ini @@ -52,8 +52,7 @@ commands = [testenv:type] description = run type check on code base -deps = - mypy==0.782 +extras = typing commands = mypy --python-version 3.8 src/build mypy --python-version 2.7 src/build