Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove push/pop in PCLConfig #2726

Closed
taketwo opened this issue Dec 19, 2018 · 5 comments · Fixed by #3431
Closed

Remove push/pop in PCLConfig #2726

taketwo opened this issue Dec 19, 2018 · 5 comments · Fixed by #3431
Labels
module: cmake
Milestone
pcl-1.10.0

Comments

@taketwo
Copy link
Member

taketwo commented Dec 19, 2018

We can get rid of the push/pop, especially if we are setting policy 74.

Originally posted by @claudiofantacci in #2454 (comment)
@taketwo
Copy link
Member Author

taketwo commented May 5, 2019

Implementing this will hopefully fix #3062.

@taketwo taketwo added this to the pcl-1.10.0 milestone Jun 26, 2019
@taketwo taketwo mentioned this issue Oct 18, 2019
Closed
11 tasks
@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Oct 20, 2019 •
edited
Loading

From what I can tell, if we simply remove the push/pop policy our PCLConfig.cmake will start producing warnings in consumer projects. As a test I did the following:

  1. Compiled and installed pcl master (done with CMake 3.10, what comes in 18.04)
  2. Installed a local version of CMake 3.16.0-rc2 (3.12+ required to trigger policy 0074)
  3. Created a CMake project with the following content
PROJECT(pcl-config-test)

CMAKE_MINIMUM_REQUIRED(VERSION 3.5)

find_package(PCL)
message("PCL_INCLUDE_DIRS: ${PCL_INCLUDE_DIRS}")
message("PCL_LIBRARIES: ${PCL_LIBRARIES}")
Here's the current output without the push/pop approach (Click to expand) 
[...]
CMake Warning (dev) at /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:252 (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.

  CMake variable FLANN_ROOT is set to:

    /usr

  For compatibility, CMake is ignoring the variable.
Call Stack (most recent call first):
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:307 (find_flann)
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:541 (find_external_library)
  CMakeLists.txt:5 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Checking for module 'flann'
--   Found flann, version 1.9.1
-- Found FLANN: /usr/lib/x86_64-linux-gnu/libflann_cpp.so
-- The imported target "vtkRenderingPythonTkWidgets" references the file
   "/usr/lib/x86_64-linux-gnu/libvtkRenderingPythonTkWidgets.so"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/cmake/vtk-6.3/VTKTargets.cmake"
but not all the files it references.

-- The imported target "vtk" references the file
   "/usr/bin/vtk"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/cmake/vtk-6.3/VTKTargets.cmake"
but not all the files it references.

[...]

CMake Warning (dev) at /home/sergio/Development/3rdparty/CMake/install/3.16.0-rc2/share/cmake-3.16/Modules/FindOpenGL.cmake:275 (message):
  Policy CMP0072 is not set: FindOpenGL prefers GLVND by default when
  available.  Run "cmake --help-policy CMP0072" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  FindOpenGL found both a legacy GL library:

    OPENGL_gl_LIBRARY: /usr/lib/x86_64-linux-gnu/libGL.so

  and GLVND libraries for OpenGL and GLX:

    OPENGL_opengl_LIBRARY: /usr/lib/x86_64-linux-gnu/libOpenGL.so
    OPENGL_glx_LIBRARY: /usr/lib/x86_64-linux-gnu/libGLX.so

  OpenGL_GL_PREFERENCE has not been set to "GLVND" or "LEGACY", so for
  compatibility with CMake 3.10 and below the legacy GL library will be used.
Call Stack (most recent call first):
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:329 (find_package)
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:541 (find_external_library)
  CMakeLists.txt:5 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.
[...]
and with (Click to expand) 
- The imported target "vtkRenderingPythonTkWidgets" references the file
   "/usr/lib/x86_64-linux-gnu/libvtkRenderingPythonTkWidgets.so"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/cmake/vtk-6.3/VTKTargets.cmake"
but not all the files it references.

-- The imported target "vtk" references the file
   "/usr/bin/vtk"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/cmake/vtk-6.3/VTKTargets.cmake"
but not all the files it references.

[...] 

CMake Warning (dev) at /home/sergio/Development/3rdparty/CMake/install/3.16.0-rc2/share/cmake-3.16/Modules/FindOpenGL.cmake:275 (message):
  Policy CMP0072 is not set: FindOpenGL prefers GLVND by default when
  available.  Run "cmake --help-policy CMP0072" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  FindOpenGL found both a legacy GL library:

    OPENGL_gl_LIBRARY: /usr/lib/x86_64-linux-gnu/libGL.so

  and GLVND libraries for OpenGL and GLX:

    OPENGL_opengl_LIBRARY: /usr/lib/x86_64-linux-gnu/libOpenGL.so
    OPENGL_glx_LIBRARY: /usr/lib/x86_64-linux-gnu/libGLX.so

  OpenGL_GL_PREFERENCE has not been set to "GLVND" or "LEGACY", so for
  compatibility with CMake 3.10 and below the legacy GL library will be used.
Call Stack (most recent call first):
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:329 (find_package)
  /home/sergio/Development/3rdparty/pcl/install/dev/share/pcl-1.9/PCLConfig.cmake:541 (find_external_library)
  CMakeLists.txt:5 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found OpenGL: /usr/lib/x86_64-linux-gnu/libOpenGL.so

It is doing it's job of suppressing the warning related to _ROOT variables. This makes me thing that removing the push/pop is not the way to proceed.

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Oct 20, 2019 •
edited
Loading

I've found the issue. Entering a CMake function or macro effectively creates a new policy stack (well not really, but when in comes to push/pop it's equivalent). Here's a minimal failing example.

PROJECT(pcl-config-test)
CMAKE_MINIMUM_REQUIRED(VERSION 3.5)

cmake_policy(PUSH)

macro(my_macro)
  cmake_policy(POP)
  return()
endmacro()


my_macro()
cmake_policy(POP)

This means we cannot return() out of PCLConfig from within any of the functions or macros.

@SergioRAgostinho
Copy link
Member

Turns out you were right, but I'm not sure if for the right reasons. After a tip from the CMake mailing list and rereading the docs.

CMake also manages a new entry for scripts loaded by include() and find_package() commands except when invoked with the NO_POLICY_SCOPE option (see also policy CMP0011).

Test: CMakeLists.txt

project(pcl-config-test)
cmake_minimum_required(VERSION 3.5)

cmake_policy(SET CMP0074 OLD)
cmake_policy(GET CMP0074 POLICY_STATUS)
message(STATUS "POLICY_STATUS: ${POLICY_STATUS}")

find_package(PCL)

cmake_policy(GET CMP0074 POLICY_STATUS)
message(STATUS "POLICY_STATUS: ${POLICY_STATUS}")

My mock PCLConfig.cmake

if(POLICY CMP0074)
  # TODO: update *_ROOT variables to be PCL_*_ROOT or equivalent.
  # CMP0074 directly affects how Find* modules work and *_ROOT variables.  Since
  # this is a config file that will be consumed by parent projects with (likely)
  # NEW behavior, we need to push a policy stack.
  cmake_policy(SET CMP0074 NEW)
endif()

cmake_policy(GET CMP0074 POLICY_STATUS)
message(STATUS "POLICY_STATUS: ${POLICY_STATUS}")

The output

-- The C compiler identification is GNU 7.4.0
-- The CXX compiler identification is GNU 7.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- POLICY_STATUS: OLD
-- POLICY_STATUS: NEW
-- POLICY_STATUS: OLD
-- Configuring done
-- Generating done

Push/pop free! PR following up.

@SergioRAgostinho SergioRAgostinho mentioned this issue Oct 21, 2019
Merged
@taketwo
Copy link
Member Author

taketwo commented Oct 29, 2019

Thanks for doing the research, I'm glad you've come back out of this rabbit hole alive 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module: cmake
Projects
None yet
Milestone
pcl-1.10.0
Development

Successfully merging a pull request may close this issue.

Remove policy push/pop from "PCLConfig.cmake" file SergioRAgostinho/pcl
2 participants
@taketwo @SergioRAgostinho