Fix gtest-not-found bug

[mikepurvis]

This change fixes #1030, which arises from the changes made in #1022.

It works for me on Ubuntu 16.04, but it should be checked by a user on Debian Buster to confirm that the original fix still works as expected.

[dirk-thomas]

@sloretz can you please try this with Debian Buster?

[mikepurvis]

@jwhendy Can you also check if this fixes your build issues on Arch?

[mikepurvis]

Obviously this is changing the signature of the function, but I'm pretty sure the only place that's calling it is in this file; it's not really an external interface, so it should be safe to change.

[jwhendy]

I can confirm that the fix-1030 branch from @mikepurvis works when building ros-melodic-realtime-tools on arch linux, while kinetic-devel does not.

I do get a bunch of warnings like this, but they don't seem to affect the build.

CMake Warning (dev) at /usr/src/gmock/CMakeLists.txt:47 (add_subdirectory):
  Policy CMP0013 is not set: Duplicate binary directories are not allowed.
  Run "cmake --help-policy CMP0013" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  The binary directory

    /home/jwhendy/catkin_ws/build/moveit_tutorials/gtest

  is already used to build a source directory.  This command uses it to build
  source directory

    /usr/src/gtest

  which can generate conflicting build files.  CMake does not support this
  use case but it used to work accidentally and is being allowed for
  compatibility.
This warning is for project developers.  Use -Wno-dev to suppress it.

Edit: this is one output from my catkin build process, but most/all packages seem to spit it out. Just adding this as it might be confusing that realtime-tools was the topic of discussion and this error was for moveit_tutorials.

Thanks for taking a stab at this @mikepurvis !

[dirk-thomas]

@jwhendy Please provide more context: what platform are you seeing these warnings on, what version of CMake are you using? Also this seems to be unrelated to this PR. Can you please check if the same warning happens with older versions of catkin?

[sloretz]

@sloretz can you please try this with Debian Buster?

I'm able to build on Debian buster up to and including realtime_tools (with ros-controls/realtime_tools#46) using this branch, though there are a couple extra include paths passed to g++. The ones ending in /include make sense to me, but the ones without it don't. I haven't figured out where they come from yet.

/usr/bin/c++  -DROSCONSOLE_BACKEND_LOG4CXX -DROS_BUILD_SHARED_LIBS=1 -DROS_PACKAGE_NAME=\"realtime_tools\" -I/home/sloretz/rtt/src/realtime_tools/include -I/home/sloretz/ws/noetic/devel_isolated/roscpp/include -I/home/sloretz/ws/noetic/devel_isolated/roscpp/include/ros -I/home/sloretz/ws/noetic/src/ros_comm/clients/roscpp/include -I/home/sloretz/ws/noetic/src/ros_comm/utilities/xmlrpcpp/include -I/home/sloretz/ws/noetic/src/ros_comm/utilities/xmlrpcpp/include/xmlrpcpp -I/home/sloretz/ws/noetic/devel_isolated/rosgraph_msgs/include -I/home/sloretz/ws/noetic/devel_isolated/std_msgs/include -I/home/sloretz/ws/noetic/src/std_msgs/include -I/home/sloretz/ws/noetic/src/rosconsole/include -I/home/sloretz/ws/noetic/src/roscpp_core/roscpp_serialization/include -I/home/sloretz/ws/noetic/src/roscpp_core/rostime/include -I/home/sloretz/ws/noetic/src/roscpp_core/roscpp_traits/include -I/home/sloretz/ws/noetic/src/roscpp_core/cpp_common/include -isystem /usr/src/googletest/googlemock/include -isystem /usr/src/googletest/googlemock -isystem /usr/src/googletest/googletest/include -isystem /usr/src/googletest/googletest  -O3 -DNDEBUG   -pthread -DGTEST_HAS_PTHREAD=1 -o CMakeFiles/realtime_box_tests.dir/test/realtime_box_tests.cpp.o -c /home/sloretz/rtt/src/realtime_tools/test/realtime_box_tests.cpp

From above, highlighting:

    -isystem /usr/src/googletest/googlemock/include
    -isystem /usr/src/googletest/googlemock
    -isystem /usr/src/googletest/googletest/include
    -isystem /usr/src/googletest/googletest

[jwhendy]

@dirk-thomas sorry on the delay, I was out of town. It is unrelated to this PR, but not unrelated to my overall issues of getting catkin and gmock to work on Arch in #1030. I forgot that I already commented on this over there.

I'll add a followup comment there instead of muddying the waters here, but this is almost certainly an issue on how to get Arch + gmock package files correct, nothing to do with catkin. Sorry about that.

[sloretz]

there are a couple extra include paths passed to g++. The ones ending in /include make sense to me, but the ones without it don't. I haven't figured out where they come from yet.

I think this is from the CMakeLists.txt shipped with googletest, so it can be ignored.

# Where Google Test's .h files can be found.
set(gtest_build_include_dirs
  "${gtest_SOURCE_DIR}/include"
  "${gtest_SOURCE_DIR}")
include_directories(${gtest_build_include_dirs})

[sloretz]

I don't understand why this works, but it works on Debian Buster. LGTM, though I haven't tried it on all platforms.

[dirk-thomas]

I don't understand why this works,

🤐

but it works on Debian Buster.

🚢 🤞

[dirk-thomas]

Thanks to everyone for creating / testing the patch.

[dirk-thomas]

The double "googletest" seems to be a mistake. The include and src directories should be siblings. See #1088.