Pcl: CMake Policy CMP0074

Created on 10 Sep 2018  ยท  27Comments  ยท  Source: PointCloudLibrary/pcl

This affects all of your find* modules. See CMP0074 documentation, this policy is introduced in CMake 3.12+.

Naively, the fix would just be

if (POLICY CMP0074)
  cmake_policy(SET CMP0074 NEW)
endif()

I looked specifically at the FindFLANN.cmake file, and it does appear to respect FLANN_ROOT. Here's the relevant CMake (v3.12.1) output:

CMake Warning (dev) at CMakeLists.txt:295 (find_package):
  Policy CMP0074 is not set: find_package uses <PackageName>_ROOT variables.
  Run "cmake --help-policy CMP0074" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  Environment variable FLANN_ROOT is set to:

    /opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm

  For compatibility, CMake is ignoring the variable.
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Checking for module 'flann>=1.7.0'
--   Found flann, version 1.9.1
-- Found FLANN: /opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/lib/libflann_cpp.so (Required is at least version "1.7.0")
-- FLANN found (include: /opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/include, lib: optimized;/opt/spack/opt/spack/linu$
-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/lib/libflann_cpp.so;debug;/opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2
goiwqpdmeuoiwa2untqskm/lib/libflann_cpp.so)

For reference:

$ ls -R /opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/lib/
/opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/lib/:
libflann_cpp_s.a  libflann_cpp.so@  libflann_cpp.so.1.9@  libflann_cpp.so.1.9.1*  libflann_s.a  libflann.so@  libflann.so.1.9@  libflann.so.1.9.1*  pkgconfig/

/opt/spack/opt/spack/linux-fedora27-x86_64/gcc-7.3.1/flann-1.9.1-3qqgbuypp2goiwqpdmeuoiwa2untqskm/lib/pkgconfig:
flann.pc

I did a little snooping on Flann specifically, and that module _appears_ to be respecting FLANN_ROOT, but it's also worth mentioning that my flann.pc _also_ shows up in $PKG_CONFIG_PATH, so while the find_{path,library} are seemingly respecting FLANN_ROOT, it's unclear to me if anything here actually needs to change -- pkg-config // cmake // find modules are an area of CMake I'm not super comfortable with...

Other find modules are probably affected, I don't have time to check them all right now. This is a low priority issue, but the reason there is no pull request attached here is because a more thorough investigation of your vendored find modules needs to take place to actually set CMP0074 to NEW.

cmake

Most helpful comment

I think we should commit to OLD. Even if you get a response by Sunday, we don't want to rework finder and config scripts in a rush and squeeze it in the upcoming release. There is a lot of potential for subtle bugs that can only be discovered by many people trying to use PCL on many systems.

All 27 comments

We have 19 finder scripts, so this will take some effort to review. Definitely not before the 1.9.0 release.

Yes, this is very low priority! I doubt many people are even using 3.12 yet :wink:

Mac users probably are. Homebrew keeps things up to date. vcpkg users probably as well. Honestly it's mostly the Linux users who get stuck on old versions. ๐Ÿ˜… But I definitely don't want to touch this before releasing.

This warning, and possibly other like this one, but for different targets, are generated by the Find<package>.cmake shipped with PCL because PCLConfig.cmake configures FLANN_ROOT (and many many other <package>_ROOT variables, in particular if PCL_ALL_IN_ONE_INSTALLER is true).

By my understanding, the new policy basically claims that <package>_ROOT variables are now reserved to CMake as hint paths to look for packages configuration and Find<package>.cmake files invoking find_package().

Having a quick look at PCLConfig.cmake, it seems to me that PCL needs quite some changes to comply with this policy. For examples FindFLANN.cmake uses FLANN_ROOT and ENV{FLANN_ROOT} to look directly for the headers and shared/static library.

For the time being, to remove the warning one could just enforce the OLD behaviour that is the default one anyway.

To completely address this issue, we need to check and refactor the variables with suffix _ROOT and eventually set the policy to NEW to the whole project.

I was wanting to focus on c++14 for the next release but seems likes reworking CMake might be more important. I'll ping you all for help after the release if you don't mind. It would be good to assemble a little task force.

