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 install do not use wheel.install-dir when editable.rebuild is trigerred #866

Closed
psalvaggio opened this issue Aug 19, 2024 · 7 comments · Fixed by #867
Closed

Editable install do not use wheel.install-dir when editable.rebuild is trigerred #866

psalvaggio opened this issue Aug 19, 2024 · 7 comments · Fixed by #867
Labels
good first issue Good for newcomers

Comments

@psalvaggio
Copy link
Contributor

I am trying to make a dual-purpose package for use with conda that supports consumption both via Python and C++ (via CMake find_package()). I also would like to support editable installs. My conda recipe looks like:

  1. pip install -no-deps
  2. Symlink the relevant things (include/ lib64/cmake/$PROJECT_NAME, my libraries) into $CONDA_PREFIX so my project is easily consumable by other things in the venv.

I have 2 avenues to make this work:

  1. Use wheel.install-dir and make my CMake agnostic to scikit-build-core. This is my preferred option. However, I am running into an issue with the editable rebuilds not respecting install-dir. When I trigger a rebuild, I see all of my install targets go to site-packages instead of site-packages/<project_name>.
  2. Make my CMake aware of SKBUILD and use that to set a prefix on my install locations. This can be made to work, but I then run into an issue with step 2 of my conda recipe, where symlinks into the CONDA_PREFIX will result in a broken CMake config due to the extra <project_name> in the install paths. I also tried to install to SKBUILD_DATA_DIR directly, bypassing the symlinking step, however, this result in /tmp paths showing up in my installed CMake config files (target-release.cmake generated via install(EXPORT).

Is there a way to make editable installs respect the wheel.install-dir option?

@LecrisUT
Copy link
Collaborator

LecrisUT commented Aug 19, 2024

  1. Use wheel.install-dir and make my CMake agnostic to scikit-build-core. This is my preferred option. However, I am running into an issue with the editable rebuilds not respecting install-dir. When I trigger a rebuild, I see all of my install targets go to site-packages instead of site-packages/<project_name>.

Can you elaborate on this. I have a project setting tool.scikit-build.wheel.install-dir="spglib" and it correctly populates the CMake installed files inside site-packages/spglib. How does your CMake workflow work?

I have also been considering whether to allow my CMake projects to install the python bindings outside of the SKBUILD environment, and ultimately I have decided against that because I couldn't find a way to populate the metadata files that would allow to pip uninstall cleanly, and there is no equivalent paths to the GnuInstallDir that are relocatable.

@psalvaggio
Copy link
Contributor Author

Sure... my CMake looks roughly like this:

install(
    TARGETS ${PROJECT_NAME}
    EXPORT ${PROJECT_NAME}Targets
    PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}
    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
)

if (SKBUILD EQUAL "2")
  install(TARGETS ${PY_BINDINGS} DESTINATION .)
endif()

# standard CMake config boilerplate
# install(EXPORT) goes to ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}

when I do this and I pip install --no-deps --no-build-isolation -e ., things initially look fine and everything is where I expect. However, when I change something in my source code and trigger a rebuild, things get installed to site-packages instead of site-packages/<project_name>. My pyproject.toml looks like:

[build-system]
requires = ["scikit-build-core"]
build-backend = "scikit_build_core.build"

[tool.scikit-build]
editable.rebuild = true
build-dir = "build"
wheel.install-dir = "<projecT_name<"
sdist.exclude = [
   "*.cpp",
   "*.c",
   "*.h",
   "CMakeLists.txt",
]

@LecrisUT
Copy link
Collaborator

LecrisUT commented Aug 19, 2024

Great thanks for the elaboration. Indeed I managed to replicate this issue, and indeed this is a bug.

The bug is quite obvious:

DIR = os.path.abspath(os.path.dirname(__file__))

result = subprocess.run(
["cmake", "--install", ".", "--prefix", DIR, *self.install_options],

(DIR must be made editable, or an additional parameter of INSTALL_DIR should be passed to better expand the path there. DIR in that context points to site-packages)

@LecrisUT LecrisUT changed the title Editable install issue with install-dir Editable install do not use wheel.install-dir when editable.rebuild is trigerred Aug 19, 2024
@LecrisUT
Copy link
Collaborator

@psalvaggio would you want to write a patch for this? The other relevant part is

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)
editable_txt += f"\n\ninstall({arguments_str})\n"

where you need to alter editable_txt to include the wheel.install-dir in the generated _<pkg>_editable.py file

@LecrisUT LecrisUT added the good first issue Good for newcomers label Aug 19, 2024
@psalvaggio
Copy link
Contributor Author

I'm not familiar with this codebase or how this feature works at all. If you think this is a simple fix, I could probably figure it out. I'll see if I can carve out some time in the next couple of nights to try this out.

@henryiii
Copy link
Collaborator

Oops, wrong copy paste.

@henryiii
Copy link
Collaborator

Should be fixed in 0.10.4!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants