Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add PCL package #1891
Add PCL package #1891
Changes from all commits
78029bd
07d0522
fd43a92
8774793
32fef4e
8b74bde
2c755e9
843cf55
9050c8d
6b6ddb5
3cc5c64
3c21176
13125af
2347012
2f0a1ec
2145466
d636240
2741a6e
c272820
53da8bb
8fd2f64
5646161
d03b370
db4eb9a
9129ede
388be41
5143ee9
605c465
1652b16
9a39937
6aa6ef9
0bf1936
dae7be3
33499eb
00a4b9e
41911a9
eba3a02
1730606
414e203
19cb75d
a71774f
2235aaa
634a2f8
e0d7968
9382f89
c6fef55
8c9555e
d1b16fe
de87a86
ec5056e
d7cc565
a52af55
74580bc
8476322
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 tested on macOS. This line needed to prevent fail.
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.
After that getting error. Not sure what is wrong.
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.
@sakrist The error on that picture is different, it shows that the package version that you passed to
conan create
command does not exist inconandata.yml
file. Please, share your entire build log as pure text.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.
True. This is new error I get after fix in first comment.
oh, I try build 1.12.0 but available only 1.12.1
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.
Successfully compiled 1.12.1 package on MacBook M1 Pro.
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.
PCL
creates several pkgconfig files (one per component) and one CMake config file (PCLConfig.cmake
) with a lot of non namespaced imported targets: common kdtree octree search sample_consensus filters 2d geometry io features ml segmentation surface registration keypoints tracking recognition stereo.Here are inter dependencies extracted from
PCLConfig.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.
Conan center index packages do not support custom PKG config or cmake files.
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.
Yes, but
package_info
should contain all necessary stuff to mimic those files through pkg_config, cmake_find_package and cmake_find_package_multi generators.Even without using components, libs should be listed in proper order.
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.
here is a starter to help you:
Maybe some values are wrong, didn't check exact lib names or pkgconfig files. You'll have also to properly define external dependencies of each component (and eventually system libs dependencies if any depending on os).
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.
Components is still marked as an experimental feature. Are there any other conan center index packages doing this?
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.
Yes, it's experimental yet, but it should not break, probably it should be increased. Yes, we have a lot of recipe using it (thanks to @SpaceIm): https://github.com/conan-io/conan-center-index/search?l=Python&q=components&type=Code
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.
Attempted to get as close as I can to PCL's target names, however it creates targets without namespaces along the lines of
PCL_SURFACE_LIBRARIES
etc. I've mimicked this with components, but conan's targets will be namespaced soPCL::PCL_SURFACE_LIBRARIES
.I don't see a way in conan/cmake to distinguish between a non-namespaced target called
PCL_SURFACE_LIBRARIES
and a plain cmake variable calledPCL_SURFACE_LIBRARIES
that contains the list of libraries to link to.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.
Conan cmake generator, primarily, focus on the targets approach since that is considered the best practice now a days.
We can not create and variables (yet).
I think the components work done is complete for a first PR, feel free to mark this resolved!
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.
We have now several recipes which are able to provide non-namespaced targets. Take a look at OpenCV recipe for a complex example with targets.