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

Support setting install-dir to wheel_dir instead of derivatives of wheel_dir/platlib #901

Open
Jacobfaib opened this issue Sep 18, 2024 · 11 comments

Comments

@Jacobfaib
Copy link

I am migrating a project over from scikit-build (SKB) to scikit-build-core (SKBC). Our project is set up into 2 parts:

  1. The C++ cmake files and cmakelists. They only concern themselves with the C++ libs and are completely oblivious to others.
  2. The Python cmake files. They internally call the C++ cmake build, then build stuff on top of it.

In SKB, this worked. SKB would call cmake --install, and the C++ cmake files would first populate

$prefix/lib/
$prefix/include/
$prefix/bin/
$prefix/share/

then the Python cmakelists would install and populate my_python_module/ as well, resulting in a "install" dir consisting of

...
$prefix/my_python_module/

The important point:

$prefix/bin/
$prefix/lib/
$prefix/lib/python3.12/site-packages/my_python_module/
$prefix/share/

This is currently not possible in SKBC


However, SKBC does not allow this, because installs everything under platlib:

$prefix/platlib/lib/
$prefix/platlib/include/
$prefix/platlib/bin/
$prefix/platlib/share/
$prefix/platlib/my_python_module/

which means all of those directories now end up under site-packages. And you can't fudge this by using wheel.install-dir = ".." because thats not allowed.

I cannot use SKBUILD_DATA_DIR or SKBUILD_INCLUDE_DIR etc in the C++ cmake files. They must be written completely scikit-build agnostic, as they must be usable without scikit-build driving the compilation.

@Jacobfaib
Copy link
Author

Or, perhaps better yet, make the whatever generates install_dir into a python function (just like scikit-build did), so that folks can monkey-patch it:

# custom_build_backend.py
from scikit_build_core.build import *
import scikit_build_core as _skbcore

def my_install_dir() -> Path:
  ...

_skbcore.install_dir = my_install_dir

def build_wheel(...):
  ...

@henryiii
Copy link
Collaborator

henryiii commented Sep 19, 2024

This is currently not possible in SKBC

Yes, it is, though it's an experimental feature (due to the syntax). Set:

[tool.scikit-build]
experimental = true
wheel.install-dir = "/data"

This is SKBUILD_DATA_DIR, which is $prefix.

However, I don't think it's correct to construct $prefix/lib/python3.12/site-packages/my_python_module/. platlib is configurable. Some systems have lib64, for example. You need to write the Python package to the actual wheel platlib directory. And you should (ideally) not have relative links between scripts, bin, platlib, metadata, or headers.

If you wanted to install into the true root (not platlib or data), that might not be possible, but that's very much a wheel-specific structure and it's fine to require some setup in CMake for that.

I cannot use SKBUILD_DATA_DIR or SKBUILD_INCLUDE_DIR etc in the C++ cmake files. They must be written completely scikit-build agnostic, as they must be usable without scikit-build driving the compilation.

You can use if(DEFINED SKBUILD), that would allow you to avoid the experimental setting. You can also wrap your CMakeLists.txt from another CMakeLists.txt designed for scikit-build-core. It's not reasonable to assume you can't write any CMake code for a CMake build backend.1

into a python function

Scikit-build-core expects you to use CMake if CMake can be used. The plugins (setuptools and hatching, or making your own) also are customizable in Python. We eventually will likely have some sort of hook system as well, but the goal is to make sure it does things that are truly hard or impossible to do from the CMake side and static configuration.

Footnotes

  1. Forgot you already had a two step build, you should be able to do this in the Python CMake files part.

@henryiii
Copy link
Collaborator

What happens, out of curiosity, if you set CMAKE_INSTALL_PREFIX to SKBUILD_DATA_DIR inside the Python CMake files? Does the cmake --install command override this?

@LecrisUT
Copy link
Collaborator

