catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.46k stars 3.02k forks source link

Installing Catch2 when using FetchContent #2164

Open triskeldeian opened 3 years ago

triskeldeian commented 3 years ago

Due to the behaviors documented in https://github.com/catchorg/Catch2/issues/1373 Catch doesn't install anything when used as a subproject but this doesn't allow it to be installed when used through the FetchContent macro, that requires the package to be added with the add_subdirectory.

Could be useful to add an option, maybe even mark as advanced if necessary, to force the installation if the user wants to, even when used as subproject

saxbophone commented 3 years ago

Another solution might be for Catch to always install its targets, even when used as a sub-project, and leave it up to dependents to opt out of Catch's install targets using add_subdirectory(<SRC> <BIN> EXCLUDE_FROM_ALL). This could be documented in the instructions for using FetchContent() and friends to get Catch.

dimztimz commented 3 years ago

What you need to do is to not consume Catch2 via add_subdirectory(), but instead use find_package(Catch2). With find_pacakge() you make sure that Catch2 is already installed, even before building your project. Generally you should use find_package() for consuming dependencies and a package manager to download and install them. FetchContent + add_subdirectory() is a hack that doesn't always fulfill the needs.

saxbophone commented 3 years ago

What you need to do is to not consume Catch2 via add_subdirectory(), but instead use find_package(Catch2). With find_pacakge() you make sure that Catch2 is already installed, even before building your project. Generally you should use find_package() for consuming dependencies and a package manager to download and install them. FetchContent + add_subdirectory() is a hack that doesn't always fulfill the needs.

I agree that using find_package() is an option, but for flexibility's sake, Catch2 (as well as many other CMake-based projects) also support the add_subdirectory() approach and it is this feature that I think is being discussed here.

There are some useful benefits to using the add_subdirectory() approach (such as more fine-grained control about the exact version of the dependency is being used, avoiding the need to manage an installed copy on the target system), so I don't think switching to using find_package() is a useful solution to the problems raised in this issue when it might be solved by changing the CMake file in Catch.

triskeldeian commented 3 years ago

Another solution might be for Catch to always install its targets, even when used as a sub-project, and leave it up to dependents to opt out of Catch's install targets using add_subdirectory(<SRC> <BIN> EXCLUDE_FROM_ALL). This could be documented in the instructions for using FetchContent() and friends to get Catch.

In theory I agree with you, but that would be a breaking change, while adding an option will give you the possibility without breaking existing builds

triskeldeian commented 3 years ago

What you need to do is to not consume Catch2 via add_subdirectory(), but instead use find_package(Catch2). With find_pacakge() you make sure that Catch2 is already installed, even before building your project. Generally you should use find_package() for consuming dependencies and a package manager to download and install them. FetchContent + add_subdirectory() is a hack that doesn't always fulfill the needs.

@saxbophone already gave you a more than valid answer. I would also add that FetchContent is not an hack but a very valid solution to a big problem. Even without using that, though, there are many situations where the usage of the add_subdirectory can be useful. For example I load Catch in my core project as a git subtree and I want the plugin projects depending from that Core to be able to use Catch without having to rely on an external installation but using the one I ship with the core package

dimztimz commented 3 years ago

I would also add that FetchContent is not an hack but a very valid solution to a big problem.

FetchContent + add_subdirectory, git submodule + add_subdirecory or manually copied source tree and add_subdirectory are all cases of bundling or packaging your dependencies. See https://wiki.gentoo.org/wiki/Why_not_bundle_dependencies and https://www.youtube.com/watch?v=sBP17HQAQjk . It is something that should be avoided.

I too use Catch2 via add_subdirectory because it is a very private and isolated dependency, a dependency of the tests and my tests don't get installed, so Catch2 doesn't need to be installed. So the current behavior is OK for this use case.

If any of your dependencies becomes public then you should strictly consume it via find_package() and let a package manager manage their building and installation.

triskeldeian commented 3 years ago

I understand your point @dimztimz but I'd rather have the choice of bundling my dependency if, in my use case, the advantages are greater than the disadvantages. This may be particularly useful for header only libraries like Catch. I don't want to break existing code, therefore I proposed a harmless solution and even suggested to put it among the advanced options so that the dev can use it only when he really know what he is doing, but I don't see the point of an upstream forbidding me to develop my code the way I'd like.

saxbophone commented 3 years ago

Yeah, I think allowing for flexibility is really useful for end-users. @dimztimz is not wrong that bundling or "vendoring" dependencies is not ideal, but it would be unfortunate to not allow dependents to use the various different methods that CMake provides for retrieving dependencies —these are after all, documented methods that CMake provides for dependency resolution. For my example use-case, it makes it easier for doing cross-platform builds on continuous integration, as I don't need to add separate code for installing third party dependencies on macOS, Windows and Linux.

dnmiller commented 3 years ago

I've been using the following. What you actually need are the cmake modules, so you can do

include(FetchContent)
FetchContent_Declare(
  Catch2
  GIT_SHALLOW    TRUE
  GIT_REPOSITORY https://github.com/catchorg/Catch2.git
  GIT_TAG        v3.0.0-preview3)
FetchContent_MakeAvailable(Catch2)

# For catch 2 only
# list(APPEND CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/contrib)
# For catch 3 only
list(APPEND CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/extras)

Then use

include(CTest)
include(Catch)
catch_discover_tests(${target_exe})

as you would normally.

xealits commented 1 month ago
...
FetchContent_MakeAvailable(Catch2)

# For catch 2 only
# list(APPEND CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/contrib)
# For catch 3 only
list(APPEND CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/extras)

Then use

include(CTest)
include(Catch)
catch_discover_tests(${target_exe})

as you would normally.

In my case, it does not work with catch v3.6.0 and this msgpack implementation in C++17, which is also included via FetchContent, right after Catch2 in the main CMakeLists. I.e. if I do find_package(Catch) right after include(Catch), like what the msgpack lib does, it fails:

CMake Error at CMakeLists.txt:35 (find_package):
  By not providing "FindCatch.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Catch", but
  CMake did not find one.
...

Also, I do not need to list(APPEND CMAKE_MODULE_PATH ...), because the ${Catch2_SOURCE_DIR}/extras is already in the list after FetchContent_MakeAvailable.

There is an example how to integrate FetchContent and find_package in CMake docs. They demonstrate it on Catch2 and GoogleTest. But what they show breaks Catch2 build completely:

...
FetchContent_Declare(
  Catch2
  GIT_REPOSITORY https://github.com/catchorg/Catch2.git
  GIT_TAG        605a34765aa5d5ecbf476b4598a862ada971b0cc # v3.0.1
  FIND_PACKAGE_ARGS   # <---- this thing somehow breaks Catch2 CMake config
)

# This will try calling find_package() first for both dependencies
FetchContent_MakeAvailable(Catch2)

I get build/_deps/catch2-subbuild/catch2-populate-prefix/tmp/catch2-populate-gitclone.cmake with stuff like:

execute_process(
  COMMAND "/usr/bin/git"  checkout v3.6.0 --
  WORKING_DIRECTORY "catch2-src/advice.detachedHead=false"
  RESULT_VARIABLE error_code
  )

And CMake says FetchContent_MakeAvailable(Catch2) failed with ninja: build stopped: subcommand failed on:

cd /.../build/_deps/catch2-src && /usr/bin/cmake -P /.../build/_deps/catch2-subbuild/catch2-populate-prefix/tmp/catch2-populate-gitupdate.cmake