fix: editable installs now respect the value of wheel.install-dir (#867)

Attempting to fix:
#866

I'm passing down wheel.install-dir to the editable redirect script and
appending it to DIR so that rebuilds happen in the correct path. This
worked for me in a local testing environment, but I'm not familiar with
this codebase, so I don't know if this is an optimal solution.

---------

Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <[email protected]>
Co-authored-by: Cristian Le <[email protected]>

.pre-commit-config.yaml

@@ -88,7 +88,7 @@ repos:
           - rich
           - setuptools-scm
           - tomli
-          - types-setuptools>=69.2
+          - types-setuptools~=70.0
 
   - repo: https://github.com/henryiii/check-sdist
     rev: "v1.0.0rc2"

src/scikit_build_core/build/_editable.py

@@ -30,6 +30,7 @@ def editable_redirect(
     verbose: bool,
     build_options: Sequence[str],
     install_options: Sequence[str],
+    install_dir: str,
 ) -> str:
     """
     Prepare the contents of the _editable_redirect.py file.
@@ -46,6 +47,7 @@ def editable_redirect(
         verbose,
         build_options,
         install_options,
+        install_dir,
     )
     arguments_str = ", ".join(repr(x) for x in arguments)
     editable_txt += f"\n\ninstall({arguments_str})\n"

src/scikit_build_core/build/wheel.py

@@ -58,7 +58,9 @@ def _make_editable(
 ) -> None:
     modules = mapping_to_modules(mapping, libdir)
     installed = libdir_to_installed(libdir)
-
+    if settings.wheel.install_dir.startswith("/"):
+        msg = "Editable installs cannot rebuild an absolute wheel.install-dir. Use an override to change if needed."
+        raise AssertionError(msg)
     editable_txt = editable_redirect(
         modules=modules,
         installed=installed,
@@ -67,6 +69,7 @@ def _make_editable(
         verbose=settings.editable.verbose,
         build_options=build_options,
         install_options=install_options,
+        install_dir=settings.wheel.install_dir,
     )
 
     wheel.writestr(

src/scikit_build_core/resources/_editable_redirect.py

@@ -32,7 +32,8 @@ def __init__(
         verbose: bool,
         build_options: list[str],
         install_options: list[str],
-        dir: str = DIR,
+        dir: str,
+        install_dir: str,
     ) -> None:
         self.known_source_files = known_source_files
         self.known_wheel_files = known_wheel_files
@@ -42,6 +43,8 @@ def __init__(
         self.build_options = build_options
         self.install_options = install_options
         self.dir = dir
+        self.install_dir = os.path.join(DIR, install_dir)
+
         # Construct the __path__ of all resource files
         # I.e. the paths of all package-like objects
         submodule_search_locations: dict[str, set[str]] = {}
@@ -136,7 +139,14 @@ def rebuild(self) -> None:
         result.check_returncode()
 
         result = subprocess.run(
-            ["cmake", "--install", ".", "--prefix", DIR, *self.install_options],
+            [
+                "cmake",
+                "--install",
+                ".",
+                "--prefix",
+                self.install_dir,
+                *self.install_options,
+            ],
             cwd=self.path,
             stdout=sys.stderr if verbose else subprocess.PIPE,
             env=env,
@@ -159,6 +169,7 @@ def install(
     verbose: bool = False,
     build_options: list[str] | None = None,
     install_options: list[str] | None = None,
+    install_dir: str = "",
 ) -> None:
     """
     Install a meta path finder that redirects imports to the source files, and
@@ -169,6 +180,8 @@ def install(
     :param path: The path to the build directory, or None
     :param verbose: Whether to print the cmake commands (also controlled by the
                     SKBUILD_EDITABLE_VERBOSE environment variable)
+    :param install_dir: The wheel install directory override, if one was
+                        specified
     """
     sys.meta_path.insert(
         0,
@@ -180,5 +193,7 @@ def install(
             verbose,
             build_options or [],
             install_options or [],
+            DIR,
+            install_dir,
         ),
     )

tests/test_editable.py

@@ -1,4 +1,5 @@
 import sys
+import textwrap
 from pathlib import Path
 
 import pytest
@@ -105,3 +106,53 @@ def test_cython_pxd(monkeypatch, tmp_path, editable, editable_mode, isolated):
         *editable_flag,
         ".",
     )
+
+
+@pytest.mark.compile()
+@pytest.mark.configure()
+@pytest.mark.integration()
+@pytest.mark.usefixtures("package_simplest_c")
+def test_install_dir(isolated):
+    isolated.install("pip>=23")
+    isolated.install("scikit-build-core")
+
+    settings_overrides = {
+        "build-dir": "build/{wheel_tag}",
+        "wheel.install-dir": "other_pkg",
+        "editable.rebuild": "true",
+    }
+    # Create a dummy other_pkg package to satisfy the import
+    other_pkg_src = Path("./src/other_pkg")
+    other_pkg_src.joinpath("simplest").mkdir(parents=True)
+    other_pkg_src.joinpath("__init__.py").write_text(
+        textwrap.dedent(
+            """
+            from .simplest._module import square
+            """
+        )
+    )
+    other_pkg_src.joinpath("simplest/__init__.py").touch()
+
+    isolated.install(
+        "-v",
+        *[f"--config-settings={k}={v}" for k, v in settings_overrides.items()],
+        "--no-build-isolation",
+        "-e",
+        ".",
+    )
+
+    # Make sure the package is correctly installed in the subdirectory
+    other_pkg_path = isolated.platlib / "other_pkg"
+    c_module_glob = list(other_pkg_path.glob("simplest/_module*"))
+    assert len(c_module_glob) == 1
+    c_module = c_module_glob[0]
+    assert c_module.exists()
+    # If `install-dir` was not taken into account it would install here
+    failed_c_module = other_pkg_path / "../simplest" / c_module.name
+    assert not failed_c_module.exists()
+
+    # Run an import in order to re-trigger the rebuild and check paths again
+    out = isolated.execute("import other_pkg.simplest")
+    assert "Running cmake" in out
+    assert c_module.exists()
+    assert not failed_c_module.exists()

tests/test_editable_redirect.py

@@ -44,6 +44,7 @@ def test_editable_redirect():
         build_options=[],
         install_options=[],
         dir=str(Path("/sitepackages")),
+        install_dir="",
     )
 
     assert finder.submodule_search_locations == process_dict_set(

tests/test_editable_unit.py

@@ -181,6 +181,7 @@ def test_navigate_editable_pkg(editable_package: EditablePackage, virtualenv: VE
         verbose=False,
         build_options=[],
         install_options=[],
+        install_dir="",
     )
 
     site_packages.joinpath("_pkg_editable.py").write_text(editable_txt)