prevent GCC flags propagating to NVCC

[jasjuang]

With the merge of #2100 now all the compile definitions got carried along when target_link_libraries(${PROJECT_NAME} ${PCL_LIBRARIES}) is called, however we don't want these definitions to be propagated to CUDA files because NVCC can't recognize it. This PR prevents this problem.

[taketwo]

We are still keeping our CMake minimum requirement pretty low (at least for this release). We should double-check that this does not rely on recent CMake versions.

[taketwo]

I'm afraid there are a bunch of problems here:

• COMPILER_LANGUAGE was introduced in 3.3
• it is not supported by Visual Studio, so portable buildsystems should not use it
• CUDA became a first-class language in 3.8, before it probably counts as CXX

[jasjuang]

It is actually supported by Visual studio https://gitlab.kitware.com/cmake/cmake/merge_requests/1657 and https://gitlab.kitware.com/cmake/cmake/issues/17435.

If we want to support old cmake versions, maybe we can do a bunch of if else (if(CMAKE_VERSION VERSION_LESS 3.3)) etc to guard it? And for people that's still stuck on old cmakes they will have no way to avoid this error.

[taketwo]

Yes, I think we should add a version check, we can not just break the config file for the users of old CMake.

And for people that's still stuck on old cmakes they will have no way to avoid this error.

I hope this is not a wide-spread use case to link CUDA targets against PCL. If there will be people complaining, there is a workaround: link the "old way" against "raw" libraries ${PCL_${COMPONENT}_LIBRARY}.

@SergioRAgostinho what do you think?

[SergioRAgostinho]

I'm ok with the version check. I'm not sure I fully understand

I hope this is not a wide-spread use case to link CUDA targets against PCL. If there will be people complaining, there is a workaround: link the "old way" against "raw" libraries ${PCL_${COMPONENT}_LIBRARY}.

because of inexperience with compiling CUDA targets. But if there's a workaround I assume I don't need to worry much.

We are still keeping our CMake minimum requirement pretty low (at least for this release)

My next goal is C++14. I was hoping to bump the min CMake version to 3.1 to support CXX_STANDARD.

[taketwo]

It just struck me that before 3.8 a special command was used to create CUDA targets, and this command did not respect transitive properties anyway. So everything should be fine with the users of old CMake as long as we add guards.

My next goal is C++14. I was hoping to bump the min CMake version to 3.1 to support CXX_STANDARD.

I think we can lift minimum requirements for our dependencies. The reasoning would be: we are going for C++14 => oldest Ubuntu LTS that supports it is 16.04 => use this as a baseline. This means, among other things, that we can require CMake 3.5. But I think we can have a separate issue for deciding on the baseline and versions after the release.

[jasjuang]

@taketwo @SergioRAgostinho What is the decision for this PR? Does something like

if(CMAKE_VERSION VERSION_LESS 3.3)
set_target_properties(${pcl_component}
        PROPERTIES
          INTERFACE_COMPILE_DEFINITIONS "${definitions}"
          INTERFACE_COMPILE_OPTIONS "${PCL_COMPILE_OPTIONS}"
          INTERFACE_INCLUDE_DIRECTORIES "${PCL_${COMPONENT}_INCLUDE_DIRS}"
          INTERFACE_LINK_LIBRARIES "${PCL_${COMPONENT}_LINK_LIBRARIES}"
      )
else()
set_target_properties(${pcl_component}
        PROPERTIES
          INTERFACE_COMPILE_DEFINITIONS "${definitions}"
          INTERFACE_COMPILE_OPTIONS "$<$<COMPILE_LANGUAGE:CXX>:${PCL_COMPILE_OPTIONS}>"
          INTERFACE_INCLUDE_DIRECTORIES "${PCL_${COMPONENT}_INCLUDE_DIRS}"
          INTERFACE_LINK_LIBRARIES "${PCL_${COMPONENT}_LINK_LIBRARIES}"
      )
endif()

work?

[taketwo]

LGTM. And after the release we will lift CMake requirement and drop the first branch.

[SergioRAgostinho]

Now I fully got the "transitive properties" and why setting compiler language to cxx won't create a problem before or after 3.8.

@jasjuang I also believe that should solve it. Go for it.

[jasjuang]

@taketwo @SergioRAgostinho Just pushed the approved change. Please merge!

[SergioRAgostinho]

I think this closes it for this release then 👏 Now it's closing up.

[taketwo]

#2439 please also

[guteksan]

Hello,
It looks like you have fixed this problem in 1.9.0 release, however for some reason I need to use PCL in verson 1.8.1, and I have exactly this problem: I am linking my project with CUDA and PCL and flag '-fPIC' is passed to nvcc, which fails at it.

I tried the workaround mentioned above, use

target_link_libraries(xxx PUBLIC
    ${PCL_SEGMENTATION_LIBRARIES})

instead of

target_link_libraries(xxx PUBLIC
    ${PCL_LIBRARIES})

but this does not work (I think fPIC flag comes from segmentation module, other modules seem to be safe). Can you suggest another workaround how to link PCL with CUDA?