-
Notifications
You must be signed in to change notification settings - Fork 551
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
Fix build of downstream packages. #407
Conversation
LGTM. Thanks! 👍 |
The pkgconfig files are definitely still wrong as they have no way to expand the generator expressions. But I'll leave that to others as my interest is in the CMake config files. Could you please require that contributors (or Jenkins) do a more complex test than "hey, this library builds alone" before doing such substantial changes to the project? |
If I remember well, the Jenkins pipeline passed for the original PR. Another user also tested and said that it solved the issues with libnabo. Can you provide more info about your setup and how you tested it? Based on that feedback we could improve the tests. |
@peci1 I agree that those build modifications are annoying, but we don't do that really often. It's also rare that a library test for downstream dependencies, although I agree that |
@peci1 thanks for being quick to solve the dependency issues. We should converge soon ;) |
Yes, but the author of the PR also said that he only tested in build-space. Testing install-space is what's probably missing. It seems to me that a good way to test the integration would be to have some kind of integration builds which would build all the things down to e.g. ethzasl_icp_mapper (actually doing |
Could you also have a look at https://github.com/ethz-asl/laser_slam/tree/master/sensor_drivers/velodyne_assembler ? I get the following error:
|
@peci1 The above issue has been pointed out before by other colleagues. The Octree Grid filter might be missing a pthread depend, which can be fixed easily by linking against pthread. |
Okay. Is that something you're about to fix in libpm? |
@peci1 I'll take a look during the day and tag you in the PR. Out of curiosity: what is your build chain? |
We use ROS melodic and build ethzasl_icp_mapper with catkin tools. |
Additional questions:
Pull request: #417 |
gcc 7.5.0 (I think bionic default), building for amd64 (but soon we'll also need arm64) |
Fixed libpointmatcher_ros build in catkin workspace.