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

Update versions of dependencies #124

Merged
merged 1 commit into from
Dec 16, 2020

Conversation

davidbrochart
Copy link
Member

No description provided.

@davidbrochart
Copy link
Member Author

@tdegeus maybe you have some thoughts about the issue with highfive=2.2.2?

@tdegeus
Copy link
Member

tdegeus commented Dec 10, 2020

It is high time that I update to the new API, because most, if not all, code here can go. But in terms of issue, could you tell me what the issue is?

@davidbrochart
Copy link
Member Author

We get the following error with highfive=2.2.2:

CMake Error: install(EXPORT "xtensor-io-targets" ...) includes target "xtensor-io" which requires target "libdeps" that is not in any export set.
CMake Error in CMakeLists.txt:
  export called with target "xtensor-io" which requires target "libdeps" that
  is not in any export set.

@tdegeus
Copy link
Member

tdegeus commented Dec 10, 2020

Ah, HighFive's CMake support is not to my liking. I tried to entirely rewrite it, but that gave too much resistance of the maintainers: BlueBrain/HighFive#257 . It would be great if you could report the issue in the library, there are quite some issues now, so maybe a critical mass can push for some overhaul.
As a work-around, what if you try to add libdeps, which is where HDF5 is configured, to xtensor-io's target? And if that doesn't work, what about making our own temporary target libdeps then when HighFive is searched that target will not be overwritten. One then just has to add HDF5 to our target here.

@tdegeus
Copy link
Member

tdegeus commented Dec 10, 2020

It's create here : https://github.com/BlueBrain/HighFive/blob/master/CMake/HighFiveTargetDeps.cmake . So one has to just cherry-pick from it.

CMakeLists.txt Outdated
@@ -248,7 +248,7 @@ endif()
include(GNUInstallDirs)
include(CMakePackageConfigHelpers)

install(TARGETS xtensor-io
install(TARGETS xtensor-io libdeps
EXPORT ${PROJECT_NAME}-targets)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the way to go. libdeps should probably be installed by highfive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this would cause xtensor-io to install libdeps as part of its cmake, and therefore libdeps to be installed in xtensor-io's cmake.

@davidbrochart
Copy link
Member Author

We'll update highfive in another PR, as it requires some changes upstream.

@SylvainCorlay SylvainCorlay merged commit 26048ef into xtensor-stack:master Dec 16, 2020
@davidbrochart davidbrochart deleted the update_deps branch December 16, 2020 22:16
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

Successfully merging this pull request may close these issues.

3 participants