Create CMake imported targets for header-only modules

[aslze]

The original PCLConfig.cmake.in creates imported targets only for compiled PCL modules. This little change adds imported targets for header-only modules too (actually INTERFACE targets), such as 2d geometry, etc. Then PCL can be used like:

find_package(PCL COMPONENTS common 2d)
add_executable(program main.cpp)
target_link_libraries(prog pcl_2d)

Here, pcl_2d is a header only component, which depends on pcl_common (so it will be automatically added). You can also omit the COMPONENTS, and all will be available.

[aslze]

So it seems Build.Ubuntu-16-04 fails. (the test "feature_gasd_estimation" fails). But I'm pretty sure this is not caused by this PR as Ionly changed PCLConfig.cmake.in which does not affect actual code, only the way PCL is imported in client apps.

[taketwo]

Yes, feature_gasd_estimation has been failing for some days now, it's unrelated to your PR.

[taketwo]

Hi, thanks for submitting a PR.

As far as I can see, the actual difference between header-only and regular library is in how the add_library is called and in that a regular library has IMPORTED_LOCATION properties setup. The rest (includes, compile options and definitions, etc) are the same. Therefore, I was wondering wouldn't it be more concise if the else() for header-only was inserted at line 617, and only had one command inside, add_library(${pcl_component} INTERFACE IMPORTED) . And then, the script would proceed to set remaining target properties (lines 622-650), no matter what the type of the module is. What do you think?

[aslze]

Hi,

Yes, I think that's right. As you say INTERFACE targets are similar to normal targets except the LOCATION and IMPLIB, which are given in the first block (up to line 616).

OTOH, I noticed that this new way of importing and linking PCL only works for an installed tree, not for a build tree. For build tree importing, it lacks the directory of pcl_config.h. I'll push a change that fixes this. And also the change you propose.

[taketwo]

For build tree importing, it lacks the directory of pcl_config.h. I'll push a change that fixes this.

Thanks, good catch.

Everything looks good to me now in terms of functionality. Could you please fix indentation in lines 623-658? Also please use spaces instead of tabs.

[aslze]

Done.

[kunaltyagi]

I noticed that there's a check for cmake version_less 3.3 but PCL has a minimum required version of 3.5. Maybe some cmake can be removed as part of this or another PR.

[aslze]

I noticed that there's a check for cmake version_less 3.3 but PCL has a minimum required version of 3.5. Maybe some cmake can be removed as part of this or another PR.

Yes, I didn't know that. Actually, when I saw that check I wanted to test the script on an "older" CMake and downloaded 3.2. When I tried to configure, it said "Minimum CMake required is 3.5". Oops. So I guess some lines can be removed.

[taketwo]

When I tried to configure, it said "Minimum CMake required is 3.5".

Can you clarify what did you try to configure? PCL itself, or a downstream project that consumes PCL?

I'm a bit fuzzy here, but I had the impression that it's often the case that there are two minimum required CMake versions. One is to be able to configure the project, and the second is to be able to consume the project config. But again, not sure if we need to support this in PCL.

[aslze]

Sorry, I mean I tried to configure another project that imports PCL. And the requirement is in line 16 of PCLConfig.cmake.

And, yes, there is sometimes that distinction. If you export targets using CMake's own mechanisms (it generates a XxxConfig.cmake with everything) it normally writes a minimum version required to import the generated config file. A version which is usually older (even 2.x).

I'm not sure I would worry about this. CMake 3.5 is not too recent.

[taketwo]

OK. Then I propose to squash-merge this and have a follow-up PR that removes the block for the old CMake version.