From a79056bb023ae99c2e0f09f49b5d525e7b3eff8a Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Fri, 31 Jul 2020 17:50:37 +0200 Subject: AutoGen: Fix over-specified direct dependencies of custom command The AutoMoc timestamp creating custom command explicitly depended on all dependencies of the origin target (associated to the AutoGen target). When an origin target depended on a shared library 'libfoo.so', if it was re-linked, the AutoMoc custom command would touch its output timestamp file, and thus cause needless rebuilding of sources, despite the shared library not having any influence on the AutoMoc generated files. Introduce a new '_autogen_timestamp_deps' utility target, which will serve as an 'order-only' dependency for the custom command. This will prevent needless rebuilding, because touching 'libfoo.so' will not cause the custom command to be re-executed. The new AutoMoc dependency tree looks like: '_autogen_timestamp_deps (serves as order-only dep)' <- '/timestamp' file ( + moc deps file) <- '_autogen' target. Fixes: #21020 --- Source/cmQtAutoGenInitializer.cxx | 47 +++++++++++++++++++++++++++++++++++++-- Source/cmQtAutoMocUic.cxx | 4 +++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index 629367d..c8caddf 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -1180,11 +1180,54 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() if (useNinjaDepfile) { // Create a custom command that generates a timestamp file and // has a depfile assigned. The depfile is created by JobDepFilesMergeT. - - // Add additional autogen target dependencies + // + // Also create an additional '_autogen_timestamp_deps' that the custom + // command will depend on. It will have no sources or commands to + // execute, but it will have dependencies that would originally be + // assigned to the pre-Qt 5.15 'autogen' target. These dependencies will + // serve as a list of order-only dependencies for the custom command, + // without forcing the custom command to re-execute. + // + // The dependency tree would then look like + // '_autogen_timestamp_deps (order-only)' <- '/timestamp' file <- + // '_autogen' target. + const auto timestampTargetName = + cmStrCat(this->GenTarget->GetName(), "_autogen_timestamp_deps"); + std::vector timestampTargetProvides; + cmCustomCommandLines timestampTargetCommandLines; + + // Add additional autogen target dependencies to + // '_autogen_timestamp_deps'. for (const cmTarget* t : this->AutogenTarget.DependTargets) { dependencies.push_back(t->GetName()); } + + cmTarget* timestampTarget = this->LocalGen->AddUtilityCommand( + timestampTargetName, true, this->Dir.Work.c_str(), + /*byproducts=*/timestampTargetProvides, + /*depends=*/dependencies, timestampTargetCommandLines, false, nullptr); + this->LocalGen->AddGeneratorTarget( + cm::make_unique(timestampTarget, this->LocalGen)); + + // Set FOLDER property on the timestamp target, so it appears in the + // appropriate folder in an IDE or in the file api. + if (!this->TargetsFolder.empty()) { + timestampTarget->SetProperty("FOLDER", this->TargetsFolder); + } + + // Make '/timestamp' file depend on '_autogen_timestamp_deps' and on the + // moc and uic executables (whichever are enabled). + dependencies.clear(); + dependencies.push_back(timestampTargetName); + + if (this->Moc.ExecutableTarget != nullptr) { + dependencies.push_back(this->Moc.ExecutableTarget->Target->GetName()); + } + if (this->Uic.ExecutableTarget != nullptr) { + dependencies.push_back(this->Uic.ExecutableTarget->Target->GetName()); + } + + // Create the custom command that outputs the timestamp file. const char timestampFileName[] = "timestamp"; const std::string outputFile = cmStrCat(this->Dir.Build, "/", timestampFileName); diff --git a/Source/cmQtAutoMocUic.cxx b/Source/cmQtAutoMocUic.cxx index 893bd6b..f159a3d 100644 --- a/Source/cmQtAutoMocUic.cxx +++ b/Source/cmQtAutoMocUic.cxx @@ -2163,7 +2163,9 @@ std::string escapeDependencyPath(cm::string_view path) void cmQtAutoMocUicT::JobDepFilesMergeT::Process() { if (Log().Verbose()) { - Log().Info(GenT::MOC, "Merging MOC dependencies"); + Log().Info(GenT::MOC, + cmStrCat("Merging MOC dependencies into ", + MessagePath(BaseConst().DepFile.c_str()))); } auto processDepFile = [](const std::string& mocOutputFile) -> std::vector { -- cgit v0.12 From 7445c9a58a2444c8918e81a9264b1584001ca013 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Mon, 3 Aug 2020 12:23:07 +0200 Subject: AutoGen: Add test to check for correct AutoMoc dependencies When using Qt 5.15.0 or above together with Ninja, check that touching a source file of a dependency does not needlessly re-run AUTOMOC for the dependee target. --- Tests/RunCMake/CMakeLists.txt | 3 +++ Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake | 9 +++++++++ Tests/RunCMake/Ninja/RunCMakeTest.cmake | 21 +++++++++++++++++++++ Tests/RunCMake/Ninja/app.cpp | 6 ++++++ Tests/RunCMake/Ninja/app_qt.cpp | 11 +++++++++++ Tests/RunCMake/Ninja/simple_lib.cpp | 6 ++++++ 6 files changed, 56 insertions(+) create mode 100644 Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake create mode 100644 Tests/RunCMake/Ninja/app.cpp create mode 100644 Tests/RunCMake/Ninja/app_qt.cpp create mode 100644 Tests/RunCMake/Ninja/simple_lib.cpp diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index e9f8bca..0b2ef6a 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -134,6 +134,9 @@ if(CMAKE_GENERATOR MATCHES "Ninja") if(CMAKE_Fortran_COMPILER) list(APPEND Ninja_ARGS -DTEST_Fortran=1) endif() + if(CMake_TEST_Qt5 AND Qt5Core_FOUND) + list(APPEND Ninja_ARGS -DCMake_TEST_Qt5=1 -DCMAKE_TEST_Qt5Core_Version=${Qt5Core_VERSION}) + endif() add_RunCMake_test(Ninja) set(NinjaMultiConfig_ARGS -DCYGWIN=${CYGWIN} diff --git a/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake b/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake new file mode 100644 index 0000000..d69a119 --- /dev/null +++ b/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake @@ -0,0 +1,9 @@ +enable_language(CXX) + +find_package(Qt5Core REQUIRED) + +set(CMAKE_AUTOMOC ON) + +add_library(simple_lib SHARED simple_lib.cpp) +add_executable(app_with_qt app.cpp app_qt.cpp) +target_link_libraries(app_with_qt PRIVATE simple_lib Qt5::Core) diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index 0eada08..c1f6346 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -132,6 +132,7 @@ ${ninja_stderr} message(FATAL_ERROR "top ninja build failed exited with status ${ninja_result}") endif() + set(ninja_stdout "${ninja_stdout}" PARENT_SCOPE) endfunction(run_ninja) function (run_LooseObjectDepends) @@ -316,3 +317,23 @@ function (run_ChangeBuildType) run_ninja("${RunCMake_TEST_BINARY_DIR}" -w dupbuild=err) endfunction() run_ChangeBuildType() + +function(run_Qt5AutoMocDeps) + if(CMake_TEST_Qt5 AND CMAKE_TEST_Qt5Core_Version VERSION_GREATER_EQUAL 5.15.0) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/Qt5AutoMocDeps-build) + run_cmake(Qt5AutoMocDeps) + unset(RunCMake_TEST_OPTIONS) + # Build the project. + run_ninja("${RunCMake_TEST_BINARY_DIR}") + # Touch just the library source file, which shouldn't cause a rerun of AUTOMOC + # for app_with_qt target. + touch("${RunCMake_SOURCE_DIR}/simple_lib.cpp") + # Build and assert that AUTOMOC was not run for app_with_qt. + run_ninja("${RunCMake_TEST_BINARY_DIR}") + if(ninja_stdout MATCHES "Automatic MOC for target app_with_qt") + message(FATAL_ERROR + "AUTOMOC should not have executed for 'app_with_qt' target:\nstdout:\n${ninja_stdout}") + endif() + endif() +endfunction() +run_Qt5AutoMocDeps() diff --git a/Tests/RunCMake/Ninja/app.cpp b/Tests/RunCMake/Ninja/app.cpp new file mode 100644 index 0000000..57380e4 --- /dev/null +++ b/Tests/RunCMake/Ninja/app.cpp @@ -0,0 +1,6 @@ +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + return 0; +} diff --git a/Tests/RunCMake/Ninja/app_qt.cpp b/Tests/RunCMake/Ninja/app_qt.cpp new file mode 100644 index 0000000..302c672 --- /dev/null +++ b/Tests/RunCMake/Ninja/app_qt.cpp @@ -0,0 +1,11 @@ +#include + +class Mango : public QObject +{ + Q_OBJECT +public: +Q_SIGNALS: + void eatFruit(); +}; + +#include "app_qt.moc" diff --git a/Tests/RunCMake/Ninja/simple_lib.cpp b/Tests/RunCMake/Ninja/simple_lib.cpp new file mode 100644 index 0000000..cf8d689 --- /dev/null +++ b/Tests/RunCMake/Ninja/simple_lib.cpp @@ -0,0 +1,6 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif + void dummy_symbol() +{ +} -- cgit v0.12