[PATCH] D70026: [cmake] Always build the libLLVM shared library

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/CMakeLists.txt:655-656
 
 if(WIN32 OR CYGWIN)
-  if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
+  if(BUILD_SHARED_LIBS)
     set(LLVM_ENABLE_PLUGINS_default ON)
----------------
tstellar wrote:
> This change I'm not so sure about.  Can we drop the WIN32 part of the condition?   Does CYGWIN support building shared libs?
Why the desire to drop `WIN32`?  Both Windows and cygwin support shared libraries.


================
Comment at: llvm/tools/llvm-config/CMakeLists.txt:56
 string(REPLACE ";" " " LLVM_TARGETS_BUILT "${LLVM_TARGETS_TO_BUILD}")
+set(LLVM_ENABLE_DYLIB NOT MSVC)
 llvm_canonicalize_cmake_booleans(
----------------
I think that this should really be `NOT CMAKE_SYSTEM_NAME STREQUAL Windows`.  I can build for windows using clang (not clang-cl) in which case, we get the value wrong.


================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:12
 
-if(LLVM_BUILD_LLVM_DYLIB)
-  if(MSVC)
-    message(FATAL_ERROR "Generating libLLVM is not supported on MSVC")
-  endif()
-
+if(NOT MSVC)
   llvm_map_components_to_libnames(LIB_NAMES ${LLVM_DYLIB_COMPONENTS})
----------------
Similar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70026