From 25f575e8513a7e49ac73c7e1cb24dcaf3803358e Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Wed, 23 Oct 2024 17:19:00 +0200 Subject: [PATCH] CMake: Run finalizers for test-like executables on all platforms Previously we ran some finalizer functions for tests and manual tests, only for specific platforms, and via different code paths. This change introduces a new unified way of running all finalizers for all test-like executables, including benchmarks. To ensure a smoother transition, the new way is opt-out, and the old way can be enabled by setting the QT_INTERNAL_SKIP_TEST_FINALIZERS_V2 variable to true in case we encounter some issues in the CI. The finalizers are only run for test-like executables, and not all internal executables, because there are some unsolved issues there. One particular case is in qtdeclarative where that will create a cycle for qmlimportscanner to depend on itself. A proper solution here would be to have some kind of mapping or mechanism to exclude finalizers for targets where they would try to run themselves. Pick-to: 6.8 Task-number: QTBUG-93625 Task-number: QTBUG-112212 Change-Id: I52b3a1c02c298c4a18ce2c75d7e491ae79d191a0 Reviewed-by: Alexey Edelev Reviewed-by: Joerg Bornemann --- cmake/QtExecutableHelpers.cmake | 36 ++++++++++++++++++++++++++++----- cmake/QtTestHelpers.cmake | 8 ++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/cmake/QtExecutableHelpers.cmake b/cmake/QtExecutableHelpers.cmake index 975f3806ea2..05b3603e048 100644 --- a/cmake/QtExecutableHelpers.cmake +++ b/cmake/QtExecutableHelpers.cmake @@ -38,11 +38,37 @@ function(qt_internal_add_executable name) _qt_is_benchmark_test ${arg_QT_BENCHMARK_TEST} ) - if(ANDROID) - _qt_internal_android_executable_finalizer(${name}) - endif() - if(WASM) - qt_internal_wasm_add_finalizers(${name}) + # Opt out to skip the new way of running test finalizers, and instead use the old way for + # specific platforms. + # TODO: Remove once we confirm that the new way of running test finalizers for all platforms + # doesn't cause any issues. + if(NOT QT_INTERNAL_SKIP_TEST_FINALIZERS_V2) + # We don't run finalizers for all executables on all platforms, because there are still + # some unsolved issues there. One of them is trying to run finalizers for the + # qmlimportscanner executable would create a circular depenendecy trying to run + # qmlimportscanner on itself. + # + # For now, we only run finalizers for test-like executables on all platforms, and all + # android and wasm internal executables. + # For android and wasm all executables, to be behavior compatible with the old way of + # running finalizers. + if(ANDROID + OR WASM + OR arg_QT_TEST + OR arg_QT_MANUAL_TEST + OR arg_QT_BENCHMARK_TEST) + set(QT_INTERNAL_USE_POOR_MANS_SCOPE_FINALIZER TRUE) + _qt_internal_finalize_target_defer("${name}") + endif() + else() + if(ANDROID) + # This direct calls the finalizer, which in the v2 way is deferred. + _qt_internal_android_executable_finalizer(${name}) + endif() + if(WASM) + # This defer calls the finalizer. + qt_internal_wasm_add_finalizers(${name}) + endif() endif() if(arg_QT_APP AND QT_FEATURE_debug_and_release AND CMAKE_VERSION VERSION_GREATER_EQUAL "3.19.0") diff --git a/cmake/QtTestHelpers.cmake b/cmake/QtTestHelpers.cmake index 87cefada04e..8bacc141525 100644 --- a/cmake/QtTestHelpers.cmake +++ b/cmake/QtTestHelpers.cmake @@ -1126,6 +1126,14 @@ function(qt_internal_collect_command_environment out_path out_plugin_path) endfunction() function(qt_internal_add_test_finalizers target) + # Opt out to skip the new way of running test finalizers, and instead use the old way for + # specific platforms. + # TODO: Remove once we confirm that the new way of running test finalizers for all platforms + # doesn't cause any issues. + if(QT_INTERNAL_SKIP_TEST_FINALIZERS_V2) + return() + endif() + # It might not be safe to run all the finalizers of _qt_internal_finalize_executable # within the context of a Qt build (not a user project) when targeting a host build. # At least one issue is missing qmlimportscanner at configure time.