[PATCH] D108098: [cmake] properly support target_link_directories and target_include_directories

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula accepted this revision.
bondhugula added a comment.

Thanks for fixing this!



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:489
 
+    # Bring in the target include directories from our original target
+    target_include_directories(${obj_name} PRIVATE $<TARGET_PROPERTY:${name},INCLUDE_DIRECTORIES>)
----------------
Nit: terminate with a full stop - here and below.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:533
+    # Bring in the target link directories from our original target
+    target_link_directories(${name_static} PRIVATE $<TARGET_PROPERTY:${name},LINK_DIRECTORIES>)
+
----------------
Is there something to fix for `target_link_libraries`?
`target_link_directories` is to be rarely used per https://cmake.org/cmake/help/latest/command/target_link_libraries.html#command:target_link_libraries
Instead it's `target_link_libraries` is preferred.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108098/new/

https://reviews.llvm.org/D108098