Is it possible to break the project in the main C++ project and a python bindings project? Then you could cmake configure/build/install the main project into $prefix and the pip install the python bindings with the cmake file there having find_package. That way you can satisfy both pythonic paths needed for building the wheel, as well as custom installations like in conda, spack or whatever environment you would have there.

@Jacobfaib
Copy link
Author

Yes, it is, though it's an experimental feature (due to the syntax). Set:

Ah I did not know that. Will try that out.

platlib is configurable. Some systems have lib64, for example. You need to write the Python package to the actual wheel platlib directory. And you should (ideally) not have relative links between scripts, bin, platlib, metadata, or headers.

Yep, the C++ cmake files already handle this. They have essentially

if(SYSTEM_USES_LIB64)
  set(lib_install_dir lib64)
else()
  set(lib_install_dir lib)
endif()

install(TARGETS my_lib DESTINATION ${lib_install_dir})

You can use if(DEFINED SKBUILD), that would allow you to avoid the experimental setting.

True, but I'd rather not. The way our cmake lists are set up is that any language bindings (e.g. python) sit on top of the C++ build. The C++ cmake lists are correctly constructed to install into C++ directories. We have many language bindings, so the language bindings cmakelists are where the special casing for each language should happen.

Does the cmake --install command override this?

Yes, because SKBC passes --prefix /foo/bar/baz to the install command. The runtime option overrides any hardcoded CMAKE_INSTALL_PREFIX.


Is it possible to break the project in the main C++ project and a python bindings project?

That is exactly how it is structured. The workflow is approximately as follows:

# Build the C++ libraries, using CppLibCmakeLists.txt
$ cmake -S . -B cpp_build
$ cmake --build cpp_build
...
# Build and install the python bindings using PythonLibCMakeLists.txt
$ CMAKE_ARGS="-Dcpp_build_ROOT=./cpp_build" pip install . # calls into SKBC

The problem here is that SKBC will install both the python project and C++ project

Then you could cmake configure/build/install the main project into $prefix and the pip install the python bindings with the cmake file there having find_package.

I would rather not split these for several reasons:

  1. Doing the cmake install of the C++ bits separately from pip install means that pip uninstall would not remove the afterwards. And since we are only installing the C++ bits for the python bindings, this seems weird. Furthermore, cmake -- in their infinite wisdom -- provide no way to uninstall software once it has been installed. So resetting the environment would require manually finding and deleting all those files.
  2. Asking our users to manually perform a C++ install just to install python bindings also seems counterintuitive. If the python bindings need the C++ installed, then our python build system should just know how to do that.

@LecrisUT
Copy link
Collaborator

I am a bit confused on the workflow then. If you are purely concerned about installing within the pip environment, than you should just install everything with wheel.install-dir = "my_package" so that $CMAKE_INSTALL_LIBDIR will expand to site_packages/my_package/lib. You need to patch the RPATH to make it runnable after installation, and for windows it's another fun process. I have that design implemented in this iteration of spglib.

I would rather not split these for several reasons:

1. Doing the cmake install of the C++ bits separately from `pip install` means that `pip uninstall` would not remove the afterwards. And since we are only installing the C++ bits for the python bindings, this seems weird. Furthermore, `cmake` -- in their infinite wisdom -- provide no way to uninstall software once it has been installed. So resetting the environment would require manually finding and deleting all those files.

2. Asking our users to manually perform a C++ install just to install python bindings also seems counterintuitive. If the python bindings need the C++ installed, then our python build system should just know how to do that.

My suggestion there was about supporting esoteric packaging environments like conda and spack, but for plain pip install I just bundle everything self-consistently. More specifically I use add_subdirectory(../) to include the main library without having to split the build processes.

@Jacobfaib
Copy link
Author

so that $CMAKE_INSTALL_LIBDIR will expand to site_packages/my_package/lib

I need the C++ libs to be installed to $prefix/lib though, not under site-packages/my_package/lib because...

supporting esoteric packaging environments like conda and spack

...we use the wheel we build with pip to populate our conda packages :)

