Do only include(CTest) if dashboard is needed

[ClausKlein]

See https://cmake.org/cmake/help/latest/manual/ctest.1.html

[camio]

We're currently using include(CTest) to allow for configuration using the BUILD_TESTING variable. Is it better to not do this?

[ClausKlein]

Yes, see https://github.com/beman-project/execution26/pull/55/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20a for some reasons.

[camio]

@ClausKlein I don't understand what you're attempting to communicate by pointing me to that diff. It looks like that change removed the ability for someone to turn on tests and examples for that project when it is being used as a dependency. This is an important use-case to support.

[ClausKlein]

As a standalone project, testing and installing is always available.

But if I add project with FetchContent, I do not want to run its tests or compile the examples!

Even more, sometimes I don't want to install it, witch may be excluded with the EXCLUDE_FROM_ALL option.

from Chapter 40. Making Projects Consumable:

40.2. Don’t Assume A Top Level Build

Many projects have never considered the use case of being absorbed into a larger parent build directly. For such projects, there are a few common problems that are often encountered. The first and perhaps most prevalent is the use of global path variables like CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR. When the project is absorbed into a larger parent build, the relative location of the global path variables will change. The project might then no longer find things at the locations it expected. In general, avoid using CMAKE_SOURCE_DIR or CMAKE_BINARY_DIR unless a path truly needs to be relative to the top level of the source or build tree, even when absorbed into a larger parent build. Prefer instead to use directory-relative variables like CMAKE_CURRENT_SOURCE_DIR and CMAKE_CURRENT_BINARY_DIR, or project-relative variables like PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR.

Another common problem is modifying variables that have the potential to affect the whole build, not just the project. A classic example is forcing the value of cache variables like BUILD_SHARED_LIBS, BUILD_TESTING, CMAKE_BUILD_TYPE, CMAKE_MODULE_PATH or CMAKE_RUNTIME_OUTPUT_DIRECTORY. Using the FORCE or INTERNAL keywords with the set() command on these variables prevents a parent project from overriding that choice. Quite often, these variables shouldn’t be defined by the project as cache variables at all, and maybe shouldn’t even be set by the project to begin with. For variables like CMAKE_MODULE_PATH, append to existing values rather than replace them.

...

# CMake 3.21 or later is required for PROJECT_IS_TOP_LEVEL

  option(MYPROJ_ENABLE_TESTING "..." ${PROJECT_IS_TOP_LEVEL})
  if(MYPROJ_ENABLE_TESTING)
      add_subdirectory(tests)
  endif()
  if(PROJECT_IS_TOP_LEVEL)
      add_subdirectory(packaging)
  else()
      # Users are unlikely to be interested in testing this
      # project, so don't show it in the basic options
      mark_as_advanced(MYPROJ_ENABLE_TESTING)
  endif()

[bretbrownjr]

I agree with the use cases, though I don't like the idea of maintaining boilerplate and hand-maintained options across N repos to keep this workflow functioning as specified.

I'm in favor of adding changes to the standard and to this project in this direction (Craig Scott is typically correct), but only if we are committed as a group to factoring out all the boilerplate at some point.

CMake default features notwithstanding, it is an anti pattern to have much more than declaration of your project structure in your build configuration.

By this I mean CMakeLists.txt should mainly consist of properties unique to this project: names, files, dependencies, directory structure, etc. Logic to behave differently depending on build context is not unique to exemplar. Every project in Beman should not maintain a compatible copy of basically the same CMake logic.

[bretbrownjr]

I'll add that CMake itself needs to evolve better facilities for relevant features at some point. If Beman ends up creating CMake modules to successfully assist in reducing project complexity, we should circle back and open up issues in upstream CMake to illustrate the relevant need.

Beman doesn't exist to innovate in C++ tooling best practices, so if we feel compelled to do that sort of innovation, we should consider it technical debt and develop a plan to unwind it reasonably (perhaps by getting enhancements to CMake itself).

[ClausKlein]

By this I mean CMakeLists.txt should mainly consist of properties unique to this project: names, files, dependencies, directory structure, etc. Logic to behave differently depending on build context is not unique to exemplar. Every project in Beman should not maintain a compatible copy of basically the same CMake logic.

Then you need some CMake modules from a central repo to keep the CMake logicin sync.

see i.e. https://github.com/aminya/project_options and https://github.com/MarkSchofield/WindowsToolchain

or maybe: https://github.com/arsenm/sanitizers-cmake

[camio]

But if I add project with FetchContent, I do not want to run its tests or compile the examples!

Projects added with FetchContent should not build tests and examples by default, but there are valid uses for turning them on when they are part of a larger project:

1. Someone might want to turn on all tests for a large project, including dependencies, as part of a nightly job or when experimenting with different compiler/flag combinations.
2. Someone may want to make changes to a dependency as part of making changes to a larger project. They could use FETCHCONTENT_SOURCE_DIR_X to use a local checkout of the dependency, make changes, and run tests without needing to generate a new project.

The quoted text in https://github.com/beman-project/exemplar/issues/60#issuecomment-2451396397 has a decent pattern that I think will allow for both use-cases:

option(BEMAN_EXEMPLAR_ENABLE_TESTING "Build Beman.Exemplar tests" ${PROJECT_IS_TOP_LEVEL})
option(BEMAN_EXEMPLAR_ENABLE_EXAMPLES "Build Beman.Exemplar examples" ${PROJECT_IS_TOP_LEVEL})
option(BEMAN_EXEMPLAR_ENABLE_DOCUMENTATION "Build Beman.Exemplar documentation" ${PROJECT_IS_TOP_LEVEL})

# ...

The only drawback I can think of with this approach is that we aren't honoring the BUILD_TESTING variable provided by the CTest module. I'm beginning to think avoiding the CTest module is the right thing to do anyway because the single option it creates isn't granular enough for decently sized projects.