xtensor-stack / xtensor-python

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

CMake: find_packages #139

Open ax3l opened 6 years ago

ax3l commented 6 years ago

Hi,

we should improve the way we are currently looking for xtensor-python dependencies:

find_package(xtl REQUIRED)
message(STATUS "Found xtl: ${xtl_INCLUDE_DIRS}/xtl")
find_package(xtensor REQUIRED)
message(STATUS "Found xtensor: ${xtensor_INCLUDE_DIRS}/xtensor")
find_package(pybind11 REQUIRED)
message(STATUS "Found pybind11: ${pybind11_INCLUDE_DIRS}/pybind11")
find_package(NumPy REQUIRED)
message(STATUS "Found numpy: ${NUMPY_INCLUDE_DIRS}")

I think we could add the following:

SylvainCorlay commented 6 years ago

Probably. Indeed, it is also the right time to do so as we just updated everything to modern cmake.

xtl 0.4.0 xtensor 0.15.0 (but I would recommend 0.15.1)

Regarding pybind11, we have been conservative about updating it as there has been some breakages between 2.0 and 2.1... We should probably test the latest versions as they come out.

ax3l commented 6 years ago

Regarding pybind11, the upcoming 2.3.0 release looks very promising as well: the descriptions can now fully be build in C++11 mode (previously only in C++14 compiles and otherwise generated at runtime).

Nevertheless, 2.2.2 is only 4 days old, so there might be some time until it gets a version assigned.

JohanMabille commented 6 years ago

@ax3l Just a question about the indirect dependency, if C depends on B that depends on A, don't we have to do find_package(A) in the CMakeLists.txt of C?

In the case of the xtensor's stack I thought that we could remove the find_package because the headers are all in the same directory (that is, %PREFIX%/include); besides, I remember experiencing complaints about headers not found in such a configuration (but that was for another project), that were solved by adding the find_package(A). But I migh have missed something in the CmakeLists.txt of B and A.

Asked differently, is it possible with cmake to handle transitive dependencies without find_package for the inderect dependencies, and this whatever the location of the headers is?

ax3l commented 6 years ago

Just a question about the indirect dependency, if C depends on B that depends on A, don't we have to do find_package(A) in the CMakeLists.txt of C?

With modern CMake one can finally forget about this and only account for the dependencies one faces directly. This was always especially troublesome for header-only libs, since we as developers had to explain users "you only depend on B but... you don't know that... but B depends on A so you depend on B and A at compile time".

Asked differently, is it possible with cmake to handle transitive dependencies without find_package for the inderect dependencies, and this whatever the location of the headers is?

Yes, that's exactly it. Just expose these indirect dependencies properly as PUBLIC dependencies to B when "building" and installing B and its config package. and they (A) will be forwarded to any CMake target that will then depend on B.

ax3l commented 6 years ago

side note: but in the case of xtensor-python you don't depend solely indirect on xtl but also directly on xtl: https://github.com/QuantStack/xtensor-python/blob/0.17.0/include/xtensor-python/pycontainer.hpp#L34

But xsimd is such a solely indirect dependency via xtensor.

JohanMabille commented 6 years ago

@ax3l thank you for the clarification! I agree with you, xtensor-python directly depends on xtl, this question was meant to improve my cmake skill ;)

ax3l commented 6 years ago

Well, I was just thinking loud as well to verify I got the dependencies right ;)