1
0
Эх сурвалжийг харах

Autogen: Fix missing autogen dependencies if a target is linked twice

Autogen contains code to forward dependencies from an origin target
to its associated <origin>_autogen target. This code also contains a
check to skip forwarding a dependency if it does not appear in the
dependency graph for all configured build types.
This is done by counting the number of times a dependency appears in
the graph for each configured build type.

Unfortunately the code did not account for the case when a dependency
appears more than once in the link graph for a single build type. This
means that for a single-config build, if the same dependency is linked
twice, the dependency will be skipped altogether.

This can lead to build errors in a project where a hypothetical App
target depends on a Lib target, and thus expects App_autogen to depend
on Lib_autogen and any of its dependencies, but the latter is skipped.

Fix this by incrementing the count of a target in the dependency graph
only once per build type.

Fixes: #26700
Alexandru Croitor 8 сар өмнө
parent
commit
2b314e9009

+ 8 - 4
Source/cmQtAutoGenInitializer.cxx

@@ -1451,8 +1451,11 @@ bool cmQtAutoGenInitializer::InitAutogenTarget()
     if (this->AutogenTarget.DependOrigin) {
       // add_dependencies/addUtility do not support generator expressions.
       // We depend only on the libraries found in all configs therefore.
-      std::map<cmGeneratorTarget const*, std::size_t> commonTargets;
+      std::map<cmGeneratorTarget const*, std::size_t> targetsPartOfAllConfigs;
       for (std::string const& config : this->ConfigsList) {
+        // The same target might appear multiple times in a config, but we
+        // should only count it once.
+        std::set<cmGeneratorTarget const*> seenTargets;
         cmLinkImplementationLibraries const* libs =
           this->GenTarget->GetLinkImplementationLibraries(
             config, cmGeneratorTarget::UseTo::Link);
@@ -1460,14 +1463,15 @@ bool cmQtAutoGenInitializer::InitAutogenTarget()
           for (cmLinkItem const& item : libs->Libraries) {
             cmGeneratorTarget const* libTarget = item.Target;
             if (libTarget &&
-                !StaticLibraryCycle(this->GenTarget, libTarget, config)) {
+                !StaticLibraryCycle(this->GenTarget, libTarget, config) &&
+                seenTargets.insert(libTarget).second) {
               // Increment target config count
-              commonTargets[libTarget]++;
+              targetsPartOfAllConfigs[libTarget]++;
             }
           }
         }
       }
-      for (auto const& item : commonTargets) {
+      for (auto const& item : targetsPartOfAllConfigs) {
         if (item.second == this->ConfigsList.size()) {
           this->AutogenTarget.DependTargets.insert(item.first->Target);
         }

+ 38 - 0
Tests/RunCMake/Autogen_1/AutogenDuplicateDependency.cmake

@@ -0,0 +1,38 @@
+enable_language(CXX)
+
+find_package(Qt${with_qt_version} REQUIRED COMPONENTS Core)
+
+set(CMAKE_AUTOMOC ON)
+
+file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/lib.cpp"
+     CONTENT "void foo() {}")
+file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/main.cpp"
+     CONTENT "int main() {return 0;}")
+
+# Test case.
+# App depends on Lib, which means App_autogen depends on Lib_autogen by default.
+# Which means building App_autogen should trigger the generation of the
+# fancy_generated.txt file, which is a dependency of Lib_autogen.
+
+# Create a shared library and an executable.
+add_library(Lib SHARED "${CMAKE_CURRENT_BINARY_DIR}/lib.cpp")
+add_executable(App "${CMAKE_CURRENT_BINARY_DIR}/main.cpp")
+
+# Link Lib into App more than once. Previously this was not properly handled by AUTOGEN,
+# which discarded the Lib_autogen dependency from App_autogen entirely, and the
+# file was not generated.
+foreach(i RANGE 1 ${LIB_LINK_COUNT})
+  target_link_libraries(App PRIVATE Lib)
+endforeach()
+
+# Add a custom target that generates a file.
+set(generated_file_path "${CMAKE_CURRENT_BINARY_DIR}/fancy_generated.txt")
+add_custom_command(
+    OUTPUT "${generated_file_path}"
+    COMMAND ${CMAKE_COMMAND} -E touch "${generated_file_path}"
+)
+add_custom_target(generate_file DEPENDS "${generated_file_path}")
+
+# Make sure the file is generated as part of building the Lib_autogen target.
+set_target_properties(Lib PROPERTIES
+    AUTOGEN_TARGET_DEPENDS generate_file)

+ 15 - 0
Tests/RunCMake/Autogen_1/RunCMakeTest.cmake

@@ -131,4 +131,19 @@ if (DEFINED with_qt_version)
       endblock()
     endif()
   endif()
+  block()
+    foreach(LIB_LINK_COUNT RANGE 1 4)
+      set(RunCMake_TEST_VARIANT_DESCRIPTION "-link-count-${LIB_LINK_COUNT}")
+      set(RunCMake_TEST_BINARY_DIR
+        "${RunCMake_BINARY_DIR}/AutogenDuplicateDependency-build-${LIB_LINK_COUNT}")
+      run_cmake_with_options(AutogenDuplicateDependency ${RunCMake_TEST_OPTIONS} -DLIB_LINK_COUNT=${LIB_LINK_COUNT})
+      set(RunCMake_TEST_NO_CLEAN 1)
+      set(RunCMake_TEST_EXPECT_stdout "fancy_generated.txt")
+      run_cmake_command("AutogenDuplicateDependency-build"
+        ${CMAKE_COMMAND} --build . --target App_autogen --verbose)
+      unset(RunCMake_TEST_VARIANT_DESCRIPTION)
+      unset(RunCMake_TEST_EXPECT_stdout)
+      unset(RunCMake_TEST_NO_CLEAN)
+    endforeach()
+  endblock()
 endif ()