Skip to content

Commit

Permalink
CMake: Run finalizers for test-like executables on all platforms
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Joerg Bornemann <[email protected]>
  • Loading branch information
alcroito committed Nov 6, 2024
1 parent 81e7d3f commit 25f575e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
36 changes: 31 additions & 5 deletions cmake/QtExecutableHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 8 additions & 0 deletions cmake/QtTestHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 25f575e

Please sign in to comment.