]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
cmake: make `test-ci` target skip building dependencies
authorViktor Szakats <commit@vsz.me>
Sun, 22 Sep 2024 10:01:36 +0000 (12:01 +0200)
committerViktor Szakats <commit@vsz.me>
Mon, 23 Sep 2024 09:52:55 +0000 (11:52 +0200)
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

index 22226b4a41e28de422a7e17903983d4c2cec3c50..b42c93df07a7b78ffa2c9ef4b93db5ba99954cb7 100644 (file)
@@ -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()