To completely address this issue, we need to check and refactor the variables with suffix _ROOT and eventually set the policy to NEW to the whole project.

This is a really good point. I asked about this on the cpplang slack and one of the CMake devs told me setting *_ROOT in the config file is discouraged. His words:

basically provide the necessary find modules themselves, but don't cache the results of your find calls when installing.

I will stew on this. At the very least, the currently cached values should probably just get a PCL_XXX_ROOT (prefix PCL_) to satisfy the policy. If the user setup an environment variable XXX_ROOT then presumably this environment variable will persist. If it's a configure argument, though, it will lead to trouble for them down the road (they'd need to provide the same -DXXX_ROOT="..." to the project consuming pcl)?

I don't get the point behind this advice. I absolutely want PCLConfig to find exactly the same dependencies as were used during PCL build.

I will stew on this

Could you explain this expression for non-native speakers here? :)

Likely means that he will reflect on the topic for a while.

--

On Tuesday, Sep 11, 2018 at 7:42 PM, Sergey Alexandrov

I don't get the point behind this advice. I absolutely want PCLConfig to find exactly the same dependencies as were used during PCL build.

>

I will stew on this

Could you explain this expression for non-native speakers here? :)

โ€”
You are receiving this because you commented.
Reply to this email directly, view it on GitHub (https://github.com/PointCloudLibrary/pcl/issues/2425#issuecomment-420378361), or mute the thread (https://github.com/notifications/unsubscribe-auth/ABoUVgdDKVIXRbS6Wu72CBtIBERUrPaoks5uaAP_gaJpZM4Wgp9y).

Yes, I meant I need to think about it more. Origin: stew (like beef stew) takes a few hours to cook, you kind of just let it sit there for a while :)

I will probably reach out to the mailing list to solicit advice, the slack conversation was brief and I didn't include much context. The person who told this to me is a cmake king whose advice is generally law ;)

For the time being, to remove the warning one could just enforce the OLD behaviour that is the default one anyway.

I'm open to enforcing this for now.

This is a really good point. I asked about this on the cpplang slack and one of the CMake devs told me setting *_ROOT in the config file is discouraged.

๐Ÿ‘

At the very least, the currently cached values should probably just get a PCL_XXX_ROOT (prefix PCL_) to satisfy the policy.

Exactly, if we want to "minimize" refactoring and maintain the same logic and notation, prefixing PCL_ could be a good way to go.

I don't get the point behind this advice. I absolutely want PCLConfig to find exactly the same dependencies as were used during PCL build.

Yes, and it will be available to still do that ๐Ÿ˜„ The policy just says that <package>_ROOT is now reserved to CMake, just, as an example, like <package>_DIR is. As a consequence, there is the need of a refactoring, not a complete change in the configuration of PCL project.

I will probably reach out to the mailing list to solicit advice, the slack conversation was brief and I didn't include much context. The person who told this to me is a cmake king whose advice is generally law ;)

Ok thanks, just let us know. I'm confident that we understaood the problem and that a refactoring is all we need. Anyway, I will also discuss this topic here in my workplace with a big CMake contributor and let ou know asap.

I'm open to enforcing this for now.

Ok, I'll try to think about this. I need to understand whether it is better to enforce the policy project wise, or just Find<package>.cmake-wise with localized policy(PUSH) and policy(POP).

Yes, and it will be available to still do that smile The policy just says that _ROOT is now reserved to CMake, just, as an example, like _DIR is

Yes, the policy does not talk about caching. But the quote from the CMake dev reads: "don't cache the results of your find calls when installing."

Yes, the policy does not talk about caching. But the quote from the CMake dev reads: "don't cache the results of your find calls when installing."

To me that means that you should not have in a configure file that is consumed by external projects targeting PCL, any variable in the form of <package>_ROOT. That for PCL implies that you can just have the same Config and scripts, but you have to refactor all <package>_ROOT variables in PCL_<package>_ROOT (for example).

I don't know whether we are saying the same thing or not actually ahahah ๐Ÿคฃ

I believe we are saying different things and have different interpretations of what the guy meant. Thus :+1: if @svenevs will further clarify this through the mailing list.

