Browse Source

Fix dependencies on targets linked through object libraries

When an object library is used via `target_link_libraries`, any targets
listed in the object library's `INTERFACE_LINK_LIBRARIES` closure should
become direct dependencies of the consuming target.  However, these were
accidentally left out by `cmComputeTargetDepends::CollectTargetDepends`
because object libraries are encountered through external object sources
first and then added to the `emitted` set which blocks them from being
processed as link dependencies.

This was not noticed by the test case in commit bab24e782c
(target_link_libraries: Propagate dependencies of object libraries,
2018-12-10, v3.14.0-rc1~260^2) because the relevant dependency appears
transitively through the object library target itself.

Re-order the logic to process link dependencies first, and then external
object sources.  That way object libraries used via
`target_link_libraries` will be treated as such by dependency analysis.

This also adds missing backtrace information for object libraries used
via `target_link_libraries`.  The missing information was mentioned in a
FIXME comment in the RunCMake.FileAPI test added by commit ea0a060168
(fileapi: Add test for codemodel v2, 2018-11-09, v3.14.0-rc1~257^2~7).
That comment itself was dropped by commit a0de350e2f (FileAPI test:
Break gen_check_targets() into JSON files, 2020-02-07), but we can now
update the corresponding location in the `.json` files to have the
now-expected backtrace information.

Fixes: #20421
Brad King 5 years ago
parent
commit
a833aa1167

+ 14 - 13
Source/cmComputeTargetDepends.cxx

@@ -198,6 +198,20 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
     std::vector<std::string> const& configs =
       depender->Makefile->GetGeneratorConfigs();
     for (std::string const& it : configs) {
+      cmLinkImplementation const* impl = depender->GetLinkImplementation(it);
+
+      // A target should not depend on itself.
+      emitted.insert(cmLinkItem(depender, false, cmListFileBacktrace()));
+      emitted.insert(cmLinkItem(depender, true, cmListFileBacktrace()));
+      for (cmLinkImplItem const& lib : impl->Libraries) {
+        // Don't emit the same library twice for this target.
+        if (emitted.insert(lib).second) {
+          this->AddTargetDepend(depender_index, lib, true, false);
+          this->AddInterfaceDepends(depender_index, lib, it, emitted);
+        }
+      }
+
+      // Add dependencies on object libraries not otherwise handled above.
       std::vector<cmSourceFile const*> objectFiles;
       depender->GetExternalObjects(objectFiles, it);
       for (cmSourceFile const* o : objectFiles) {
@@ -222,19 +236,6 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
           }
         }
       }
-
-      cmLinkImplementation const* impl = depender->GetLinkImplementation(it);
-
-      // A target should not depend on itself.
-      emitted.insert(cmLinkItem(depender, false, cmListFileBacktrace()));
-      emitted.insert(cmLinkItem(depender, true, cmListFileBacktrace()));
-      for (cmLinkImplItem const& lib : impl->Libraries) {
-        // Don't emit the same library twice for this target.
-        if (emitted.insert(lib).second) {
-          this->AddTargetDepend(depender_index, lib, true, false);
-          this->AddInterfaceDepends(depender_index, lib, it, emitted);
-        }
-      }
     }
   }
 

+ 5 - 0
Tests/ObjectLibrary/Transitive/CMakeLists.txt

@@ -5,3 +5,8 @@ add_library(FooObject1 OBJECT FooObject.c)
 target_link_libraries(FooObject1 PRIVATE FooStatic)
 add_executable(Transitive1 Transitive.c)
 target_link_libraries(Transitive1 PRIVATE FooObject1)
+
+add_library(FooObject2 OBJECT FooObject.c)
+target_link_libraries(FooObject2 INTERFACE FooStatic)
+add_executable(Transitive2 Transitive.c)
+target_link_libraries(Transitive2 PRIVATE FooObject2)

+ 14 - 1
Tests/RunCMake/FileAPI/codemodel-v2-data/targets/c_object_exe.json

@@ -131,7 +131,20 @@
     "dependencies": [
         {
             "id": "^c_object_lib::@5ed5358f70faf8d8af7a$",
-            "backtrace": null
+            "backtrace": [
+                {
+                    "file": "^object/CMakeLists\\.txt$",
+                    "line": 7,
+                    "command": "target_link_libraries",
+                    "hasParent": true
+                },
+                {
+                    "file": "^object/CMakeLists\\.txt$",
+                    "line": null,
+                    "command": null,
+                    "hasParent": false
+                }
+            ]
         },
         {
             "id": "^ZERO_CHECK::@5ed5358f70faf8d8af7a$",

+ 14 - 1
Tests/RunCMake/FileAPI/codemodel-v2-data/targets/cxx_object_exe.json

@@ -131,7 +131,20 @@
     "dependencies": [
         {
             "id": "^cxx_object_lib::@5ed5358f70faf8d8af7a$",
-            "backtrace": null
+            "backtrace": [
+                {
+                    "file": "^object/CMakeLists\\.txt$",
+                    "line": 11,
+                    "command": "target_link_libraries",
+                    "hasParent": true
+                },
+                {
+                    "file": "^object/CMakeLists\\.txt$",
+                    "line": null,
+                    "command": null,
+                    "hasParent": false
+                }
+            ]
         },
         {
             "id": "^ZERO_CHECK::@5ed5358f70faf8d8af7a$",