More PR feedback

Signed-off-by: Bernát Gábor <[email protected]>

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

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)

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:

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',
+)

tests/test_env.py

@@ -13,15 +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.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)])
 
 
 @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

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'])