Skip to content

Commit

Permalink
Improve backend loading from backend-path (#165)
Browse files Browse the repository at this point in the history
* Test backend is loaded from backend-path

* Actively import from backend-path instead of checking

Previously `_in_process` would only modify `sys.path`, use
`importlib.import_module` and hope the backend is loaded from the right
path.

This would lead to misleading error messages in some cases when the
environment isolation is not perfect.

This change modifies the behavior by using
`importlib.machinery.PathFinder.find_spec` to locate the backend module
within a set of path entries.

* Unify usages of BackedInvalid with BackendUnavailable

… when error regards import problems.

* Fix path sep error when testing on windows

* Improve explanation in code comment

* Ensure backend info is available in exception

* Replace custom import sequence with MetaPathFinder

This should address review comments and increase the robustness of the
solution.

* Improve test docstring for intree backend

* Replace comment with a more appropriate description

* Mark BackendInvalid as no longer used/deprecated

* Remove unused BackendInvalid

* Test nested intree backend

* Add negative tests for nested intree backends

* Avoid passing nested modules to PathFinder
  • Loading branch information
abravalheri authored Aug 19, 2023
1 parent a3456dd commit 084b02e
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 49 deletions.
1 change: 0 additions & 1 deletion docs/pyproject_hooks.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ Exceptions

Each exception has public attributes with the same name as their constructors.

.. autoexception:: pyproject_hooks.BackendInvalid
.. autoexception:: pyproject_hooks.BackendUnavailable
.. autoexception:: pyproject_hooks.HookMissing
.. autoexception:: pyproject_hooks.UnsupportedOperation
2 changes: 0 additions & 2 deletions src/pyproject_hooks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"""

from ._impl import (
BackendInvalid,
BackendUnavailable,
BuildBackendHookCaller,
HookMissing,
Expand All @@ -14,7 +13,6 @@
__version__ = "1.0.0"
__all__ = [
"BackendUnavailable",
"BackendInvalid",
"HookMissing",
"UnsupportedOperation",
"default_subprocess_runner",
Expand Down
20 changes: 7 additions & 13 deletions src/pyproject_hooks/_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,12 @@ def read_json(path):
class BackendUnavailable(Exception):
"""Will be raised if the backend cannot be imported in the hook process."""

def __init__(self, traceback):
self.traceback = traceback


class BackendInvalid(Exception):
"""Will be raised if the backend is invalid."""

def __init__(self, backend_name, backend_path, message):
super().__init__(message)
def __init__(self, traceback, message=None, backend_name=None, backend_path=None):
# Preserving arg order for the sake of API backward compatibility.
self.backend_name = backend_name
self.backend_path = backend_path
self.traceback = traceback
super().__init__(message or "Error while importing backend")


class HookMissing(Exception):
Expand Down Expand Up @@ -334,12 +329,11 @@ def _call_hook(self, hook_name, kwargs):
if data.get("unsupported"):
raise UnsupportedOperation(data.get("traceback", ""))
if data.get("no_backend"):
raise BackendUnavailable(data.get("traceback", ""))
if data.get("backend_invalid"):
raise BackendInvalid(
raise BackendUnavailable(
data.get("traceback", ""),
message=data.get("backend_error", ""),
backend_name=self.build_backend,
backend_path=self.backend_path,
message=data.get("backend_error", ""),
)
if data.get("hook_missing"):
raise HookMissing(data.get("missing_hook_name") or hook_name)
Expand Down
66 changes: 40 additions & 26 deletions src/pyproject_hooks/_in_process/_in_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import traceback
from glob import glob
from importlib import import_module
from importlib.machinery import PathFinder
from os.path import join as pjoin

# This file is run as a script, and `import wrappers` is not zip-safe, so we
Expand All @@ -40,15 +41,10 @@ def read_json(path):
class BackendUnavailable(Exception):
"""Raised if we cannot import the backend"""

def __init__(self, traceback):
self.traceback = traceback


class BackendInvalid(Exception):
"""Raised if the backend is invalid"""

def __init__(self, message):
def __init__(self, message, traceback=None):
super().__init__(message)
self.message = message
self.traceback = traceback


class HookMissing(Exception):
Expand All @@ -59,38 +55,58 @@ def __init__(self, hook_name=None):
self.hook_name = hook_name


def contained_in(filename, directory):
"""Test if a file is located within the given directory."""
filename = os.path.normcase(os.path.abspath(filename))
directory = os.path.normcase(os.path.abspath(directory))
return os.path.commonprefix([filename, directory]) == directory


def _build_backend():
"""Find and load the build backend"""
# Add in-tree backend directories to the front of sys.path.
backend_path = os.environ.get("_PYPROJECT_HOOKS_BACKEND_PATH")
ep = os.environ["_PYPROJECT_HOOKS_BUILD_BACKEND"]
mod_path, _, obj_path = ep.partition(":")

if backend_path:
# Ensure in-tree backend directories have the highest priority when importing.
extra_pathitems = backend_path.split(os.pathsep)
sys.path[:0] = extra_pathitems
sys.meta_path.insert(0, _BackendPathFinder(extra_pathitems, mod_path))

ep = os.environ["_PYPROJECT_HOOKS_BUILD_BACKEND"]
mod_path, _, obj_path = ep.partition(":")
try:
obj = import_module(mod_path)
except ImportError:
raise BackendUnavailable(traceback.format_exc())

if backend_path:
if not any(contained_in(obj.__file__, path) for path in extra_pathitems):
raise BackendInvalid("Backend was not loaded from backend-path")
msg = f"Cannot import {mod_path!r}"
raise BackendUnavailable(msg, traceback.format_exc())

if obj_path:
for path_part in obj_path.split("."):
obj = getattr(obj, path_part)
return obj


class _BackendPathFinder:
"""Implements the MetaPathFinder interface to locate modules in ``backend-path``.
Since the environment provided by the frontend can contain all sorts of
MetaPathFinders, the only way to ensure the backend is loaded from the
right place is to prepend our own.
"""

def __init__(self, backend_path, backend_module):
self.backend_path = backend_path
self.backend_module = backend_module
self.backend_parent, _, _ = backend_module.partition(".")

def find_spec(self, fullname, _path, _target=None):
if "." in fullname:
# Rely on importlib to find nested modules based on parent's path
return None

# Ignore other items in _path or sys.path and use backend_path instead:
spec = PathFinder.find_spec(fullname, path=self.backend_path)
if spec is None and fullname == self.backend_parent:
# According to the spec, the backend MUST be loaded from backend-path.
# Therefore, we can halt the import machinery and raise a clean error.
msg = f"Cannot find module {self.backend_module!r} in {self.backend_path!r}"
raise BackendUnavailable(msg)

return spec


def _supported_features():
"""Return the list of options features supported by the backend.
Expand Down Expand Up @@ -342,8 +358,6 @@ def main():
except BackendUnavailable as e:
json_out["no_backend"] = True
json_out["traceback"] = e.traceback
except BackendInvalid as e:
json_out["backend_invalid"] = True
json_out["backend_error"] = e.message
except GotUnsupportedOperation as e:
json_out["unsupported"] = True
Expand Down
1 change: 1 addition & 0 deletions tests/samples/buildsys_pkgs/nested/buildsys.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from ..buildsys import * # noqa: F403
3 changes: 3 additions & 0 deletions tests/samples/pkg_nested_intree/backend/intree_backend.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# PathFinder.find_spec only take into consideration the last segment
# of the module name (not the full name).
raise Exception("This isn't the backend you are looking for")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def get_requires_for_build_sdist(config_settings):
return ["intree_backend_called"]
3 changes: 3 additions & 0 deletions tests/samples/pkg_nested_intree/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[build-system]
build-backend = 'nested.intree_backend'
backend-path = ['backend']
4 changes: 3 additions & 1 deletion tests/test_call_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ def get_hooks(pkg, **kwargs):
def test_missing_backend_gives_exception():
hooks = get_hooks("pkg1")
with modified_env({"PYTHONPATH": ""}):
with pytest.raises(BackendUnavailable):
msg = "Cannot import 'buildsys'"
with pytest.raises(BackendUnavailable, match=msg) as exc:
hooks.get_requires_for_build_wheel({})
assert exc.value.backend_name == "buildsys"


def test_get_requires_for_build_wheel():
Expand Down
60 changes: 54 additions & 6 deletions tests/test_inplace_hooks.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from inspect import cleandoc
from os.path import abspath, dirname
from os.path import join as pjoin
from pathlib import Path

import pytest
from testpath import modified_env
from testpath.tempdir import TemporaryDirectory

from pyproject_hooks import BackendInvalid, BuildBackendHookCaller
from pyproject_hooks import BackendUnavailable, BuildBackendHookCaller
from tests.compat import tomllib

SAMPLES_DIR = pjoin(dirname(abspath(__file__)), "samples")
Expand Down Expand Up @@ -49,15 +52,60 @@ def test_backend_out_of_tree(backend_path):
BuildBackendHookCaller(SOURCE_DIR, "dummy", backend_path)


def test_intree_backend():
hooks = get_hooks("pkg_intree")
@pytest.mark.parametrize("example", ("pkg_intree", "pkg_nested_intree"))
def test_intree_backend(example):
hooks = get_hooks(example)
with modified_env({"PYTHONPATH": BUILDSYS_PKGS}):
res = hooks.get_requires_for_build_sdist({})
assert res == ["intree_backend_called"]


def test_intree_backend_not_in_path():
hooks = get_hooks("pkg_intree", backend="buildsys")
@pytest.mark.parametrize("backend", ("buildsys", "nested.buildsys"))
def test_intree_backend_not_in_path(backend):
hooks = get_hooks("pkg_intree", backend=backend)
with modified_env({"PYTHONPATH": BUILDSYS_PKGS}):
with pytest.raises(BackendInvalid):
msg = f"Cannot find module {backend!r} in .*pkg_intree.*backend"
with pytest.raises(BackendUnavailable, match=msg):
hooks.get_requires_for_build_sdist({})


def test_intree_backend_loaded_from_correct_backend_path():
"""
PEP 517 establishes that the backend code should be loaded from ``backend-path``,
and recognizes that not always the environment isolation is perfect
(e.g. it explicitly mentions ``--system-site-packages``).
Therefore, even in a situation where a third-party ``MetaPathFinder`` has
precedence over ``importlib.machinery.PathFinder``, the backend should
still be loaded from ``backend-path``.
"""
hooks = get_hooks("pkg_intree", backend="intree_backend")
with TemporaryDirectory() as tmp:
invalid = Path(tmp, ".invalid", "intree_backend.py")
invalid.parent.mkdir()
invalid.write_text("raise ImportError('Do not import')", encoding="utf-8")
install_finder_with_sitecustomize(tmp, {"intree_backend": str(invalid)})
with modified_env({"PYTHONPATH": tmp}): # Override `sitecustomize`.
res = hooks.get_requires_for_build_sdist({})
assert res == ["intree_backend_called"]


def install_finder_with_sitecustomize(directory, mapping):
finder = f"""
import sys
from importlib.util import spec_from_file_location
MAPPING = {mapping!r}
class _Finder: # MetaPathFinder
@classmethod
def find_spec(cls, fullname, path=None, target=None):
if fullname in MAPPING:
return spec_from_file_location(fullname, MAPPING[fullname])
def install():
if not any(finder == _Finder for finder in sys.meta_path):
sys.meta_path.insert(0, _Finder)
"""
sitecustomize = "import _test_finder_; _test_finder_.install()"
Path(directory, "_test_finder_.py").write_text(cleandoc(finder), encoding="utf-8")
Path(directory, "sitecustomize.py").write_text(sitecustomize, encoding="utf-8")

0 comments on commit 084b02e

Please sign in to comment.