The implication of the advice was entirely removing the *_ROOT variables from PCLConfig.cmake. The rename was a proposition to satisfy the policy.

I will reach out to the mailing list today and post back tomorrow if I get responses :) depending on responses I will likely just add a hot fix PR too set the policy to old for the imminent release.

Update: no bites (maybe I asked poorly). How imminent is the imminent release? Could we just commit to doing OLD on this policy on say Sunday if nobody responds on the list?

I think we should commit to OLD. Even if you get a response by Sunday, we don't want to rework finder and config scripts in a rush and squeeze it in the upcoming release. There is a lot of potential for subtle bugs that can only be discovered by many people trying to use PCL on many systems.

My ideas was to go with OLD now as well. The effort to adopt the new policy starts after that, alongside whatever we identify as "good practices which are currently not being enforced".

My two cents here: go for OLD with policy(PUSH) & policy(POP) and then tackle this issue with proper time and test after the release.

Any specific reason to use push/pop? We typically have:

if(POLICY xyz)
  cmake_policy(SET xyz OLD)
endif()

It is just a convenient way to isolate the code that needs a temporary cmake_policy and will be modified in the near future, as well as controlling the policy stack of the project. It may not be the case here, but if you set a policy project wide you, or contributors, may end up adding even more code that eventually will need to be updated when the policy will changed. policy(PUSH) and policy(POP) is used automatically when a policy is set in files calld by include() and find_package(), hence it may be implicitly call in our particular setting. As a strictly personal opinion, I also find it a little more under my control since a policy(PUSH) must be followed by a policy(POP) ๐Ÿ˜„.

It may not be the case here, but if you set a policy project wide you, or contributors, may end up adding even more code that eventually will need to be updated when the policy will changed.

This is interesting and desirable. Remains to understand the difference in effort from adopting the simple project wide setting vs the push/pop the policy wherever is needed.

@claudiofantacci I'm not sure that cmake_policy(PUSH) and cmake_policy(POP) make sense here. We're electing to OLD because sifting through the whole project to find out what may / may not need to change is a big task, we just want a band-aid. (To be very clear, as you will read, I am confused / asking for clarification on where you think the PUSH|POP should go).

From an old but relevant mailing list post

if(POLICY CMP<NNNN>)
  cmake_policy(PUSH)
  cmake_policy(SET CMP<NNNN> OLD)
endif()

# ...
# other code where the policy is set to OLD
# ...

if(POLICY CMP<NNNN>)
  cmake_policy(POP)
endif()

The problem is that in our case this means finding / checking every find_package call to see if we should OLD or NEW there, or alternatively doing a PUSH at the very beginning and a POP at the very end of the root CMakeLists.txt. Which is the same as just setting it to OLD at the top without the PUSH|POP, right?

Does this make sense / do people agree? This one is kind of confusing because (as stated) find_package does introduce a new "policy stack". From CMP0074:

This policy provides compatibility with projects that have not been updated to avoid using <PackageName>_ROOT variables for other purposes.

So um. Do we set it in PCLConfig.cmake too? That's the file that's 100% violating this. If I'm understanding how these call stacks actually work, I think we _do_ need to set it here. But I'm getting really turned around :cry:

@svenevs setting the policy project wide will not remove the warning for people targeting PCL within their project. The policy is not exported in the PCL configuration file if it set in the main CMakeLists.

Here is the output of git grep _ROOT (you can use the find feature of your browser to highlight the _ROOT substring of the following output):

