From 7b88bb44432fd8c510484ece305669372f2f1982 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Sun, 22 Sep 2024 12:01:36 +0200 Subject: [PATCH] cmake: make `test-ci` target skip building dependencies Make `test-ci` not depend on the `testdeps` target. `test-ci` is designed to run curl tests in CI. In CI we build all necessary dependencies explicitly beforehand, and they are always ready when calling the `test-ci` step. Thus, it isn't necessary to enforce them via a dependency rule. Dropping it saves redundant work and delay in CI jobs. The `testdeps` dependency should not normally be a problem. It's supposed to be a no-op if those targets are already built. In practice however, it causes a delay and/or an actual rebuild of those dependencies depending on generator (= build tool) used and other factors. As observed in the GHA/windows workflow, the `testdeps` dependency: - with Ninja, causes no delay, and no extra work: https://github.com/curl/curl/actions/runs/10980099984/job/30485440389#step:25:18 - with GNU Make, caused a re-evaluation of `testdeps` targets, but did not actually rebuild them. This re-evaluation took a noticeable time (esp. with non-bundled tests): https://github.com/curl/curl/actions/runs/10980099984/job/30485440155#step:14:11 (with bundles) https://github.com/curl/curl/actions/runs/10973851013/job/30471690331#step:14:11 (w/o bundles) verbose: https://github.com/curl/curl/actions/runs/10980506956/job/30486434629#step:14:13 - with MSBuild, caused a re-evaluation of `testdeps` targets, and triggered a _rebuild_: https://github.com/curl/curl/actions/runs/10980099984/job/30485435968#step:14:19 (with bundles) https://github.com/curl/curl/actions/runs/10973851013/job/30471689714#step:14:19 (w/o bundles) verbose: https://github.com/curl/curl/actions/runs/10980506956/job/30486436368#step:14:48 It's suspected that our use of `-DCMAKE_VS_GLOBALS=TrackFileAccess=false` in CI is contributing to this. This option is supposed to affect incremental builds only. For some reason it affects CI builds too, even though they are not incremental, and no sources are changed between build steps. Reported-by: Aki Sakurai Ref: #14999 Notice that `test-*` targets depending on `testdeps` is NOT sufficient to build everything to run tests, e.g. it misses to build the curl tool, which is essential. This is also true for autotools' `test-ci` target which misses to build libcurl and the curl tool. Perhaps it'd be best to drop `testdeps` as a dependency for _all_ `test-*` targets and make it official to require building dependencies manually. Alternatively these targets could be fixed to rebuild everything necessary to run tests. Closes #15001 --- tests/CMakeLists.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 22226b4a41..b42c93df07 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -43,6 +43,13 @@ function(add_runtests _targetname _test_flags) if(CURL_TEST_BUNDLES) set(_test_flags "${_test_flags} -bundle") endif() + unset(_depends) + # Skip walking through dependent targets before running tests in CI. + # This avoids: GNU Make doing a slow re-evaluation of all targets and + # skipping them, MSBuild doing a re-evaluation, and actually rebuilding them. + if(NOT _targetname STREQUAL "test-ci") + set(_depends "testdeps") + endif() # Use a special '$TFLAGS' placeholder as last argument which will be # replaced by the contents of the environment variable in runtests.pl. # This is a workaround for CMake's limitation where commands executed by @@ -53,7 +60,7 @@ function(add_runtests _targetname _test_flags) "${PERL_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/runtests.pl" ${_test_flags_list} "\$TFLAGS" - DEPENDS testdeps + DEPENDS "${_depends}" VERBATIM USES_TERMINAL ) endfunction() -- 2.47.3