[PATCH] D79059: Fix and re-submit D78534 - [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

[Han Zhu via Phabricator via cfe-commits]

zhuhan0 created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
zhuhan0 edited the summary of this revision.

D78354 <https://reviews.llvm.org/D78354> enabled installing PIC version of both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC is ON. But it broke the no-PIC build when LLVM_ENABLE_PIC=OFF with the following error:

  CMake Error at                                                             
  /b/s/w/ir/cache/builder/src/third_party/llvm/clang/tools/libclang/CMakeLists.txt:123
  (target_compile_definitions):                                              
      target_compile_definitions called with non-compilable target type

This is because as the code loops through ${name} and ${name}_static, it introduced a side effect, which is adding an empty libclang_static to targets. Later target_compile_definitions is called on libclang_static. That function requires that target must have been created by a command such as add_executable() or add_library(), so it crashed.

The solution is to not naively loop through both libclang and libclang_static, but only the ones that are actually added by llvm_add_library(). Here's the library build type to library target name mapping:

| SHARED only       | libclang                     |
| STATIC only       | libclang                     |
| SHARED and STATIC | libclang and libclang_static |
|




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79059

Files:
  clang/cmake/modules/AddClang.cmake


Index: clang/cmake/modules/AddClang.cmake
===================================================================
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -45,7 +45,7 @@
 
 macro(add_clang_library name)
   cmake_parse_arguments(ARG
-    "SHARED;INSTALL_WITH_TOOLCHAIN"
+    "SHARED;STATIC;INSTALL_WITH_TOOLCHAIN"
     ""
     "ADDITIONAL_HEADERS"
     ${ARGN})
@@ -81,7 +81,10 @@
       ${ARG_ADDITIONAL_HEADERS} # It may contain unparsed unknown args.
       )
   endif()
-  if(ARG_SHARED)
+
+  if(ARG_SHARED AND ARG_STATIC)
+    set(LIBTYPE SHARED STATIC)
+  elseif(ARG_SHARED)
     set(LIBTYPE SHARED)
   else()
     # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
@@ -99,38 +102,45 @@
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
-  if(TARGET ${name})
-    target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
+  set(libs ${name})
+  if(ARG_SHARED AND ARG_STATIC)
+    set(libs ${name} ${name}_static)
+  endif()
 
-    if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
-      set(export_to_clangtargets)
-      if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-          "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-          NOT LLVM_DISTRIBUTION_COMPONENTS)
-        set(export_to_clangtargets EXPORT ClangTargets)
-        set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
-      endif()
+  foreach(lib ${libs})
+    if(TARGET ${lib})
+      target_link_libraries(${lib} INTERFACE ${LLVM_COMMON_LIBS})
 
-      install(TARGETS ${name}
-        COMPONENT ${name}
-        ${export_to_clangtargets}
-        LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-        ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-        RUNTIME DESTINATION bin)
+      if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
+        set(export_to_clangtargets)
+        if(${lib} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+            "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+            NOT LLVM_DISTRIBUTION_COMPONENTS)
+          set(export_to_clangtargets EXPORT ClangTargets)
+          set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
+        endif()
 
-      if (NOT LLVM_ENABLE_IDE)
-        add_llvm_install_targets(install-${name}
-                                 DEPENDS ${name}
-                                 COMPONENT ${name})
-      endif()
+        install(TARGETS ${lib}
+          COMPONENT ${lib}
+          ${export_to_clangtargets}
+          LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+          ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+          RUNTIME DESTINATION bin)
 
-      set_property(GLOBAL APPEND PROPERTY CLANG_LIBS ${name})
-    endif()
-    set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${name})
-  else()
-    # Add empty "phony" target
-    add_custom_target(${name})
+        if (NOT LLVM_ENABLE_IDE)
+          add_llvm_install_targets(install-${lib}
+                                  DEPENDS ${lib}
+                                  COMPONENT ${lib})
+        endif()
+
+        set_property(GLOBAL APPEND PROPERTY CLANG_LIBS ${lib})
+      endif()
+      set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${lib})
+    else()
+      # Add empty "phony" target
+      add_custom_target(${lib})
   endif()
+endforeach()
 
   set_target_properties(${name} PROPERTIES FOLDER "Clang libraries")
   set_clang_windows_version_resource_properties(${name})


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79059.260815.patch
Type: text/x-patch
Size: 3471 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200429/aec4b3fc/attachment-0001.bin>