Repository: scikit-build/scikit-build-core
Pull request #543 (2023-11-06 16:37:06)

fix: filter CMAKE_ARGS and debug printout
I think this would help with https://github.com/scikit-build/scikit-build-core/issues/542.



Patch:
diff --git a/src/scikit_build_core/build/_editable.py b/src/scikit_build_core/build/_editable.py
new file mode 100644
index 0000000..fe2fa4f
--- /dev/null
+++ b/src/scikit_build_core/build/_editable.py
@@ -0,0 +1,69 @@
+from __future__ import annotations
+
+import os
+from pathlib import Path
+
+from ..resources import resources
+from ._pathutil import (
+    is_valid_module,
+    path_to_module,
+    scantree,
+)
+
+__all__ = ["editable_redirect", "libdir_to_installed", "mapping_to_modules"]
+
+
+def __dir__() -> list[str]:
+    return __all__
+
+
+def editable_redirect(
+    *,
+    modules: dict[str, str],
+    installed: dict[str, str],
+    reload_dir: Path | None,
+    rebuild: bool,
+    verbose: bool,
+    build_options: list[str],
+    install_options: list[str],
+) -> str:
+    """
+    Prepare the contents of the _editable_redirect.py file.
+    """
+
+    editable_py = resources / "_editable_redirect.py"
+    editable_txt: str = editable_py.read_text(encoding="utf-8")
+
+    arguments = (
+        modules,
+        installed,
+        os.fspath(reload_dir) if reload_dir else None,
+        rebuild,
+        verbose,
+        build_options,
+        install_options,
+    )
+    arguments_str = ", ".join(repr(x) for x in arguments if x is not None)
+    editable_txt += f"\n\ninstall({arguments_str})\n"
+    return editable_txt
+
+
+def mapping_to_modules(mapping: dict[str, str], libdir: Path) -> dict[str, str]:
+    """
+    Convert a mapping of files to modules to a mapping of modules to installed files.
+    """
+    return {
+        path_to_module(Path(v).relative_to(libdir)): str(Path(k).resolve())
+        for k, v in mapping.items()
+        if is_valid_module(Path(v).relative_to(libdir))
+    }
+
+
+def libdir_to_installed(libdir: Path) -> dict[str, str]:
+    """
+    Convert a mapping of files to modules to a mapping of modules to installed files.
+    """
+    return {
+        path_to_module(v.relative_to(libdir)): str(v.relative_to(libdir))
+        for v in scantree(libdir)
+    }
diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py
index 4cb8631..14de9d0 100644
--- a/src/scikit_build_core/build/wheel.py
+++ b/src/scikit_build_core/build/wheel.py
@@ -17,15 +17,12 @@ from .._shutil import fix_win_37_all_permissions
 from ..builder.builder import Builder, archs_to_tags, get_archs
 from ..builder.wheel_tag import WheelTag
 from ..cmake import CMake, CMaker
-from ..resources import resources
 from ..settings.metadata import get_standard_metadata
 from ..settings.skbuild_read_settings import SettingsReader
+from ._editable import editable_redirect, libdir_to_installed, mapping_to_modules
 from ._init import setup_logging
 from ._pathutil import (
-    is_valid_module,
     packages_to_file_mapping,
-    path_to_module,
-    scantree,
 )
 from ._scripts import process_script_dir
 from ._wheelfile import WheelWriter
@@ -34,6 +31,8 @@ from .generate import generate_file_contents
 if TYPE_CHECKING:
     from collections.abc import Sequence
 
+    from ..settings.skbuild_model import ScikitBuildSettings
+
 __all__ = ["_build_wheel_impl"]
 
 
@@ -41,6 +40,44 @@ def __dir__() -> list[str]:
     return __all__
 
 
