Fixing a typo in example/CMakeLists.txt and suggesting two related minor changes

[pkestene]

Description

This issue is about cmake build system in branch prev3-develop.

1. If think the following line
   https://github.com/cburstedde/p4est/blob/prev3-develop/example/CMakeLists.txt#L11
   should be changed into

list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/../cmake)

since subfolder Modules doesn't exists.

2. This is about a minor change, I'd like to suggest:

When trying to build p4est examples using cmake, for example on a machine where p4est/sc is already installed by the system (e.g. using deb package on Ubuntu machine), I found that the CMakeCache.txt is kind of polluted because variable SC_CONFIG_H is set to /usr/include/sc_config.h eventhough I used CMAKE_PREFIX_PATH to the location where I've just built the latest version of p4est (branch prev3-develop). So this information (SC_CONFIG_PATH set to /usr/include/sc_config.h) gives the wrong impression that cmake detected the system installation.

Actually this behavior is related to the way example/CMakeLists.txt is trying to detect SC et P4EST :

find_package(SC REQUIRED)
find_package(P4EST REQUIRED)

The macro find_package will ALWAYS first use the MODULE mode to detect SC and P4EST (that means use FindSC.cmake and FindP4EST.cmake), and if the detection fails, it will then use the CONFIG mode (that means use SCConfig.cmake and P4ESTConfig.cmake).

So no matter what CMAKE_PREFIX_PATH is, find_package(SC REQUIRED), will always set variable SC_CONFIG_H to /usr/include/sc_config.h on a system which has p4est/sc install in system path, during the MODULE pass which will fail, and succeed later during the CONFIG pass of find_package(SC REQUIRED).

So finally, I suggest the following change

find_package(SC CONFIG)
if(NOT SC_FOUND)
  find_package(SC REQUIRED)
endif()

find_package(P4EST CONFIG)
if(NOT P4EST_FOUND)
  find_package(P4EST REQUIRED)
endif()

thus, we always first try the CONFIG mode, if it fails, only then we required the MODULE mode to succeed.
That way, on a system where p4est/sc is already installed, the CMakeCache.txt will not be polluted by variable SC_CONFIG_H = /usr/include/sc_config.h, because if CMAKE_PREFIX_PATH is correctly set, the CONFIG mode will succeed.

3. Another related suggested change:
   the template file $(P4EST_TOPLEVEL)/cmake/config.cmake.in contains a call to find_dependency(SC), which actually is just a wrapper to find_package(SC) where arguments like QUIET or REQUIRED are forwarded from up stack call.

Here, again in example/CMakeLists.txt, when calling find_package(P4EST REQUIRED), if the MODULE fails, the CONFIG mode is called, i.e. calls P4ESTConfig.cmake, which calls find_dependency(SC) and again tries to find SC in MODULE mode, and if /usr/include/sc_config.h exists, it will be reported in CMakeCache.txt, eventhough the CMAKE_PREFIX_PATH was set to use another installed version of p4est/sc

I think it is safe to change the call to find_depencency(SC) by:

if( (DEFINED SC_FOUND) AND (NOT SC_FOUND) )
  find_dependency(SC CONFIG)
endif()

because:

• if SC has already been detected, there is no need to do it again
• if SC should be detected inside P4ESTConfig.cmake, it shoud be in CONFIG mode only.

To Reproduce
What can we do to reproduce the issue reported?

Additional information
Operating system, build configuration, etc.

[cburstedde]

Impressive stuff, thanks again. I think this sounds fine but I am no expert on CMake.

[cburstedde]

Merged -- thanks for the elaborate report!!