★ wanayoo — archive 1999 https://github.com/xtensor-stack/xtensor-python/issues/139Nouvelle recherche | Portail wanayoo
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

CMake: find_packages #139

Open
ax3l opened this issue Feb 11, 2018 · 7 comments
Open

CMake: find_packages #139

ax3l opened this issue Feb 11, 2018 · 7 comments

Comments

@ax3l
Copy link

@ax3l ax3l commented Feb 11, 2018

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:

  • add min-versions to each dependency in the find_package call
  • expose in the readme that numpy is a direct dependency, it's not documented; which version range is known to work?
  • xtl is not only an indirect dependency of xtensor (which would mean we can remove the find_package and take automatically added PUBLIC dependency of the xtensor dependency) but also a direct dependency used in xtensor-python code: expose it in the readme with its version range as well
  • docs: does it run with pybind11 2.2.2 as well? :)
@SylvainCorlay
Copy link
Member

@SylvainCorlay SylvainCorlay commented Feb 11, 2018

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
Copy link
Author

@ax3l ax3l commented Feb 11, 2018

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
Copy link
Member

@JohanMabille JohanMabille commented Feb 12, 2018

@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
Copy link
Author

@ax3l ax3l commented Feb 12, 2018

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
Copy link
Author

@ax3l ax3l commented Feb 12, 2018

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
Copy link
Member

@JohanMabille JohanMabille commented Feb 12, 2018

@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
Copy link
Author

@ax3l ax3l commented Feb 12, 2018

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

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.