+def _make_editable(
+    *,
+    builder: Builder,
+    libdir: Path,
+    mapping: dict[str, str],
+    name: str,
+    reload_dir: Path | None,
+    settings: ScikitBuildSettings,
+    wheel: WheelWriter,
+) -> None:
+    modules = mapping_to_modules(mapping, libdir)
+    installed = libdir_to_installed(libdir)
+
+    options = []
+    if not builder.config.single_config and builder.config.build_type:
+        options += ["--config", builder.config.build_type]
+    ext_build_opts = ["-v"] if builder.settings.cmake.verbose else []
+
+    editable_txt = editable_redirect(
+        modules=modules,
+        installed=installed,
+        reload_dir=reload_dir,
+        rebuild=settings.editable.rebuild,
+        verbose=settings.editable.verbose,
+        build_options=options + ext_build_opts,
+        install_options=options,
+    )
+
+    wheel.writestr(
+        f"_{name}_editable.py",
+        editable_txt.encode(),
+    )
+    wheel.writestr(
+        f"_{name}_editable.pth",
+        f"import _{name}_editable\n".encode(),
+    )
+
+
 def _get_packages(
     *,
     packages: Sequence[str] | None,
@@ -286,48 +323,16 @@ def _build_wheel_impl(
             wheel.build(wheel_dirs)
 
             if editable:
-                modules = {
-                    path_to_module(Path(v).relative_to(wheel_dirs["platlib"])): str(
-                        Path(k).resolve()
-                    )
-                    for k, v in mapping.items()
-                    if is_valid_module(Path(v).relative_to(wheel_dirs["platlib"]))
-                }
-                installed = {
-                    path_to_module(v.relative_to(wheel_dirs["platlib"])): str(
-                        v.relative_to(wheel_dirs["platlib"])
-                    )
-                    for v in scantree(wheel_dirs["platlib"])
-                }
-                editable_py = resources / "_editable_redirect.py"
-                editable_txt = editable_py.read_text(encoding="utf-8")
-                reload_dir = (
-                    os.fspath(build_dir.resolve()) if settings.build_dir else None
-                )
-                options = []
-                if not builder.config.single_config and builder.config.build_type:
-                    options += ["--config", builder.config.build_type]
-                ext_build_opts = ["-v"] if builder.settings.cmake.verbose else []
-
-                arguments = (
-                    modules,
-                    installed,
-                    reload_dir,
-                    settings.editable.rebuild,
-                    settings.editable.verbose,
-                    options + ext_build_opts,
-                    options,
-                )
-                arguments_str = ", ".join(repr(x) for x in arguments)
-                editable_txt += f"\n\ninstall({arguments_str})\n"
-
-                wheel.writestr(
-                    f"_{normalized_name}_editable.py",
-                    editable_txt.encode("utf-8"),
-                )
-                wheel.writestr(
-                    f"_{normalized_name}_editable.pth",
-                    f"import _{normalized_name}_editable\n".encode(),
+                reload_dir = build_dir.resolve() if settings.build_dir else None
+
+                _make_editable(
+                    libdir=wheel_dirs["platlib"],
+                    mapping=mapping,
+                    reload_dir=reload_dir,
+                    builder=builder,
+                    settings=settings,
+                    wheel=wheel,
+                    name=normalized_name,
                 )
 
     if metadata_directory is not None:
diff --git a/src/scikit_build_core/builder/builder.py b/src/scikit_build_core/builder/builder.py
index 9badebb..75ce56e 100644
--- a/src/scikit_build_core/builder/builder.py
+++ b/src/scikit_build_core/builder/builder.py
@@ -21,7 +21,7 @@ from .sysconfig import (
 )
 
 if TYPE_CHECKING:
-    from collections.abc import Iterable, Mapping, Sequence
+    from collections.abc import Generator, Iterable, Mapping, Sequence
 
     from packaging.version import Version
 
@@ -69,6 +69,20 @@ def archs_to_tags(archs: list[str]) -> list[str]:
     return archs
 
 
+def _filter_env_cmake_args(env_cmake_args: list[str]) -> Generator[str, None, None]:
+    """
+    Filter out CMake arguments that are not supported from CMAKE_ARGS.
+    """
+
+    unsupported_args = ("-DCMAKE_BUILD_TYPE", "-DCMAKE_INSTALL_PREFIX")
+
+    for arg in env_cmake_args:
+        if arg.startswith(unsupported_args):
+            logger.warning("Unsupported CMAKE_ARGS ignored: {}", arg)
+        else:
+            yield arg
+
+
 @dataclasses.dataclass
 class Builder:
     settings: ScikitBuildSettings
@@ -80,9 +94,13 @@ class Builder:
         """
         # Adding CMake arguments set as environment variable
         # (needed e.g. to build for ARM OSX on conda-forge)
-        env_cmake_args = filter(None, self.config.env.get("CMAKE_ARGS", "").split(" "))
+        env_cmake_args = list(
+            filter(None, self.config.env.get("CMAKE_ARGS", "").split(" "))
+        )
+        if env_cmake_args:
+            logger.debug("Env CMAKE_ARGS: {}", env_cmake_args)
 
-        return [*self.settings.cmake.args, *env_cmake_args]
+        return [*self.settings.cmake.args, *_filter_env_cmake_args(env_cmake_args)]
 
     def get_generator(self, *args: str) -> str | None:
         return self.config.get_generator(*self.get_cmake_args(), *args)
diff --git a/src/scikit_build_core/resources/_editable_redirect.py b/src/scikit_build_core/resources/_editable_redirect.py
index a2e5fb6..ee26921 100644
--- a/src/scikit_build_core/resources/_editable_redirect.py
+++ b/src/scikit_build_core/resources/_editable_redirect.py
@@ -70,7 +70,7 @@ class ScikitBuildRedirectingFinder(importlib.abc.MetaPathFinder):
                 # Initialize the tree element if needed
                 submodule_search_locations.setdefault(parent, set())
                 # Add the parent path to the dictionary values
-                parent_path, _ = os.path.split(file)
+                parent_path = os.path.dirname(file)
                 if not parent_path:
                     # root modules are skipped so all files should be in a parent package
                     msg = f"Unexpected path to source file: {file} [{module}]"


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index ef52183..3e87f17 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -25,14 +25,9 @@ else:
     import tomllib
 
 
-from typing import TYPE_CHECKING
-
 import pytest
 from packaging.requirements import Requirement
 
-if TYPE_CHECKING:
-    from collections.abc import Generator
-
 DIR = Path(__file__).parent.resolve()
 BASE = DIR.parent
 
@@ -161,21 +156,15 @@ class VEnv:
 
 
 @pytest.fixture()
-def isolated(tmp_path: Path, pep518_wheelhouse: Path) -> Generator[VEnv, None, None]:
+def isolated(tmp_path: Path, pep518_wheelhouse: Path) -> VEnv:
     path = tmp_path / "venv"
-    try:
-        yield VEnv(path, wheelhouse=pep518_wheelhouse)
-    finally:
-        shutil.rmtree(path, ignore_errors=True)
+    return VEnv(path, wheelhouse=pep518_wheelhouse)
 
 
 @pytest.fixture()
-def virtualenv(tmp_path: Path) -> Generator[VEnv, None, None]:
+def virtualenv(tmp_path: Path) -> VEnv:
     path = tmp_path / "venv"
-    try:
-        yield VEnv(path)
-    finally:
-        shutil.rmtree(path, ignore_errors=True)
+    return VEnv(path)
 
 
 @dataclasses.dataclass(frozen=True)
diff --git a/tests/test_editable_unit.py b/tests/test_editable_unit.py
new file mode 100644
index 0000000..cd07c0f
--- /dev/null
+++ b/tests/test_editable_unit.py
@@ -0,0 +1,161 @@
+from __future__ import annotations
+
+import textwrap
+import typing
+from pathlib import Path
+from typing import NamedTuple
+
+import pytest
+
+from scikit_build_core.build._editable import (
+    editable_redirect,
+    libdir_to_installed,
+    mapping_to_modules,
+)
+from scikit_build_core.build._pathutil import packages_to_file_mapping
+
+if typing.TYPE_CHECKING:
+    from conftest import VEnv
+
+
+class EditablePackage(NamedTuple):
+    site_packages: Path
+    pkg_dir: Path
+    src_pkg_dir: Path
+
+
+@pytest.fixture(
+    params=[
+        pytest.param(False, id="abs"),
+        pytest.param(
+            True,
+            marks=pytest.mark.xfail(strict=True, reason="Rel imports broken"),
+            id="rel",
+        ),
+    ]
+)
+def editable_package(
+    request: pytest.FixtureRequest,
+    tmp_path: Path,
+    virtualenv: VEnv,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    rel = request.param
+
+    prefix = "" if rel else "pkg"
+
+    source_dir = tmp_path / "source"
+    source_dir.mkdir()
+    # Functions in build.wheel require running from this dir
+    monkeypatch.chdir(source_dir)
+
+    site_packages = virtualenv.purelib
+
+    # Create a fake package
+    pkg_dir = site_packages / "pkg"
+    pkg_dir.mkdir()
+    src_pkg_dir = source_dir / "pkg"
+    src_pkg_dir.mkdir()
+
+    # Make some fake files
+    (src_pkg_dir / "__init__.py").touch()
+    (src_pkg_dir / "module.py").write_text(
+        textwrap.dedent(
+            f"""\
+            from {prefix}.subpkg import module
+            from {prefix}.subpkg import source
+            from {prefix}.namespace import module
+            from {prefix}.namespace import source
+            """
+        )
+    )
+    (pkg_dir / "source.py").write_text(
+        textwrap.dedent(
+            f"""\
+            from {prefix}.subpkg import module
+            from {prefix}.subpkg import source
+            from {prefix}.namespace import module
+            from {prefix}.namespace import source
+            """
+        )
+    )
+
+    src_sub_package = src_pkg_dir / "subpkg"
+    src_sub_package.mkdir()
+    src_sub_package.joinpath("__init__.py").touch()
+    src_sub_package.joinpath("module.py").touch()
+
+    sub_package = pkg_dir / "subpkg"
+    sub_package.mkdir()
+    sub_package.joinpath("source.py").touch()
+
+    src_namespace_pkg = src_pkg_dir / "namespace"
+    src_namespace_pkg.mkdir()
+    src_namespace_pkg.joinpath("module.py").touch()
+
+    namespace_pkg = pkg_dir / "namespace"
+    namespace_pkg.mkdir()
+    namespace_pkg.joinpath("source.py").touch()
+
+    return EditablePackage(site_packages, pkg_dir, src_pkg_dir)
+
+
+def test_navigate_editable_pkg(editable_package: EditablePackage, virtualenv: VEnv):
+    site_packages, pkg_dir, src_pkg_dir = editable_package
+
+    # Create a fake editable install
+    packages = ["pkg"]
+    mapping = packages_to_file_mapping(
+        packages=packages,
+        platlib_dir=site_packages,
+        include=[],
+        exclude=[],
+    )
+    assert mapping == {
+        str(Path("pkg/__init__.py")): str(pkg_dir / "__init__.py"),
+        str(Path("pkg/module.py")): str(pkg_dir / "module.py"),
+        str(Path("pkg/namespace/module.py")): str(pkg_dir / "namespace/module.py"),
+        str(Path("pkg/subpkg/__init__.py")): str(pkg_dir / "subpkg/__init__.py"),
+        str(Path("pkg/subpkg/module.py")): str(pkg_dir / "subpkg/module.py"),
+    }
+    modules = mapping_to_modules(mapping, libdir=site_packages)
+
+    assert modules == {
+        "pkg": str(src_pkg_dir / "__init__.py"),
+        "pkg.module": str(src_pkg_dir / "module.py"),
+        "pkg.namespace.module": str(src_pkg_dir / "namespace/module.py"),
+        "pkg.subpkg": str(src_pkg_dir / "subpkg/__init__.py"),
+        "pkg.subpkg.module": str(src_pkg_dir / "subpkg/module.py"),
+    }
+
+    installed = libdir_to_installed(site_packages)
+    installed = {k: v for k, v in installed.items() if k.startswith("pkg")}
+
+    assert installed == {
+        "pkg.subpkg.source": str(Path("pkg/subpkg/source.py")),
+        "pkg.namespace.source": str(Path("pkg/namespace/source.py")),
+        "pkg.source": str(Path("pkg/source.py")),
+    }
+
+    editable_txt = editable_redirect(
+        modules=modules,
+        installed=installed,
+        reload_dir=None,
+        rebuild=False,
+        verbose=False,
+        build_options=[],
+        install_options=[],
+    )
+
+    site_packages.joinpath("_pkg_editable.py").write_text(editable_txt)
+    site_packages.joinpath("_pkg_editable.pth").write_text("import _pkg_editable\n")
+
+    # Test the editable install
+    virtualenv.execute("import pkg.subpkg")
+    virtualenv.execute("import pkg.subpkg.module")
+    virtualenv.execute("import pkg.subpkg.source")
+    virtualenv.execute("import pkg.namespace.module")
+    virtualenv.execute("import pkg.namespace.source")
+    # This allows debug print statements in _editable_redirect.py to be seen
+    print(virtualenv.execute("import pkg.module"))
+    print(virtualenv.execute("import pkg.source"))