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] Qhull is a private dependency #247

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

nim65s
Copy link
Contributor

@nim65s nim65s commented Nov 2, 2021

Hi,

This revert f5bb137 and remove the need for 5beff60.

On systems where Qhull is not installed, Qhull is build from the submodule. Then, if we leak the dependency to the CMake exports of hpp-fcl, all projects depending on hpp-fcl will require a system installation of Qhull, which was not available in the first place. In such cases, configuration will fail, because hpp-fcl won't be considered complete without qhull:

CMake Warning at /usr/share/cmake-3.16/Modules/CMakeFindDependencyMacro.cmake:47 (find_package):
By not providing "FindQhull.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "Qhull", but
CMake did not find one.

Could not find a package configuration file provided by "Qhull" with any of
the following names:

QhullConfig.cmake
qhull-config.cmake

Add the installation prefix of "Qhull" to CMAKE_PREFIX_PATH or set
"Qhull_DIR" to a directory containing one of the above files.  If "Qhull"
provides a separate development package or SDK, be sure it has been
installed.
Call Stack (most recent call first):
/opt/openrobots/lib/cmake/hpp-fcl/hpp-fclConfig.cmake:152 (find_dependency)
cmake/package-config.cmake:97 (find_package)
CMakeLists.txt:34 (ADD_PROJECT_DEPENDENCY)


CMake Warning at cmake/package-config.cmake:97 (find_package):
Found package configuration file:

/opt/openrobots/lib/cmake/hpp-fcl/hpp-fclConfig.cmake

but it set hpp-fcl_FOUND to FALSE so package "hpp-fcl" is considered to be
NOT FOUND.  Reason given by package:

hpp-fcl could not be found because dependency Qhull could not be found.

Call Stack (most recent call first):
CMakeLists.txt:34 (ADD_PROJECT_DEPENDENCY)

As hpp-fcl links to Qhull are private (https://github.com/humanoid-path-planner/hpp-fcl/blob/v1.7.8/src/CMakeLists.txt#L189-L198), I think we should not expose this dependency to packages depending on hpp-fcl.

Copy link
Contributor

@jmirabel jmirabel left a comment

Choose a reason for hiding this comment

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

Thanks.
Any news on releasing qhull in robotpkg ?

@jcarpent jcarpent merged commit 3b4c3e3 into coal-library:devel Nov 3, 2021
@nim65s nim65s deleted the topic/private-qhull branch November 3, 2021 08:44
@nim65s
Copy link
Contributor Author

nim65s commented Nov 3, 2021

It is already available in most major distributions. CMake is not finding it because it is an outdated version on 20.04 (2015.2) without CMake exports ; but on Arch with 2020.2, CMake exports are available and it is correctly found.

I don't remember if we already talk about this, but we can:

  1. do nothing and wait for debian/ubuntu to upgrade it ; meanwhile people needing it in hpp-fcl will build it
  2. add some CMake logic to find correctly 2015.2
  3. add a more recent version in robotpkg

I'm not sure 3 is the best way. What do you think ?

@jmirabel
Copy link
Contributor

jmirabel commented Nov 3, 2021

I wasn't aware of this. Then, don't add it in robotpkg.

The issue some years ago was that some libs were not in the binary package.

Is this qhull.cmake useful for option 2 ?

@nim65s
Copy link
Contributor Author

nim65s commented Nov 3, 2021

Looks a bit dusty, but this is surely a good basis

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