$ git grep _ROOT
CHANGES.md:* Corrected PCL_ROOT in PCLConfig.cmake
CHANGES.md:* define PCL_ROOT environment variable using the NSIS installer
PCLConfig.cmake.in:    set(BOOST_ROOT "${PCL_ROOT}/3rdParty/Boost")
PCLConfig.cmake.in:    set(EIGEN_ROOT "${PCL_ROOT}/3rdParty/Eigen")
PCLConfig.cmake.in:  elseif(NOT EIGEN_ROOT)
PCLConfig.cmake.in:    get_filename_component(EIGEN_ROOT "@EIGEN_INCLUDE_DIRS@" ABSOLUTE)
PCLConfig.cmake.in:    set(QHULL_ROOT "${PCL_ROOT}/3rdParty/Qhull")
PCLConfig.cmake.in:  elseif(NOT QHULL_ROOT)
PCLConfig.cmake.in:    get_filename_component(QHULL_ROOT "@QHULL_INCLUDE_DIRS@" PATH)
PCLConfig.cmake.in:  if(NOT OPENNI_ROOT AND ("@HAVE_OPENNI@" STREQUAL "TRUE"))
PCLConfig.cmake.in:  if(NOT OPENNI2_ROOT AND ("@HAVE_OPENNI2@" STREQUAL "TRUE"))
PCLConfig.cmake.in:  if(NOT ENSENSO_ROOT AND ("@HAVE_ENSENSO@" STREQUAL "TRUE"))
PCLConfig.cmake.in:  if(NOT davidSDK_ROOT AND ("@HAVE_DAVIDSDK@" STREQUAL "TRUE"))
PCLConfig.cmake.in:    set(FLANN_ROOT "${PCL_ROOT}/3rdParty/Flann")
PCLConfig.cmake.in:  elseif(NOT FLANN_ROOT)
PCLConfig.cmake.in:    get_filename_component(FLANN_ROOT "@FLANN_INCLUDE_DIRS@" PATH)
PCLConfig.cmake.in:    if(EXISTS "${PCL_ROOT}/3rdParty/VTK/lib/cmake")
PCLConfig.cmake.in:      set(VTK_DIR "${PCL_ROOT}/3rdParty/VTK/lib/cmake/vtk-@VTK_MAJOR_VERSION@.@VTK_MINOR_VERSION@" CACHE PATH "The directory containing VTKConfig.cmake")
PCLConfig.cmake.in:      set(VTK_DIR "${PCL_ROOT}/3rdParty/VTK/lib/vtk-@VTK_MAJOR_VERSION@.@VTK_MINOR_VERSION@" CACHE PATH "The directory containing VTKConfig.cmake")
PCLConfig.cmake.in:# PCLConfig.cmake is installed to PCL_ROOT/cmake
PCLConfig.cmake.in:  get_filename_component(PCL_ROOT "${PCL_DIR}" PATH)
PCLConfig.cmake.in:# PCLConfig.cmake is installed to PCL_ROOT/share/pcl-x.y
PCLConfig.cmake.in:  get_filename_component(PCL_ROOT "${CMAKE_CURRENT_LIST_DIR}/../.." ABSOLUTE)
PCLConfig.cmake.in:if(EXISTS "${PCL_ROOT}/include/pcl-${PCL_VERSION_MAJOR}.${PCL_VERSION_MINOR}/pcl/pcl_config.h")
PCLConfig.cmake.in:  set(PCL_INCLUDE_DIRS "${PCL_ROOT}/include/pcl-${PCL_VERSION_MAJOR}.${PCL_VERSION_MINOR}")
PCLConfig.cmake.in:  set(PCL_LIBRARY_DIRS "${PCL_ROOT}/@LIB_INSTALL_DIR@")
PCLConfig.cmake.in:  if(EXISTS "${PCL_ROOT}/3rdParty")
PCLConfig.cmake.in:  endif(EXISTS "${PCL_ROOT}/3rdParty")
PCLConfig.cmake.in:elseif(EXISTS "${PCL_ROOT}/include/pcl/pcl_config.h")
PCLConfig.cmake.in:  set(PCL_INCLUDE_DIRS "${PCL_ROOT}/include")
PCLConfig.cmake.in:  set(PCL_LIBRARY_DIRS "${PCL_ROOT}/lib")
PCLConfig.cmake.in:  if(EXISTS "${PCL_ROOT}/3rdParty")
PCLConfig.cmake.in:  endif(EXISTS "${PCL_ROOT}/3rdParty")
PCLConfig.cmake.in:else(EXISTS "${PCL_ROOT}/include/pcl-${PCL_VERSION_MAJOR}.${PCL_VERSION_MINOR}/pcl/pcl_config.h")
PCLConfig.cmake.in:endif(EXISTS "${PCL_ROOT}/include/pcl-${PCL_VERSION_MAJOR}.${PCL_VERSION_MINOR}/pcl/pcl_config.h")
cmake/Modules/FindEigen.cmake:    HINTS ${PC_EIGEN_INCLUDEDIR} ${PC_EIGEN_INCLUDE_DIRS} "${EIGEN_ROOT}" "$ENV{EIGEN_ROOT}"
cmake/Modules/FindFLANN.cmake:          HINTS ${PC_FLANN_INCLUDEDIR} ${PC_FLANN_INCLUDE_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
cmake/Modules/FindFLANN.cmake:             HINTS ${PC_FLANN_LIBDIR} ${PC_FLANN_LIBRARY_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
cmake/Modules/FindFLANN.cmake:             HINTS ${PC_FLANN_LIBDIR} ${PC_FLANN_LIBRARY_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
cmake/Modules/FindG2O.cmake:    ${G2O_ROOT}/lib/Debug
cmake/Modules/FindG2O.cmake:    ${G2O_ROOT}/lib
cmake/Modules/FindG2O.cmake:    $ENV{G2O_ROOT}/lib/Debug
cmake/Modules/FindG2O.cmake:    $ENV{G2O_ROOT}/lib
cmake/Modules/FindG2O.cmake:    ${G2O_ROOT}/lib/Release
cmake/Modules/FindG2O.cmake:    ${G2O_ROOT}/lib
cmake/Modules/FindG2O.cmake:    $ENV{G2O_ROOT}/lib/Release
cmake/Modules/FindG2O.cmake:    $ENV{G2O_ROOT}/lib
cmake/Modules/FindG2O.cmake:  ${G2O_ROOT}
cmake/Modules/FindG2O.cmake:  $ENV{G2O_ROOT}
cmake/Modules/FindG2O.cmake:  ${G2O_ROOT}/include
cmake/Modules/FindG2O.cmake:  $ENV{G2O_ROOT}/include
cmake/Modules/FindGLEW.cmake:      $ENV{GLEW_ROOT}/include
cmake/Modules/FindGLEW.cmake:      $ENV{GLEW_ROOT}/lib
cmake/Modules/FindGTSAM.cmake:# GTSAM_DIR (or GTSAM_ROOT): (Optional) The install prefix OR source tree of gtsam (e.g. /usr/local or src/gtsam)
cmake/Modules/FindGTSAM.cmake:# Use GTSAM_ROOT or GTSAM_DIR equivalently
cmake/Modules/FindGTSAM.cmake:if(GTSAM_ROOT AND NOT GTSAM_DIR)
cmake/Modules/FindGTSAM.cmake:  set(GTSAM_DIR "${GTSAM_ROOT}")
cmake/Modules/FindGtest.cmake:    HINTS "${GTEST_ROOT}" "$ENV{GTEST_ROOT}"
cmake/Modules/FindGtest.cmake:    HINTS "${GTEST_ROOT}" "$ENV{GTEST_ROOT}"
cmake/Modules/FindOpenNI.cmake:            HINTS ${PC_USB_10_INCLUDEDIR} ${PC_USB_10_INCLUDE_DIRS} "${USB_10_ROOT}" "$ENV{USB_10_ROOT}"
cmake/Modules/FindOpenNI.cmake:               HINTS ${PC_USB_10_LIBDIR} ${PC_USB_10_LIBRARY_DIRS} "${USB_10_ROOT}" "$ENV{USB_10_ROOT}"
cmake/Modules/FindOpenNI.cmake:                "${OPENNI_ROOT}"
cmake/Modules/FindOpenNI.cmake:                "$ENV{OPENNI_ROOT}"
cmake/Modules/FindOpenNI.cmake:                   "${OPENNI_ROOT}"
cmake/Modules/FindOpenNI.cmake:                   "$ENV{OPENNI_ROOT}"
cmake/Modules/FindOpenNI2.cmake:            HINTS ${PC_USB_10_INCLUDEDIR} ${PC_USB_10_INCLUDE_DIRS} "${USB_10_ROOT}" "$ENV{USB_10_ROOT}"
cmake/Modules/FindOpenNI2.cmake:               HINTS ${PC_USB_10_LIBDIR} ${PC_USB_10_LIBRARY_DIRS} "${USB_10_ROOT}" "$ENV{USB_10_ROOT}"
cmake/Modules/FindQhull.cmake:          HINTS "${QHULL_ROOT}" "$ENV{QHULL_ROOT}" "${QHULL_INCLUDE_DIR}"
cmake/Modules/FindQhull.cmake:             HINTS "${QHULL_ROOT}" "$ENV{QHULL_ROOT}"
cmake/Modules/FindQhull.cmake:             HINTS "${QHULL_ROOT}" "$ENV{QHULL_ROOT}"
cmake/Modules/NSIS.template.in:  InstallDir "@CPACK_NSIS_INSTALL_ROOT@\@CPACK_PACKAGE_INSTALL_DIRECTORY@"
cmake/Modules/NSIS.template.in:  !define MUI_STARTMENUPAGE_REGISTRY_ROOT "SHCTX" 
cmake/Modules/NSIS.template.in:  WriteRegExpandStr ${env_hklm} PCL_ROOT "$INSTDIR"
cmake/Modules/NSIS.template.in:  DeleteRegValue ${env_hklm} PCL_ROOT
cmake/Modules/NSIS.template.in:  StrCmp "$INSTDIR" "@CPACK_NSIS_INSTALL_ROOT@\@CPACK_PACKAGE_INSTALL_DIRECTORY@" 0 +2
cmake/Modules/NSIS.template.in:      StrCpy $INSTDIR "@CPACK_NSIS_INSTALL_ROOT@\@CPACK_PACKAGE_INSTALL_DIRECTORY@"
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(BOOST_ROOT "${Boost_INCLUDE_DIR}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(BOOST_ROOT "${BOOST_ROOT}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(EIGEN_ROOT "${EIGEN_INCLUDE_DIRS}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(QHULL_ROOT "${QHULL_INCLUDE_DIRS}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIRS}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(VTK_ROOT "${VTK_DIR}" PATH)
cmake/pcl_all_in_one_installer.cmake:    get_filename_component(VTK_ROOT "${VTK_ROOT}" PATH)
cmake/pcl_all_in_one_installer.cmake:        get_filename_component(VTK_ROOT "${VTK_ROOT}" PATH)
cmake/pcl_all_in_one_installer.cmake:            DIRECTORY "${${DEP}_ROOT}/"
cmake/pcl_cpack.cmake:    set(CPACK_NSIS_INSTALL_ROOT "$PROGRAMFILES64")
cmake/pcl_cpack.cmake:    set(CPACK_NSIS_INSTALL_ROOT "$PROGRAMFILES32")
cmake/pcl_utils.cmake:      set(INCLUDE_INSTALL_ROOT
cmake/pcl_utils.cmake:      set(INCLUDE_INSTALL_ROOT "include") # Android, don't put into subdir
cmake/pcl_utils.cmake:    set(INCLUDE_INSTALL_DIR "${INCLUDE_INSTALL_ROOT}/pcl")
cmake/uninstall_target.cmake.in:message(STATUS "Uninstalling \"@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@\"")
cmake/uninstall_target.cmake.in:if(EXISTS "@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@")
cmake/uninstall_target.cmake.in:        ARGS "-E remove_directory \"@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@\""
cmake/uninstall_target.cmake.in:            "Problem when removing \"@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@\"")
cmake/uninstall_target.cmake.in:else(EXISTS "@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@")
cmake/uninstall_target.cmake.in:        "Directory \"@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@\" does not exist.")
cmake/uninstall_target.cmake.in:endif(EXISTS "@CMAKE_INSTALL_PREFIX@/@INCLUDE_INSTALL_ROOT@")
doc/advanced/content/distcc.rst:      -- ROS_ROOT /opt/ros/diamondback/ros
doc/tutorials/content/bspline_fitting.rst:  $ ./bspline_fitting ${PCL_ROOT}/test/bunny.pcd
doc/tutorials/content/building_pcl.rst:Let's say PCL is placed under /PATH/TO/PCL, which we will refer to as PCL_ROOT::
doc/tutorials/content/building_pcl.rst:  $ cd $PCL_ROOT
doc/tutorials/content/building_pcl.rst:Under ${PCL_ROOT}/cmake/Modules there is a list of FindXXX.cmake files
doc/tutorials/content/building_pcl.rst:environment variable named **XXX_ROOT** to find headers and libraries.
doc/tutorials/content/building_pcl.rst:* **BOOST_ROOT**: for boost libraries with value `C:/Program Files/boost-1.4.6` for instance
doc/tutorials/content/building_pcl.rst:* **CMINPACK_ROOT**: for cminpack with value `C:/Program Files/CMINPACK 1.1.13` for instance
doc/tutorials/content/building_pcl.rst:* **QHULL_ROOT**: for qhull with value `C:/Program Files/qhull 6.2.0.1373` for instance
doc/tutorials/content/building_pcl.rst:* **FLANN_ROOT**: for flann with value `C:/Program Files/flann 1.6.8` for instance
doc/tutorials/content/building_pcl.rst:* **EIGEN_ROOT**: for eigen with value `C:/Program Files/Eigen 3.0.0` for instance
doc/tutorials/content/sources/vfh_recognition/FindFLANN.cmake:          HINTS ${PC_FLANN_INCLUDEDIR} ${PC_FLANN_INCLUDE_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
doc/tutorials/content/sources/vfh_recognition/FindFLANN.cmake:             HINTS ${PC_FLANN_LIBDIR} ${PC_FLANN_LIBRARY_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
doc/tutorials/content/sources/vfh_recognition/FindFLANN.cmake:       HINTS ${PC_FLANN_LIBDIR} ${PC_FLANN_LIBRARY_DIRS} "${FLANN_ROOT}" "$ENV{FLANN_ROOT}"
gpu/octree/src/cuda/octree_host.cu:        const static int MAX_LEVELS_PLUS_ROOT = 11;
gpu/octree/src/cuda/octree_host.cu:        int paths[MAX_LEVELS_PLUS_ROOT];          
gpu/octree/src/cuda/radius_search.cu:                MAX_LEVELS_PLUS_ROOT = 11,
surface/src/3rdparty/opennurbs/opennurbs_zlib.cpp:#define OPENNURBS_ZLIB_OUTPUT_ROOT_DIR "."
surface/src/3rdparty/opennurbs/opennurbs_zlib.cpp:#pragma comment(lib, "\"" OPENNURBS_ZLIB_OUTPUT_ROOT_DIR "/" OPENNURBS_CONFIGURATION_DIR "/" OPENNURBS_ZLIB_FILE_NAME "\"")

As you can see all the relevant <package>_ROOT variable violating CMP0074 are set in pcl_all_in_one_installer.cmake and PCLConfig.cmake.in (the latter exported to target PCL by external projects).
My suggestion is to add the policy just in this two files:

  • by my understanding PCLConfig.cmake.in is not used at PCL build time, but only by external projects targeting PCL. This will remove the warning for external projects.
  • I'm no expert of pcl_all_in_one_installer.cmake, but I see it is include()d in
git grep pcl_all_in_one_installer
CMakeLists.txt:include("${PCL_SOURCE_DIR}/cmake/pcl_all_in_one_installer.cmake")

thus we should add the policy also in this file.

Since both files will be (most likely) calld by include() and find_package() calls, we don't need to add policy(PUSH/POP) because is handeld automatically by CMake in that way. My suggestion was to use that anyway just to avoid any possible misuse, error, wrong integration by other people ๐Ÿ˜„

I have finally learned exactly what we should do for coping with the OLD case and accommodating consuming projects that may or may not be NEW. I will add a PR either tonight or tomorrow, just wanted to give an update that this will be "fixed" for the next release :smile:

Ping @svenevs. Did the world swallow you in unexpected affairs?

Fixed by #2454

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dsravankumar1987 picture dsravankumar1987  ยท  4Comments

SiWil picture SiWil  ยท  4Comments

Passworteingeben picture Passworteingeben  ยท  3Comments

dooxe picture dooxe  ยท  3Comments

SergioRAgostinho picture SergioRAgostinho  ยท  4Comments