build/ops: fix dmclock test-related build breakage by conditionalizing dmclock tests

[smithfarm]

This is an alternative PR to #14997 (that PR builds only on the most cutting-edge openSUSE version - Tumbleweed . . . still trying to figure out exactly why)

http://tracker.ceph.com/issues/19987

[smithfarm]

Pushed wip-findgtest-cmake to ceph/ceph-ci.git

https://shaman.ceph.com/builds/ceph/wip-findgtest-cmake/

[smithfarm]

Relevant excerpt from log of failed build:

[  158s] CMake Error at src/dmclock/CMakeLists.txt:20 (find_package):
[  158s]   By not providing "Findgtest.cmake" in CMAKE_MODULE_PATH this project has
[  158s]   asked CMake to find a package configuration file provided by "gtest", but
[  158s]   CMake did not find one.
[  158s] 
[  158s]   Could not find a package configuration file provided by "gtest" with any of
[  158s]   the following names:
[  158s] 
[  158s]     gtestConfig.cmake
[  158s]     gtest-config.cmake
[  158s] 
[  158s]   Add the installation prefix of "gtest" to CMAKE_PREFIX_PATH or set
[  158s]   "gtest_DIR" to a directory containing one of the above files.  If "gtest"
[  158s]   provides a separate development package or SDK, be sure it has been
[  158s]   installed.
[  158s] 
[  158s] 
[  158s] -- Configuring incomplete, errors occurred!

[tchaikov]

@smithfarm once shaman is happy, it's good to merge, i think.

[smithfarm]

@tchaikov Thanks, but SUSE builds are still not happy. Now the build error is:

[  175s] CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
[  175s]   Could NOT find GTest (missing: GTEST_LIBRARY GTEST_INCLUDE_DIR
[  175s]   GTEST_MAIN_LIBRARY)
[  175s] Call Stack (most recent call first):
[  175s]   /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:388 (_FPHSA_FAILURE_MESSAGE)
[  175s]   /usr/share/cmake/Modules/FindGTest.cmake:205 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
[  175s]   src/dmclock/CMakeLists.txt:20 (find_package)

[tchaikov]

we do include GTest as a submodule. can we point GTEST_ROOT to the path in the source tree, so FindGTest.cmake can find it?

[smithfarm]

OK, how?

i think we don't need to use the system gtest: because "battery included".

[smithfarm]

OK, dropped that last commit.

[smithfarm]

@tchaikov Something like this?

set(GTEST_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/googletest)

[tchaikov]

@smithfarm something like that, but i am not 100% sure. because at configure time (when we run cmake) the gtest lib is not ready yet, so FindGtest could fail. that's a dilemma i was facing when trying to make boost an external project, which is built on demand. i need to give it a try locally next Monday.

[smithfarm]

Same error. I'll continue to poke at it.

[smithfarm]

I think the build failure is reproducible by passing -DWITH_TESTS=OFF to cmake.

[smithfarm]

This fixes the -DWITH_TESTS=OFF build.

[smithfarm]

Looks like we can drop https://github.com/ceph/ceph/blob/master/src/dmclock/cmake/modules/Findboost.cmake and https://github.com/ceph/ceph/blob/master/src/dmclock/cmake/modules/Findgtest.cmake

[smithfarm]

@ivancich @tchaikov Here is how we could conditionalize the build of the dmclock test binaries. (Note that WITH_TESTS is not appropriate here, because it guards the build of binaries included in the "ceph-test" subpackage.)

If we're going to build these dmclock test binaries separately, my view is they should be properly conditionalized. I understand this may mean including the conditional in the upstream dmClock repo as well.

[smithfarm]

note that this PR now consists of a single (completely reworked) commit

[ivancich]

My concern regarding this PR is that it conflicts with the PR for the second part of the dmclock integration (#14997). That PR, for example, currently does not integrate src/dmclock/CMakeLists.txt with ceph's build, so the changes there will be problematic. And that removes the need for any ceph conditionals to appear within the dmclock library, which, being an external library, would seem inappropriate.

[ivancich]

WITH_DMCLOCK_TESTS should not appear below src/dmclock. A better approach is the one used in #14997. In fact a good starting point might be the final two commits in that PR. The first is a git subtree pull of the latest dmclock master that significantly reorganizes dmclock's cmake process (see README.git-subtree to see how this is done). The second (commit 8215686) are corresponding changes in ceph's cmake process. This approach as a starting point will avoid conflicts and re-visiting this issue down the line.

[ivancich]

@smithfarm I'm currently testing a slight change to #14997, which I think will handle your case as well. I'm currently testing the build. The additions to src/CMakeLists.txt are below and you can also find the entire branch here: https://github.com/ivancich/ceph-fork/tree/wip-bring-in-dmclock-p2-leap .

option(WITH_DMCLOCK_TESTS                                                                                                                                                             
  "enable the build of dmclock-tests and dmclock-data-struct tests binaries"                                                                                                          
  OFF)                                                                                                                                                                                
if(WITH_TESTS AND WITH_DMCLOCK_TESTS)                                                                                                                                                 
  # note: add_test is not being called, so dmclock tests aren't part                                                                                                                  
  # of ceph tests                                                                                                                                                                     
  add_subdirectory(dmclock/test)                                                                                                                                                      
  add_subdirectory(dmclock/support/test)                                                                                                                                              
endif(WITH_TESTS AND WITH_DMCLOCK_TESTS)  

[ivancich]

I've added a WITH_DMCLOCK_TESTS commit (04071b6) to PR for part 2 of the dmclock integration (#14997). It also includes the changes you recommended regarding cmake's find_package. @smithfarm does that work for you?

[smithfarm]

@ivancich The latest #14997 fixes the problem for us; we're going with it - thanks!