[PATCH] D90848: llvmbuildectomy - part 2

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Thanks for your effort. I love it. Seems to remove complexities of the buildsystem and the ugly split of having to define library dependencies in two files.

If needed, I can check whether anything break on Windows.



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:620
+  else()
+    # We can use PRIVATE since SO knows its dependent libs.
+    set(libtype PRIVATE)
----------------
SO = significant other?


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:631-634
+        llvm_map_components_to_libnames(llvm_libs
+            ${ARG_LINK_COMPONENTS}
+            ${LLVM_LINK_COMPONENTS}
+        )
----------------
[nit] unrelated whitespace change


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:643-645
+    set_property(TARGET ${name} PROPERTY LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS})
+    set_property(TARGET ${name} PROPERTY LLVM_LINK_LIBS ${ARG_LINK_LIBS})
+    set_property(TARGET ${name} PROPERTY LLVM_LIBTYPE ${libtype})
----------------
Is there somewhere an overview/description of these target properties?


================
Comment at: llvm/lib/ExecutionEngine/MCJIT/CMakeLists.txt:7
+
+  LINK_COMPONENTS Core ExecutionEngine Object RuntimeDyld Support Target
   )
----------------
Could you put the dependees on separate line, like `DEPENDS` options above?


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

https://reviews.llvm.org/D90848