xtensor-stack / xtensor-python

Python bindings for xtensor
BSD 3-Clause "New" or "Revised" License
347 stars 58 forks source link

expose `${NUMPY_INCLUDE_DIRS}` in `install` #191

Closed colinfang closed 5 years ago

colinfang commented 5 years ago

At the moment

target_include_directories(xtensor-python INTERFACE 
       $<BUILD_INTERFACE:${XTENSOR_PYTHON_INCLUDE_DIR}>
       $<BUILD_INTERFACE:${pybind11_INCLUDE_DIRS}>
       $<BUILD_INTERFACE:${NUMPY_INCLUDE_DIRS}>
       $<INSTALL_INTERFACE:include>

So after I make install xtensor-python, my project that links to xtensor-python cannot find NumPy library.

Can we add $<INSTALL_INTERFACE:${NUMPY_INCLUDE_DIRS}> so that the numpy header includes are automatically propagated.

Otherwise users have to copy FindNumPy.cmake from xtensor-python. and find_package(NumPy) themselves.

JohanMabille commented 5 years ago

Hi,

Adding absolute paths to the include directories of its dependency to the INTERFACE_INCLUDE_DIRECTORIES of xtensot-python target would prevent the package from being relocatable. Numpy can be installed in many locations depending on your system / package manager, and we cannot make any assumption about it in the cmake.

See Cmake's documentation for more detail.

colinfang commented 5 years ago

Could you please spare some time to shed some light on "relocatable package"? I don't quite understand the meaning of "relocateable" here. I read your link as well as cmake package

When a user does cmake .. on xtensor-python, it needs to figure out the numpy location. It happens before make install. Can we not assume at that point the numpy location is frozen, hence it is valid to hardcode it? The doc implies the path for dependencies at package runtime might be different from build time. How is it possible?

Apart from that, what about the idea of

I learned it from here

JohanMabille commented 5 years ago

relocatable package means you can install the package everywhere, the absolute paths will be computed on demand depending on some prefix. To build such a package, the files in your package must not contain any absolute path.

When a user does cmake .. on xtensor-python, it needs to figure out the numpy location. It happens before make install.

Only if you build and install from souce. If you install xtensor-python via a package manager, you need to find Numpy after the installation of xtensor-python.

Can we not assume at that point the Numpy location is frozen, hence it is valid to hardcode it?

No we cannot. Suppose I want to install different versions of Python, then I need to install Numpy twice and then I have 2 possible locations for Numpy; on Linux, the location of you packages depends on your distribution, and is totally different from Windows; if I have different conda environment, I may have xtensor-python in the root environment, and different versions of Numpy in different environments, etc etc.

Package managers are good at copying files, not at editing them. Thus, when you create a package using cmake, you run the make install install command in a sandbox environment to generate all the files required for the installation. Then an archive is created from these files and additional files specific to the package manager. When installing the package, the package manager simply copy these files to the right location. "The right location" can be everything, so the files in your package must contain relative paths only, otherwise you may search files in a different location from the one they have been installed.

When you run a find_package, CMake populates the xxx_INCLUDE_DIRS variable with absolute paths, that's why it must not be added to the install target.

What you suggest with targets and find_dependency is what we do with our packages (you don't have to search for xtensor for instance), however this is modern CMake and requires the dependencies to be built with CMake (or some cmake files creating these targets and all their properties as if they have been generated with CMake). It's not the case for Numpy, and FindNumpy.cmake does not export such targets, thus the need to manually find it in project depending on xtensor-python.

EDIT: fixed imprecisions.

colinfang commented 5 years ago

Thank you very much for the thorough explanation. I didn't know make install is also used by a package manager.

AntoinePrv commented 4 years ago

Hello, @JohanMabille wouldn't it possible to ship the file FindNumPy.cmake in the installation and call it from xtensor-pythonConfig ?