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

Added GMOCK_LIBRARY check #1769

Closed
wants to merge 1 commit into from
Closed

Added GMOCK_LIBRARY check #1769

wants to merge 1 commit into from

Conversation

ywiyogo
Copy link

@ywiyogo ywiyogo commented Oct 29, 2020

Added CMake check to avoid build error if GMOCK_LIBRARY not found.

@MichaelGrupp
Copy link
Contributor

Can you explain under which circumstances you had a problem with this?

  • build environment
  • cartographer version (Git hash)
  • build steps

Your change is also not compatible with our continuous integration build on Ubuntu Focal, as can be seen here: https://travis-ci.org/github/cartographer-project/cartographer/builds/739915655

@ywiyogo
Copy link
Author

ywiyogo commented Oct 29, 2020

@MichaelGrupp, I got the issue during compiling the cartographer on Raspbian OS of my RaspberryPi4 in ROS environment using catkin_make_isolated --install --use-ninja. The debug message of CMake shows that the GMock Library is not found:
${GMOCK_LIBRARY} returns GMOCK_LIBRARY-NOTFOUND. The Gtest is found in /usr/lib/arm-linux-gnueabihf/libgtest.a

I've just analyzed my system. I did sudo apt install google-mock and it returns GMOCK_LIBRARY-NOTFOUND. Just now, I found out that I run sudo apt install libgmock-dev, and it installed some libraries, and then CMake can find the GMOCK_LIBRARY.

@MichaelGrupp
Copy link
Contributor

MichaelGrupp commented Oct 29, 2020

Thanks for the info. As of today, we depend on the google-mock package in our package.xml.

According to Debian Buster, libgmock-dev (or googletest for src) should be used instead of google-mock:

"NOTE: This is a transitional package, retained for backwards compatibility. New code should instead use either package libgmock-dev (for compiled lib) or package googletest (for lib sources)"
https://packages.debian.org/buster/google-mock

On Stretch, google-mock is also not recommended but the suggested alternative is different (only googletest):

NOTE: This is a transitional package, retained for backwards compatibility. New code should use package googletest instead.
https://packages.debian.org/stretch/google-mock

This is a bit confusing, especially since we want to support both right now.
Interestingly, I didn't face this when doing a fresh installation on Ubuntu 20 with the same revision of Cartographer.

Poking @wohe who was involved in a similar discussion around libgmock-dev here: #1705 (comment)

@MichaelGrupp
Copy link
Contributor

MichaelGrupp commented Oct 29, 2020

Note: in our CI scripts for the non-ROS part, we have this:

if [[ "$(lsb_release -sc)" = "focal" || "$(lsb_release -sc)" = "buster" ]]
then
sudo apt-get install -y python3-sphinx libgmock-dev libceres-dev protobuf-compiler

TODO: If it is mandatory to install libgmock-dev, but only on Buster/Focal, we should note this down in the user docs if it's not there yet.

@ywiyogo
Copy link
Author

ywiyogo commented Oct 29, 2020

Thanks for the info too @MichaelGrupp. No wonder this issue is so confusing.

On Stretch, google-mock is also not recommended but the suggested alternative is different (only googletest):

That was exactly the issue on my RaspPi4. If CMake cannot find the variable ${GMOCK_LIBRARY}, it throws a build error on the list(append..) line because of the GMOCK_LIBRARY-NOTFOUND..

@MichaelGrupp
Copy link
Contributor

Update: this looks like an issue in how rosdep resolves the key google-mock. It should use the correct package on these distros. I opened a pull request in rosdistro, let's wait how this goes: ros/rosdistro#27195

@ywiyogo
Copy link
Author

ywiyogo commented Nov 20, 2020

@MichaelGrupp
What do you think about this workaround?

    if(GMOCK_LIBRARY_FOUND)
      list(APPEND GMOCK_LIBRARIES ${GMOCK_LIBRARY})
    endif()
    list(APPEND GMOCK_LIBRARIES ${GTEST_LIBRARY})

I just checked again, and the issue is only for GMOCK_LIBRARY.

@bochen87
Copy link

since ros/rosdistro#27195 was merged, can we close this PR?

@ywiyogo
Copy link
Author

ywiyogo commented Jul 22, 2022

since ros/rosdistro#27195 was merged, can we close this PR?

yes please

@bochen87 bochen87 closed this Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants