cmake: Add CURL_BUILD_TESTING option

[reddwarf69]

To let the top level project decide whether to enable the curl tests
when curl is a subproject.

[jzakrzewski]

I understand the idea and I even think curls CMake should introduce more project-specific switches to control standard features exactly to give control to top-level projects.

However, I don't like that if you set the CURL_BUILD_TESTING, the build is still allowed to complete without error even though perl may not be there.

Similarly, if you set CURL_BUILD_TESTING=NO but curl is top-level project, it gets ignored.

[reddwarf69]

I understand the idea and I even think curls CMake should introduce more project-specific switches to control standard features exactly to give control to top-level projects.

However, I don't like that if you set the CURL_BUILD_TESTING, the build is still allowed to complete without error even though perl may not be there.

Similarly, if you set CURL_BUILD_TESTING=NO but curl is top-level project, it gets ignored.

As it currently is if you set the BUILD_TESTING, the build is still allowed to complete without error even though perl may not be there.
And you get the "Perl not found, testing disabled." warning even if you set BUILD_TESTING to OFF.

I'm fine changing it, but AFAICT it would require changes I'm not sure everybody would agree to.

Would everybody agree to:

• Both BUILD_TESTING and CURL_BUILD_TESTING are ON by default
• Testing is only enabled if both are ON
• Both being ON and perl not being available is a hard error

?

So

• If you don't have perl you have to set either (or both) variables to OFF or the configuration step will simply fail. No automatic disabling with warning message for you.
• If you are using curl as the top-level project and explicitly set one to ON and the other to OFF... what's wrong with you? Unsupported case, but the tests will be disabled.
• If you explicitly set any or both to ON it just keeps the default
• If you set any to OFF testing is disabled

[jzakrzewski]

As it currently is if you set the BUILD_TESTING, the build is still allowed to complete without error even though perl may not be there.
And you get the "Perl not found, testing disabled." warning even if you set BUILD_TESTING to OFF.

Meh, I haven't checked the tests/CMakeLists.txt...
The way I see it, CURL_BUILD_TESTING should be THE switch to use (two switches controlling the same is baaad...). It should be an option() probably initialized with CMAKE_SOURCE_DIR STREQUAL CURL_SOURCE_DIR AND PERL_FOUND. If ON, it would set BUILD_TESTING (otherwise unsetting it?) and, because the custom targets are invalid without perl, there should be an explicit error if testing is enabled but perl is not found.

At least that's how I see it. Anyone else is welcome to chime in.

[reddwarf69]

The way I see it, CURL_BUILD_TESTING should be THE switch to use (two switches controlling the same is baaad...).

The problem is that the CTest Module has all but standardized BUILD_TESTING to control testing. People expect it to be there.

[jzakrzewski]

Yeah, I know. The same is with few other variables. The problem begins when a project is included via add_subdirectory() because the settings start conflicting with each other. This is why I always strongly recommended to avoid including projects that way. I realised that this trend cannot be stopped and now I'm looking for resources to recommend best practises to make that scenario working good while not compromising standard usage. I'm yet to find a good advises on the topic.

So what about turning it all around and using CURL_DISABLE_TESTING - an option() probably initialized with NOT ( CMAKE_SOURCE_DIR STREQUAL CURL_SOURCE_DIR ). If set, it would unconditionally unset BUILD_TESTING, otherwise BUILD_TESTING can be deduced/set (and I'd really error out if testing is requested but perl is not found).

[reddwarf69]

Yeah, I know. The same is with few other variables. The problem begins when a project is included via add_subdirectory() because the settings start conflicting with each other. This is why I always strongly recommended to avoid including projects that way. I realised that this trend cannot be stopped and now I'm looking for resources to recommend best practises to make that scenario working good while not compromising standard usage. I'm yet to find a good advises on the topic.

As I see it it's such a common practice that not supporting it is a bug.

So what about turning it all around and using CURL_DISABLE_TESTING - an option() probably initialized with NOT ( CMAKE_SOURCE_DIR STREQUAL CURL_SOURCE_DIR ). If set, it would unconditionally unset BUILD_TESTING, otherwise BUILD_TESTING can be deduced/set (and I'd really error out if testing is requested but perl is not found).

Playing with CACHE variables in that way scares me, specially with something like BUILD_TESTING. Are you suggesting using set() with FORCE?

I would still go for #6036 (comment). Yes, there are two variables, but:

• BUILD_TESTING is the "global" testing enabling flag. In the common case where curl is the top project... there if only one project, so there is no difference between "global" and "project" testing enabling flags. But people do know BUILD_TESTING is the global testing enabling flag. They know CTest sets it to ON by default. They are not going to think twice about it, if they want to disable testing they will set BUILD_TESTING to OFF. Somebody will go and use CURL_BUILD_TESTING to disable testing... it's OK, it will also work.
  Basically BUILD_TESTING should be called CMAKE_BUILD_TESTING because it's something cmake owns. It isn't, cmake is like that, but no big deal we are used to it.
  It's not like curl is introducing two variables. It's introducing only one, CURL_BUILD_TESTING, and honouring the cmake one: BUILD_TESTING.

• And if the top project is something else then the two flags are actually needed. BUILD_TESTING is really a "global" testing enabling flag: it's something that curl uses, but not something curl owns. If the top project has disabled BUILD_TESTING it means it doesn't want testing anywhere, and curl is simply honouring that. And if it just wants to disable curl's testing CURL_BUILD_TESTING is how you would expect the setting to be named.

[jzakrzewski]

You make valid points. I would still kinda prefer the negative "DISABLE" variable but maybe it's just me being weird ;). I'd really appreciate another voice in this discussion. @Lekensteyn ? @bagder ?

[bagder]

I trust your hunch more than mine on this @jzakrzewski. I have almost zero experience with cmake and I can't judge which of these options makes the best end solution!

[snikulov]

@jzakrzewski @reddwarf69 +1 for CURL_DISABLE_TESTS.
Also CMake has an issue to disable tests for add_subdirectory(<project> EXCLUDE_FROM_ALL) for cases with super-CMakeLists.txt scripting, described by @reddwarf69 .

One possible solution is to use cmake_dependent_option

+cmake_dependent_option(BUILD_TESTING "Build tests"
+  ON "PERL_FOUND;NOT CURL_DISABLE_TESTS"
+  OFF)
+if(BUILD_TESTING)
   add_subdirectory(tests)
 endif()

[reddwarf69]

At the end I am using the autotools build so I'm not going to be spending time on this any time soon.