Define GMOCK_* and GTEST_* variables in a new subproject

[rhaschke]

This fixes the following bug occurring for a cmake project that comprises multiple catkin sub-projects:

-- Using CATKIN_ENABLE_TESTING: ON
-- Call enable_testing()
-- Using CATKIN_TEST_RESULTS_DIR: /tmp/build/test_results
-- Found gtest sources under '/usr/src/googletest': gtests will be built
-- Found gmock sources under '/usr/src/googletest': gmock will be built
...
-- Using CATKIN_ENABLE_TESTING: ON
-- Call enable_testing()
-- Using CATKIN_TEST_RESULTS_DIR: /tmp/build/test_results
-- Found gmock: gmock and gtests will be built
CMake Error at /opt/ros/melodic/share/catkin/cmake/assert.cmake:3 (message):

  Assertion failed: GTEST_LIBRARIES (value is '')

Call Stack (most recent call first):
  /opt/ros/melodic/share/catkin/cmake/test/gtest.cmake:185 (assert)
  /opt/ros/melodic/share/catkin/cmake/test/gtest.cmake:89 (_catkin_add_executable_with_google_test)
  /opt/ros/melodic/share/catkin/cmake/test/gtest.cmake:37 (_catkin_add_google_test)
  client/CMakeLists.txt:157 (catkin_add_gtest)

I think this issue was introduced in #1091 (definitely after release 0.7.24) when a shortcut defining GTEST_FOUND was introduced as soon as all gtest and gmock targets are defined: https://github.com/ros/catkin/blob/4fe89dd8a553a74e16e14754021e6550386fe5a3/cmake/test/gtest.cmake#L312-L317

However, those targets might have been defined via a previous source build of gtest and gmock. In this case, when entering the second catkin sub-package, all GTEST_* variables are undefined again, but the targets still exist causing GTEST_FOUND to be defined w/o defining the remaining variables.

This PR fixes this by defining both GTEST* and GMOCK* variables in a cached fashion in this case.

[rhaschke]

I see. Please suggest an alternative solution then to fix the broken state.

[rhaschke]

Maybe, alternatively, we could repeat the code setting those variables where GMOCK_FOUND and GTEST_FOUND are originally defined in the offending situation: https://github.com/ros/catkin/blob/4fe89dd8a553a74e16e14754021e6550386fe5a3/cmake/test/gtest.cmake#L313-L317

To be on the really safe side, you might want to test the variables [GTEST | GMOCK]_FROM_SOURCE_* beforehand for existence.

[dirk-thomas]

Please suggest an alternative solution then to fix the broken state.

Unfortunately I don't have a proposal at hand. This CMake logic is extremely fragile and I wouldn't consider a "cmake project that comprises multiple catkin sub-projects" to be a supported use case - at least this is not used / tested anywhere. So I am not surprised recent changes broke that use case. I am happy to review / consider any kind of patch if it looks reasonably safe.

we could repeat the code setting those variables where GMOCK_FOUND and GTEST_FOUND are originally defined in the offending situation:

I would wonder if this would only cover some of the cases (depending on where/how Googletest is found) but not correctly define the variable in other cases when including multiple catkin packages within a single CMake context. Please consider updating the pull request as you imagine it. Even if it doesn't solve all cases but improves the one you are mostly interested in that would be fine with me to merge.

[rhaschke]

@dirk-thomas, I pushed the suggested alternative solution.

[dirk-thomas]

The patch looks fine to me. If it works for your use case :+1: from me.

Please target the default branch though which is noetic-devel. I am happy to backport the commit from there to kinetic-devel.

[dirk-thomas]

Maybe also update the title to reflect the changed patch.

[rhaschke]

Rebased onto noetic-devel and rephrased PR title. Thanks for back-porting to Melodic!

[dirk-thomas]

Thanks for the patch.

[dirk-thomas]

Cherry-picked to kinetic-devel since CI had passed before already: 3e1c6555618cf6c917728060c10986b0f8b890e1.