More specifically I use add_subdirectory(../) to include the main library without having to split the build processes.

We do that as well, which is the cause of the problem.

In any case, I have come up with a solution. I wish it weren't so hacky but needs must I guess...

# pyproject.toml
[tool.scikit-build]
experimental = true

[tool.scikit-build.wheel]
install-dir = "/data"
# CppLibCmakeLists.txt

install(TARGETS my_cpp_lib DESTINATION ${lib_or_lib64})
install(FILES my_cpp_headers DESTINATION include)
# PythonLibCMakeLists.txt

install(TARGETS my_cython_libs DESTINATION "../platlib/${my_package_name})

@Jacobfaib
Copy link
Author

I believe a more permanent solution would be (and this is only a humble suggestion :)) if SKBC did not use the wheel dir structure of platlib, include, lib, etc. but rather had something like

lib/ # maps to $prefix/lib
include/ # maps to $prefix/include
bin/ # maps to $prefix/bin
py_prefix/
| - platlib/ # maps to site-packages (${SKBUILD_PLATLIB_DIR})
| - lib/ # maps to Python include dir (${SKBUILD_HEADERS_DIR})
| - bin/ # maps to Python bin dir (${SKBUILD_SCRIPTS_DIR})

The result of this is that:

  1. All of the usual GNUInstallDirs options for C++ installs would work without modification (they more or less map 1-1 to the special SKBUILD_{...}_DIR options).
  2. PythonLibsCmakeLists.txt could use the specific SKBUILD_{...}_DIR to install their stuff into the python-specific directories.

@LecrisUT
Copy link
Collaborator

...we use the wheel we build with pip to populate our conda packages :)

But if it is for conda packaging than it should work by default. Just separate the project into subprojects and use find_package. I have a similar design in the current iteration of conda spglib. It does not do extra installs.

Also, please use GNUInstallDirs variables (e.g. CMAKE_INSTALL_*) we may make use of those to make installation simpler, particularly for windows environments.

@LecrisUT
Copy link
Collaborator

I believe a more permanent solution would be (and this is only a humble suggestion :)) if SKBC did not use the wheel dir structure of platlib, include, lib, etc. but rather had something like

I think it should be the opposite. When packaging for PyPI, you can make $prefix become site-packages/my_package and export this as and entry-point cmake.root so that any subsequent consumers can use it. In the case of splitting the package into main and python bindings, there are better designs to adopt. See how I handle spglib and there I have the build especially configured so that it is compatible with Fedora, PyPI, conda, spack, homebrew, and vcpkg packaging.

Can you share the link for the main package and the conda package, and the log you got where you got the C++ package installed? I also used cmake.define.Spglib_ROOT=%{__cmake_builddir} to pull in the export() artifacts and I have double-checked that there are no duplicate C libraries installed under site-packages. So I suspect there is something funky in the CMakeLists.txt that should be investigated.

@henryiii
Copy link
Collaborator

Not following everything, but a few quick comments:

Yep, the C++ cmake files already handle this. They have essentially

No! You can't do this. The value of SYSTEM_USES_LIB64 comes from when you make the wheel, but this might not be true when the wheel is installed! "Manylinux" wheels work on many Linux's, including both variants of this setting, all from one wheel. If you use this, you will have a subtly broken wheel that will not work on some valid systems, and will produce different wheels on different systems.

And that's just the most common reason for a variation here, but I believe your Python install can completely customize where these directories are. You should not assume a fixed structure. Specifically, you cannot make assumptions about the relative paths between $SKBUILD_BIN_DIR, $SKBUILD_PLATLIB_DIR, etc.

That's why we strongly encourage placing everything inside platlib/<package_name>, and then using importlib.resources to access it. If you have a standalone statically linked binary, you can place it in $SKBUILD_BIN_DIR, otherwise you need to make an project.scripts wrapper for it.

I believe you can set the GNUInstallDirs variables to the SKBUILD paths in your Python wrapper.

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

No branches or pull requests

3 participants