CMake `BUILD_TESTING` option breaks usage of libcurl via `FetchContent`

[robertmaynard]

I did this

The CMake variable BUILD_TESTING is used by the CMake include(CTest) to turn on tests globally ( https://cmake.org/cmake/help/latest/command/enable_testing.html ).

This means that projects shouldn't use cmake_dependent_option with BUILD_TESTING as that will force the variable to a given state and break other projects that consume libcurl via CMake's FetchContent

I expected the following

Easiest solution would be to rename BUILD_TESTING to CURL_ENABLE_TESTING or another variable that is prefixed with CURL_.

curl/libcurl version

curl master branch

operating system

Linux

[bdice]

Possibly related:

• cmake: Add CURL_BUILD_TESTING option #6036
• CMake: make BUILD_TESTING dependent option #6072

[vszakats]

Thanks for your report.

Do you know of any documentation or wiki page listing (non-CMAKE_*) variable names with "special" behavior like this one?

I guess curl can go by only reading this variable. Would this patch fix this for you?:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1855,10 +1855,6 @@ set(_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated")
 set(_project_config "${_generated_dir}/${PROJECT_NAME}Config.cmake")
 set(_version_config "${_generated_dir}/${PROJECT_NAME}ConfigVersion.cmake")
 
-cmake_dependent_option(BUILD_TESTING "Build tests"
-  ON "PERL_FOUND;NOT CURL_DISABLE_TESTS"
-  OFF)
-
 if(HAVE_MANUAL_TOOLS)
   set(CURL_MANPAGE "${CURL_BINARY_DIR}/docs/cmdline-opts/curl.1")
   set(CURL_ASCIIPAGE "${CURL_BINARY_DIR}/docs/cmdline-opts/curl.txt")
@@ -1876,7 +1872,7 @@ if(BUILD_EXAMPLES)
   add_subdirectory(docs/examples)
 endif()
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PERL_FOUND AND NOT CURL_DISABLE_TESTS)
   add_subdirectory(tests)
 endif()
 
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index a1539e22a3..0579b9ea2e 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -46,7 +46,7 @@ if(USE_ARES)
   include_directories(SYSTEM ${CARES_INCLUDE_DIRS})
 endif()
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PERL_FOUND AND NOT CURL_DISABLE_TESTS)
   add_library(
     curlu  # special libcurlu library just for unittests
     STATIC

[vszakats]

@robertmaynard Or would this be better, by keeping an option() instead of cmake_dependent_option() and the patch above?:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1855,9 +1855,7 @@ set(_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated")
 set(_project_config "${_generated_dir}/${PROJECT_NAME}Config.cmake")
 set(_version_config "${_generated_dir}/${PROJECT_NAME}ConfigVersion.cmake")
 
-cmake_dependent_option(BUILD_TESTING "Build tests"
-  ON "PERL_FOUND;NOT CURL_DISABLE_TESTS"
-  OFF)
+option(BUILD_TESTING "Build tests" ON)
 
 if(HAVE_MANUAL_TOOLS)
   set(CURL_MANPAGE "${PROJECT_BINARY_DIR}/docs/cmdline-opts/curl.1")
@@ -1876,7 +1874,7 @@ if(BUILD_EXAMPLES)
   add_subdirectory(docs/examples)
 endif()
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PERL_FOUND AND NOT CURL_DISABLE_TESTS)
   add_subdirectory(tests)
 endif()
 
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index 3e8f196682..c43932d03d 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -46,7 +46,7 @@ if(USE_ARES)
   include_directories(SYSTEM ${CARES_INCLUDE_DIRS})
 endif()
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PERL_FOUND AND NOT CURL_DISABLE_TESTS)
   add_library(
     curlu  # special libcurlu library just for unittests
     STATIC

curl dependencies libnghttp2, libnghttp3, libngtcp2 and libssh2 all use option() on this variable, which makes me think this might be OK.

---

Links:
https://cmake.org/cmake/help/latest/command/enable_testing.html
https://cmake.org/cmake/help/book/mastering-cmake/chapter/Testing%20With%20CMake%20and%20CTest.html

[robertmaynard]

CMake documents variables at https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html but we missed adding BUILD_TESTING which I have fixed ( https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9927 ).

Using option instead of cmake_dependent_option would be an excellent solution since option doesn't force overwrite any existing cache value, or create a cache variable when a local variable with that name exists ( behavior added in CMake 3.13 https://cmake.org/cmake/help/latest/policy/CMP0077.html ).

[vszakats]

Nice, thanks! I've opened #15355 with a proposed fix, would you mind taking a look/try and tell us if it fixes the problem?