Fix gtest_source_paths

[akela1101]

Since googletest_path is /usr/src/googletest, it resulted in /usr/src/googletest/googletest/googletest/src

Please, check if the fix is correct. For my Ubuntu18 it is.
And sorry, if I added the bugfix in wrong place (I don't know, maybe it should go to earlier branch..?).

[dirk-thomas]

Can you please clarify what issue you are seeing without this patch? And also provide reproducible steps for it.

[akela1101]

Sure...

Prerequisites

• Ubuntu18 with googletest source pacakge (and without binary googletest packages if any)
• Create a node with catkin_add_gtest() call.
  • or remove build directory for existing one.

Reproduction Steps

• Build the node (catkin build or cmake+make)
• Find the error:

CMake Error at /opt/ros/melodic/share/catkin/cmake/test/gtest.cmake:350 (add_subdirectory):
  add_subdirectory not given a binary directory but the given source
  directory "/usr/src/googletest/googlemock" is not a subdirectory of
  "build/catkin_tools_prebuild".  When
  specifying an out-of-tree source a binary directory must be explicitly
  specified.
Call Stack (most recent call first):
  /opt/ros/melodic/share/catkin/cmake/all.cmake:164 (include)
  /opt/ros/melodic/share/catkin/cmake/catkinConfig.cmake:20 (include)
  CMakeLists.txt:4 (find_package)

...

Explanation

The error means gtest_lib_dir is empty in the call add_subdirectory(${base_dir} ${gtest_lib_dir}). It happens because gtest_found is false. And this is because _gtest_source_paths from the fix is wrong ( /usr/src/googletest/googletest/googletest/src )

After the fix catkin_find_google_test_source succeeds, and add_subdirectory can add /usr/src/googletest.

[dirk-thomas]

Ubuntu18 with googletest source pacakge (and without binary googletest packages if any)

Can you please be a bit more specific. What source packages do you have installed / not installed? Do you have googletest cloned in the workspace src?

[akela1101]

What serial packages do you have installed / not installed?

I'm not sure what you mean, but I have ros-melodic-catkin : 0.7.23-1bionic.20200303.045725
CMake is also from Ubuntu: 3.10.2

Do you have googletest cloned in the workspace src?

Nope, nothing special is done. I just created an empty project with catkin_add_gtest
The configurations is like this:

cmake_minimum_required(VERSION 3.10)
project(node)

find_package(catkin REQUIRED COMPONENTS roscpp std_msgs)

catkin_package()

file(GLOB_RECURSE SOURCES "src/*.cpp")
add_executable(${PROJECT_NAME} ${SOURCES})
target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_14)

add_dependencies(${PROJECT_NAME} ${${PROJECT_NAME}_EXPORTED_TARGETS} ${catkin_EXPORTED_TARGETS})
target_include_directories(${PROJECT_NAME} SYSTEM PUBLIC ${catkin_INCLUDE_DIRS})
target_link_libraries(${PROJECT_NAME} ${catkin_LIBRARIES})

if(CATKIN_ENABLE_TESTING)
    catkin_add_gtest(${PROJECT_NAME}_test test/test.cpp)
    target_include_directories(${PROJECT_NAME}_test SYSTEM PUBLIC ${catkin_INCLUDE_DIRS})
    target_link_libraries(${PROJECT_NAME}_test ${catkin_LIBRARIES})
endif()

[dirk-thomas]

I was referring to googletest in my previous question.

[akela1101]

Oh, sorry, googletest : 1.8.0-6

[dirk-thomas]

googletest : 1.8.0-6

But not libgtest-dev?

(Which is a declared dependency:

catkin/package.xml

Line 35 in 0022fb0

<build_export_depend>gtest</build_export_depend>

mapped in https://github.com/ros/rosdistro/blob/2b3ef448986e6ec277018a48b84815895dffbfcd/rosdep/base.yaml#L1295)

[akela1101]

Both libgtest-dev and google-mock are installed as they are dependencies for ros-melodic-catkin, and googletest is a dependency for them.

[dirk-thomas]

If you have libgtest-dev installed I don't see why it would fail for you since its headers (

catkin/cmake/test/gtest.cmake

Lines 236 to 237 in 0022fb0

	set(_gtest_include_paths "${gtest_path}/include/gtest")
	set(_gtest_source_paths "${gtest_path}/src/gtest/src")

) will be picked up before the once this patch tries to fix (

catkin/cmake/test/gtest.cmake

Lines 240 to 241 in 0022fb0

	list(APPEND _gtest_include_paths "${googletest_path}/googletest/include/gtest")
	list(APPEND _gtest_source_paths "${googletest_path}/googletest/googletest/src")

).

[dirk-thomas]

What does the variable ${gtest_path} point to in your case where it fails without this patch? Does the header file ${gtest_path}/include/gtest/gtest.h exist?

[dirk-thomas]

I think I understand now why it fails on Bionic but still works on Focal. On Bionic libgtest-dev doesn't ship with headers in /usr/include/gtest/gtest.h (https://packages.ubuntu.com/bionic/amd64/libgtest-dev/filelist) where on Focal it does (https://packages.ubuntu.com/focal/amd64/libgtest-dev/filelist). So on Bionic the fallback is tried which fails due to the double-path fixed by this patch where on Focal that fallback isn't necessary.

[dirk-thomas]

👍 and especially to-be-backport to Melodic.

[akela1101]

${gtest_path} is "/usr" (line around 310).
So, I have ${gtest_path}/include/gtest/gtest.h which belongs to googletest package.
But no folder /usr/src/gtest/src, only: /usr/src/gtest

$ dpkg -L libgtest-dev
/.
/usr
/usr/share
/usr/share/doc
/usr/share/doc/libgtest-dev
/usr/share/doc/libgtest-dev/copyright
/usr/src
/usr/share/doc/libgtest-dev/changelog.Debian.gz
/usr/src/gtest

And googletest contains only /usr/src/googletest, not /usr/src/gtest.

PS:
googletest has headers in /usr/include/gtest/
It fails because it cannot find *.cc file, I guess...

[dirk-thomas]

Thanks for the patch.

[dirk-thomas]

Cherry-picked to kinetic-devel: 201ace7.