Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using system installed Catch for builds #30

Open
badshah400 opened this issue Oct 20, 2023 · 1 comment
Open

Allow using system installed Catch for builds #30

badshah400 opened this issue Oct 20, 2023 · 1 comment

Comments

@badshah400
Copy link

I am the maintainer of RPM packages of trng for openSUSE.

While updating our packages to version 4.25, we noticed that tests require the Catch2 library, but there is no way to use a system installed Catch2 instead of the bundled version in external/Catch2/. Since we already have Catch2 packages for openSUSE (and probably most other distros) — and we prefer to use system libraries over bundled ones — I wrote a patch that allows one to use a system installed Catch2 to build and run tests. The patch is fairly simple:

Index: trng4-4.25/CMakeLists.txt
===================================================================
--- trng4-4.25.orig/CMakeLists.txt
+++ trng4-4.25/CMakeLists.txt
@@ -14,6 +14,7 @@ endif()
 
 option(TRNG_ENABLE_TESTS "Enable/Disable the compilation of the TRNG tests" ON)
 option(TRNG_ENABLE_EXAMPLES "Enable/Disable the compilation of the TRNG examples" ON)
+option(USE_EXTERNAL_CATCH "Use system installed Catch2" ON)
 
 if(CMAKE_CXX_COMPILER_ID MATCHES GNU)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic -Wmaybe-uninitialized")
@@ -42,7 +43,11 @@ configure_package_config_file("cmake/Con
 
 add_subdirectory(trng)
 if(TRNG_ENABLE_TESTS)
-  add_subdirectory(external/Catch2)
+  if(USE_EXTERNAL_CATCH)
+    find_package(Catch2 2 REQUIRED)
+  else(USE_EXTERNAL_CATCH)
+    add_subdirectory(external/Catch2)
+  endif()
   add_subdirectory(tests)
 endif()
 if(TRNG_ENABLE_EXAMPLES)

I would be happy to send you a PR if this is helpful. We set external Catch2 "ON" by default, but I could of course switch it to "OFF" by default if you prefer that. Also, I guess the actual name of the option is up for discussion as well — should it be "TRNG_USE_EXTERNAL_CATCH", for example?

Thanks for working on this great library.

@rabauke
Copy link
Owner

rabauke commented Oct 28, 2023

Thanks for your suggestion. TRNG uses Catch2 version 2.x, which is a pure header-only library. I see no benefit over using a system-installed version. Using a system-installed version just raises the risk of incompatibilities. Including Catch2 as a sub-module ensures that the unit tests always compile. This is particularly convenient on Windows systems, where no package manager is available to install Catch2.

Though, the situation changes with Catch2's 3.x branch, where the framework is no longer a header-only library and needs to be built before it can be used. In this case, using a system-installed version of Cacht2 is certainly beneficial.

I think, in the long run, it would be best to port the unit tests to Catch2 3.x and to use the system installed-version only. I.e., make Catch2 3.x an optional dependency and build unit tests if and only if Catch2 has been found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants