Merge topic 'ninja-automoc-cycle' into release-3.20

ca7c76269b Tests: Add test for Ninja automoc dependency cyle
54ad3e4958 autogen: Don't include SKIP_AUTOMOC files in depfile

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Jörg Bornemann <joerg.bornemann@qt.io>
Acked-by: Craig Scott <craig.scott@crascit.com>
Merge-request: !5956

8 files changed, 61 insertions, 20 deletions

diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx
index d4138d9..a1816f1 100644
--- a/Source/cmQtAutoGenInitializer.cxx
+++ b/Source/cmQtAutoGenInitializer.cxx
@@ -1463,15 +1463,15 @@ bool cmQtAutoGenInitializer::SetupWriteAutogenInfo()
     headers.reserve(this->AutogenTarget.Headers.size());
     for (auto const& pair : this->AutogenTarget.Headers) {
       MUFile const* const muf = pair.second.get();
-      if (muf->Generated && !this->CMP0071Accept) {
-        continue;
-      }
       if (muf->SkipMoc) {
         moc_skip.insert(muf->FullPath);
       }
       if (muf->SkipUic) {
         uic_skip.insert(muf->FullPath);
       }
+      if (muf->Generated && !this->CMP0071Accept) {
+        continue;
+      }
       if (muf->MocIt || muf->UicIt) {
         headers.emplace_back(muf);
       }
diff --git a/Source/cmQtAutoMocUic.cxx b/Source/cmQtAutoMocUic.cxx
index 1e3bf06..535f786 100644
--- a/Source/cmQtAutoMocUic.cxx
+++ b/Source/cmQtAutoMocUic.cxx
@@ -2248,6 +2248,13 @@ void cmQtAutoMocUicT::JobDepFilesMergeT::Process()
   std::for_each(this->MocEval().SourceMappings.begin(),
                 this->MocEval().SourceMappings.end(), processMappingEntry);
 
+  // Remove SKIP_AUTOMOC files
+  dependencies.erase(std::remove_if(dependencies.begin(), dependencies.end(),
+                                    [this](const std::string& dep) {
+                                      return this->MocConst().skipped(dep);
+                                    }),
+                     dependencies.end());
+
   // Remove duplicates to make the depfile smaller
   std::sort(dependencies.begin(), dependencies.end());
   dependencies.erase(std::unique(dependencies.begin(), dependencies.end()),
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index 91fe6ca..5ee0612 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -141,6 +141,9 @@ endif()
 if(CMAKE_GENERATOR MATCHES "Make")
   add_RunCMake_test(Make -DMAKE_IS_GNU=${MAKE_IS_GNU})
 endif()
+if(CMake_TEST_Qt5)
+  find_package(Qt5Widgets QUIET NO_MODULE)
+endif()
 if(CMAKE_GENERATOR MATCHES "Ninja")
   set(Ninja_ARGS
     -DCMAKE_C_OUTPUT_EXTENSION=${CMAKE_C_OUTPUT_EXTENSION}
@@ -151,6 +154,9 @@ if(CMAKE_GENERATOR MATCHES "Ninja")
   endif()
   if(CMake_TEST_Qt5 AND Qt5Core_FOUND)
     list(APPEND Ninja_ARGS -DCMake_TEST_Qt5=1 -DQt5Core_DIR=${Qt5Core_DIR} -DCMAKE_TEST_Qt5Core_Version=${Qt5Core_VERSION})
+    if(Qt5Widgets_FOUND)
+      list(APPEND Ninja_ARGS -DQt5Widgets_DIR=${Qt5Widgets_DIR})
+    endif()
   endif()
   add_RunCMake_test(Ninja)
   set(NinjaMultiConfig_ARGS
@@ -190,9 +196,6 @@ if(CMake_TEST_APPLE_SILICON)
   add_RunCMake_test(AppleSilicon)
 endif()
 set(autogen_with_qt5 FALSE)
-if(CMake_TEST_Qt5)
-  find_package(Qt5Widgets QUIET NO_MODULE)
-endif()
 if(CMake_TEST_Qt5 AND Qt5Widgets_FOUND)
   set(autogen_with_qt5 TRUE)
 endif ()
diff --git a/Tests/RunCMake/Ninja/MyWindow.cpp b/Tests/RunCMake/Ninja/MyWindow.cpp
new file mode 100644
index 0000000..d87c2e9
--- /dev/null
+++ b/Tests/RunCMake/Ninja/MyWindow.cpp
@@ -0,0 +1,7 @@
+#include "MyWindow.h"
+
+MyWindow::MyWindow(QWidget* parent)
+  : QWidget(parent)
+{
+  this->m_ui.setupUi(this);
+}
diff --git a/Tests/RunCMake/Ninja/MyWindow.h b/Tests/RunCMake/Ninja/MyWindow.h
new file mode 100644
index 0000000..c267610
--- /dev/null
+++ b/Tests/RunCMake/Ninja/MyWindow.h
@@ -0,0 +1,16 @@
+#pragma once
+
+#include <QWidget>
+
+#include "ui_MyWindow.h"
+
+class MyWindow : public QWidget
+{
+  Q_OBJECT
+
+public:
+  explicit MyWindow(QWidget* parent = nullptr);
+
+private:
+  Ui::MyWindow m_ui;
+};
diff --git a/Tests/RunCMake/Ninja/MyWindow.ui b/Tests/RunCMake/Ninja/MyWindow.ui
new file mode 100644
index 0000000..fbf294c
--- /dev/null
+++ b/Tests/RunCMake/Ninja/MyWindow.ui
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>MyWindow</class>
+ <widget class="QWidget" name="MyWindow"/>
+</ui>
diff --git a/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake b/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake
index 7456608..46b840f 100644
--- a/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake
+++ b/Tests/RunCMake/Ninja/Qt5AutoMocDeps.cmake
@@ -8,6 +8,14 @@ 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)
 
+if(Qt5Widgets_DIR)
+  find_package(Qt5Widgets REQUIRED)
+  qt5_wrap_ui(_headers MyWindow.ui)
+  add_executable(app_with_widget app.cpp MyWindow.cpp ${_headers})
+  target_link_libraries(app_with_widget PRIVATE Qt5::Widgets)
+  target_include_directories(app_with_widget PRIVATE "${CMAKE_BINARY_DIR}")
+endif()
+
 add_subdirectory(QtSubDir1)
 add_subdirectory(QtSubDir2)
 add_subdirectory(QtSubDir3)
diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake
index 1b252cd..0c0619d 100644
--- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake
+++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake
@@ -186,16 +186,6 @@ function(sleep delay)
   endif()
 endfunction(sleep)
 
-function(touch path)
-  execute_process(
-    COMMAND ${CMAKE_COMMAND} -E touch ${path}
-    RESULT_VARIABLE result
-    )
-  if(NOT result EQUAL 0)
-    message(FATAL_ERROR "failed to touch main ${path} file.")
-  endif()
-endfunction(touch)
-
 macro(ninja_escape_path path out)
   string(REPLACE "\$ " "\$\$" "${out}" "${path}")
   string(REPLACE " " "\$ " "${out}" "${${out}}")
@@ -264,8 +254,8 @@ build build.ninja: RERUN ${escaped_build_ninja_dep} || ${escaped_ninja_output_pa
   # Test regeneration rules run in order.
   set(main_cmakelists "${RunCMake_SOURCE_DIR}/CMakeLists.txt")
   sleep(${fs_delay})
-  touch("${main_cmakelists}")
-  touch("${build_ninja_dep}")
+  file(TOUCH "${main_cmakelists}")
+  file(TOUCH "${build_ninja_dep}")
   run_ninja("${top_build_dir}")
   file(TIMESTAMP "${main_cmakelists}" mtime_main_cmakelists UTC)
   file(TIMESTAMP "${sub_build_ninja}" mtime_sub_build_ninja UTC)
@@ -329,14 +319,14 @@ 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)
-    set(RunCMake_TEST_OPTIONS "-DQt5Core_DIR=${Qt5Core_DIR}")
+    set(RunCMake_TEST_OPTIONS "-DQt5Core_DIR=${Qt5Core_DIR}" "-DQt5Widgets_DIR=${Qt5Widgets_DIR}")
     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")
+    file(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")
@@ -352,6 +342,11 @@ function(run_Qt5AutoMocDeps)
       message(FATAL_ERROR
         "AUTOMOC should not have executed for 'sub_exe_2' target:\nstdout:\n${ninja_stdout}")
     endif()
+    # Touch a header file to make sure an automoc dependency cycle is not introduced.
+    file(TOUCH "${RunCMake_SOURCE_DIR}/MyWindow.h")
+    run_ninja("${RunCMake_TEST_BINARY_DIR}")
+    # Need to run a second time to hit the dependency cycle.
+    run_ninja("${RunCMake_TEST_BINARY_DIR}")
   endif()
 endfunction()
 run_Qt5AutoMocDeps()