-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
build: hpp-fcl/2.4.4 #25219
base: master
Are you sure you want to change the base?
build: hpp-fcl/2.4.4 #25219
Conversation
This comment has been minimized.
This comment has been minimized.
recipes/hpp-fcl/all/conanfile.py
Outdated
copy(self, "LICENSE", src=self.source_folder, dst=os.path.join(self.package_folder, "licenses")) | ||
cmake = CMake(self) | ||
cmake.install() | ||
rmdir(self, os.path.join(self.package_folder, "lib", "cmake")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upstream is providing a lot of things in this lib/cmake dir.
Choosing to drop this is fine, but please understand that it will be your responsibility to ensure that nothing is broken following that.
In other words, upstream will not like to receive bug reports from conan users because of this choice.
Ref. "best practices" note in https://docs.conan.io/2/reference/conanfile/methods/package_info.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to remove this line since it was shamelessly copied from other examples 😑
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not removing it is not enough for it to be used I guess. I think you should either:
- keep that rmdir, but then you need to teach conan how to generate properly
hpp-fcl-config.cmake
with all required dependencies ; - or keep that directory, but then you need to teach conan not to generate any
hpp-fcl-config.cmake
, and use the upstream-generatedlib/cmake/hpp-fclConfig.cmake
file instead, as it already contains the right dependencies.
For now, I can't build your package:
/home/gsaurel/.conan2/p/b/hpp-fba0c52d2f5443/p/include/hpp/fcl/data_types.h:41:10: fatal error: 'Eigen/Core' file not found
41 | #include <Eigen/Core>
| ^~~~~~~~~~~~
This is because a not-good-enough hpp-fcl-config.cmake
file was generated by conan. You can see this by adding :
--- a/recipes/hpp-fcl/all/test_package/CMakeLists.txt
+++ b/recipes/hpp-fcl/all/test_package/CMakeLists.txt
@@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.10)
project(test_package LANGUAGES CXX)
find_package(hpp-fcl REQUIRED CONFIG)
+message(STATUS "hpp-cfcl config file was found on ${hpp-fcl_DIR}")
add_executable(${PROJECT_NAME} test_package.cpp)
target_link_libraries(${PROJECT_NAME} PRIVATE hpp-fcl::hpp-fcl)
Which display: -- hpp-cfcl config file was found on /tmp/conan-center-index/recipes/hpp-fcl/all/test_package/build/gcc-13-x86_64-gnu17-release/generators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😞
I'm sorry...
I have to give up on this since even after reading up on the docs and trying things, I wasn't able to pull this out 😕
https://docs.conan.io/2/examples/tools/cmake/cmake_toolchain/use_package_config_cmake.html
This comment has been minimized.
This comment has been minimized.
9ba4a53
to
6abcc1d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 069ef81hpp-fcl/2.4.4@#f7e7f20a83db79e957e1b6a35e0f0cf5
|
@nim65s, thanks a lot for your in-depth review! |
(it might not be easy to find my latest comment on a message based on a deleted line, so here it is: #25219 (comment)) |
Conan v1 pipeline ❌Failure in build 5 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. Conan v2 pipeline ❌
The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping Failure in build 5 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
Unfortunately, I overestimated my skills to put the recipe together for this package... Not being able to make it work, I had to use a workaround with a manually pre-compiled I encourage anyone who can/wants to finish this recipe - to either start with a new PR or to continue the work here... |
Summary
The new recipe: hpp-fcl/2.4.4
Motivation
Closes #25161
Addresses: coal-library/coal#620
Details
This PR is an attempt to put together a recipe and collaborate with the upstream team for a problem resolution:
My tested package from this recipe has issues when included in another C++ project/package. I created a conan-style recipe's test from the hpp-fcl project's README.md and it has the same failure signature:
Error details
Build environment: Dockerfile