Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editable installs now respect the value of wheel.install-dir #867

Merged
merged 11 commits into from
Aug 20, 2024
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions src/scikit_build_core/build/_editable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion src/scikit_build_core/build/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@
) -> 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)

Check warning on line 63 in src/scikit_build_core/build/wheel.py

View check run for this annotation

Codecov / codecov/patch

src/scikit_build_core/build/wheel.py#L62-L63

Added lines #L62 - L63 were not covered by tests
editable_txt = editable_redirect(
modules=modules,
installed=installed,
Expand All @@ -67,6 +69,7 @@
verbose=settings.editable.verbose,
build_options=build_options,
install_options=install_options,
install_dir=settings.wheel.install_dir,
)

wheel.writestr(
Expand Down
19 changes: 17 additions & 2 deletions src/scikit_build_core/resources/_editable_redirect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]] = {}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -180,5 +193,7 @@ def install(
verbose,
build_options or [],
install_options or [],
DIR,
install_dir,
),
)
51 changes: 51 additions & 0 deletions tests/test_editable.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sys
import textwrap
from pathlib import Path

import pytest
Expand Down Expand Up @@ -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()
1 change: 1 addition & 0 deletions tests/test_editable_redirect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions tests/test_editable_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading