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.

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

Suppress cmake warnings for pcl modules #4431

Merged
merged 2 commits into from
Apr 21, 2021

Conversation

mvieth
Copy link
Member

@mvieth mvieth commented Oct 2, 2020

See issue #3680
Example of suppressed warnings:

CMake Warning (dev) at /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:273 (message):
  The package name passed to `find_package_handle_standard_args`
  (PCL_FEATURES) does not match the name of the calling package (PCL).  This
  can lead to problems in calling code that expects `find_package` result
  variables (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /usr/local/share/pcl-1.11/PCLConfig.cmake:619 (find_package_handle_standard_args)
  CMakeLists.txt:7 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:273 (message):
  The package name passed to `find_package_handle_standard_args`
  (PCL_SAMPLE_CONSENSUS) does not match the name of the calling package
  (PCL).  This can lead to problems in calling code that expects
  `find_package` result variables (e.g., `_FOUND`) to follow a certain
  pattern.
Call Stack (most recent call first):
  /usr/local/share/pcl-1.11/PCLConfig.cmake:619 (find_package_handle_standard_args)
  CMakeLists.txt:7 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:273 (message):
  The package name passed to `find_package_handle_standard_args`
  (PCL_FILTERS) does not match the name of the calling package (PCL).  This
  can lead to problems in calling code that expects `find_package` result
  variables (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /usr/local/share/pcl-1.11/PCLConfig.cmake:619 (find_package_handle_standard_args)
  CMakeLists.txt:7 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:273 (message):
  The package name passed to `find_package_handle_standard_args` (PCL_IO)
  does not match the name of the calling package (PCL).  This can lead to
  problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /usr/local/share/pcl-1.11/PCLConfig.cmake:619 (find_package_handle_standard_args)
  CMakeLists.txt:7 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

@larshg
Copy link
Contributor

larshg commented Oct 2, 2020

I would rather search for a way to avoid the warnings all together - is it the use of components, that aren't implemented correctly. Ie. VTK and Qt has components as well.

@mvieth
Copy link
Member Author

mvieth commented Oct 2, 2020

If I see it correctly, the warnings are not avoidable and it is ok to just suppress them: In this line (or three lines further down if module is header-only), the function find_package_handle_standard_args is called for each pcl module, e.g. PCL_COMMON, PCL_SAMPLE_CONSENSUS. The user however wrote find_package(PCL).
I think the warning is rather intended to alert of different things, e.g. capitalization (rt instead of RT). See also my comment on the issue. If you see this differently, I'm interested in hearing your idea.

@mvieth mvieth marked this pull request as ready for review October 4, 2020 13:12
@kunaltyagi
Copy link
Member

Just chiming in with low CMake knowledge: Is there a reason we can't correct the name instead (for libusb)?

@mvieth
Copy link
Member Author

mvieth commented Oct 9, 2020

Just chiming in with low CMake knowledge: Is there a reason we can't correct the name instead (for libusb)?

As far as I understand it, no: e.g. in the first CMake Warning above, the problem is that USB_10 (which is passed to find_package_handle_standard_args) is not the same as OpenNI. But of course we cannot replace USB_10 with OpenNI, because we want to find USB_10. Similar in the second warning. In the third warning, libusb-1.0 and PCL differ. Again, we cannot change find_package_handle_standard_args(libusb-1.0 [...]) to find_package_handle_standard_args(PCL [...]).
find_package_handle_standard_args is also called for OpenNI, OpenNI2, and PCL, so the variables like _FOUND should be set as expected.

@kunaltyagi
Copy link
Member

Thanks mvieth. I now understand what's happening here. Based on the explanation, the changes LGTM 😄

@mvieth mvieth added the needs: code review Specify why not closed/merged yet label Oct 14, 2020
@SunBlack
Copy link
Contributor

SunBlack commented Nov 1, 2020

Couldn't the messages in FindOpenNI(2) suppressed by moving the USB_10 into a separate CMake file and calling find_package(USB_10 REQUIRED) within FindOpenNI(2)? This would also deduplicate code between FindOpenNI.cmake and FindOpenNI2.cmake

@mvieth
Copy link
Member Author

mvieth commented Nov 1, 2020

Couldn't the messages in FindOpenNI(2) suppressed by moving the USB_10 into a separate CMake file and calling find_package(USB_10 REQUIRED) within FindOpenNI(2)? This would also deduplicate code between FindOpenNI.cmake and FindOpenNI2.cmake

Hm, sounds doable, but I am not really a cmake expert, and I don't know if there would be any side effects?
There is also the file Findlibusb-1.0.cmake. Wouldn't that be the same package as USB_10?

@SunBlack
Copy link
Contributor

SunBlack commented Nov 2, 2020

Indeed.

I currently taking a look into this and I have fun 😆 Both OpenNI find scripts are looking for libusb-1.0 and define USB_10_LIBRARY - but this value is never used, as even the find script is suing LIBUSB_1_LIBRARIES (which comes from the Findlibusb-1.0.cmake)

find_library(USB_10_LIBRARY
NAMES usb-1.0
HINTS ${PC_USB_10_LIBDIR} ${PC_USB_10_LIBRARY_DIRS} "${USB_10_ROOT}" "$ENV{USB_10_ROOT}"
PATH_SUFFIXES lib)

set(OPENNI_LIBRARIES ${OPENNI_LIBRARY} ${LIBUSB_1_LIBRARIES})

@larshg
Copy link
Contributor

larshg commented Nov 2, 2020

Since LibUSB is found in the general CMakeLists here:

pcl/CMakeLists.txt

Lines 307 to 314 in 79da811

# libusb-1.0
option(WITH_LIBUSB "Build USB RGBD-Camera drivers" TRUE)
if(WITH_LIBUSB)
find_package(libusb-1.0)
if(LIBUSB_1_FOUND)
include_directories(SYSTEM "${LIBUSB_1_INCLUDE_DIR}")
endif()
endif()

It shouldn't be necessary to search for them again in submodules (ie. All grabbers which are searched for afterwards)
Simple checking LIBUSB_1_FOUND, should be sufficient. And use appropriate includes/libs variables afterwards.

@SunBlack
Copy link
Contributor

SunBlack commented Nov 2, 2020

I refactored the libusb CMake code locally already. Currently just looking why some apps are not build. PCL_ADD_EXECUTABLE(pcl_dinast_grabber ..) is called, nevertheless not in the Makefile. Even on the CI I'm missing them 🤔

@SunBlack
Copy link
Contributor

SunBlack commented Nov 3, 2020

@mvieth Can you try if #4483 removes the warning in the OpenNI modules? In best case on MacOS, otherwise you could change the if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") statement to if(TRUE) (just as test), so find_package(libusb) will be called.

@mvieth
Copy link
Member Author

mvieth commented Nov 28, 2020

@SunBlack I reverted all changes related to libusb, now only the warnings regarding the individual pcl modules are suppressed. I haven't tested your PR yet, but I think it should successfully remove the libusb warnings.

@SunBlack
Copy link
Contributor

Can confirm with CMake 3.19.1 that this PR fixes the issue.

@mvieth mvieth requested a review from kunaltyagi April 1, 2021 07:54
@mvieth mvieth merged commit f67053e into PointCloudLibrary:master Apr 21, 2021
@mvieth mvieth deleted the cmake_warns branch April 21, 2021 11:39
mvieth added a commit to mvieth/pcl that referenced this pull request Dec 27, 2021
* Suppress cmake warnings for mismatched names
cielavenir pushed a commit to mujin/pcl that referenced this pull request May 9, 2023
* Suppress cmake warnings for mismatched names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: cmake needs: code review Specify why not closed/merged yet
Projects
None yet
4 participants