Deprecate `LLVM_LIBRARY_DIRS`

[Ericson2314]

Summary

There is a FIXME to add other directories to LLVM_LIBRARY_DIRS. If we don't do that, however, it is the same as LLVM_LIBRARY_DIR. This is sort of a "plan B" revision in case we don't do that FIXME.

I still defined LLVM_LIBRARY_DIRS for sake of downstream projects, but
added a comment saying it was deprecated and should be removed. Is that
correct?

[mstorsjo]

This looks reasonable to me, but I'm not sure if I know all the aspects that might be at play here, so I don't think I'm comfortable with accepting it on my own. Adding @beanz who should know lots about our cmake setup too.

llvm/cmake/modules/CMakeLists.txt

65

Curious: Why the reordering? I presume you're moving LLVM_CONFIG_CMAKE_DIR into alphabetical order, but then LLVM_CONFIG_LIBRARY_DIR seems out of place?

[Ericson2314]

llvm/cmake/modules/CMakeLists.txt

65

Oh I just moved the prefix one to the top, since it is unlike the others, and put library on the bottom, moving it away from the other "weird" one (the include dirs). The ordering of cmake then binaries is simply because that is the way it was before --- the diff algorithm just happened to hide that in choosing to make LLVM_CONFIG_BINARY_DIR the "stable" one line.

[beanz]

LLVM_LIBRARY_DIR and LLVM_LIBRARY_DIRS aren’t intended to serve the same purpose. The S in DIRS is intentional to convey that it is a list of directories, not necessarily a single value, and it would definitely break many uses of LLVM_LIBRARY_DIR if it was a list. There are several (not correctly handled) build configurations where LLVM_SHLIB_OUTPUT_INTDIR and the shared library install directory are not the same as the static archive install directory. In those cases we _should_ be setting LLVM_LIBRARY_DIRS to be an array containing both LLVM_LIBRARY_DIR and LLVM_TOOLS_BINARY_DIR.

[Ericson2314]

llvm/cmake/modules/LLVMConfig.cmake.in

121–124

@beanz

it would definitely break many uses of LLVM_LIBRARY_DIR if it was a list.

The code right here is dangerously close to doing that. So I think some change is waranted.

Perhaps I can add back set(LLVM_LIBRARY_DIRS "@LLVM_CONFIG_LIBRARY_DIRS@") above and add a TODO next to the definitions of LLVM_CONFIG_LIBRARY_DIRS with the cases you mentioned?

[Ericson2314]

Rebase on top of D116497

I will probably just close this, but doing the rebase in case we should get rid
of LLVM_LIBRARY_DIRS after all.

[Ericson2314]

Rebase

[Ericson2314]

rebase

[Ericson2314]

Rebase

lld/CMakeLists.txt

if(LLD_BUILT_STANDALONE)
   include(TableGen)
   include(HandleLLVMOptions)
   include(GetErrcMessages)
   include(CheckAtomic)
 
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 
   include_directories(${LLVM_INCLUDE_DIRS})
-  link_directories(${LLVM_LIBRARY_DIRS})
+  link_directories("${LLVM_LIBRARY_DIR}")
 
   if(LLVM_INCLUDE_TESTS)
     find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} REQUIRED
       COMPONENTS Interpreter)
 
     # Check prebuilt llvm/utils.
     if(EXISTS ${LLVM_TOOLS_BINARY_DIR}/FileCheck${CMAKE_EXECUTABLE_SUFFIX}
         AND EXISTS ${LLVM_TOOLS_BINARY_DIR}/not${CMAKE_EXECUTABLE_SUFFIX})

lldb/cmake/modules/LLDBConfig.cmake

 
 # If LLDB is building against a prebuilt Clang, then the Clang resource
 # directory that LLDB is using for its embedded Clang instance needs to point
 # to the resource directory of the used Clang installation.
 if (NOT TARGET clang-resource-headers)
   set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
   # Iterate over the possible places where the external resource directory
   # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
+  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIR}"
                     "${LLVM_BUILD_LIBRARY_DIR}"
                     "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
     # Build the resource directory path by appending 'clang/<version number>'.
     set(CANDIDATE_RESOURCE_DIR "${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
     if (IS_DIRECTORY "${CANDIDATE_RESOURCE_DIR}")
       set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${CANDIDATE_RESOURCE_DIR}")
       break()
     endif()

llvm/cmake/modules/CMakeLists.txt

 set(LLVM_CONFIG_MAIN_INCLUDE_DIR "${LLVM_MAIN_INCLUDE_DIR}")
 set(LLVM_CONFIG_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
 set(LLVM_CONFIG_INCLUDE_DIRS
   "${LLVM_CONFIG_MAIN_INCLUDE_DIR}"
   "${LLVM_CONFIG_INCLUDE_DIR}"
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_CONFIG_CMAKE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
 set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
+set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
 
 # Generate a default location for lit
 if (LLVM_BUILD_UTILS)
   if (CMAKE_HOST_WIN32 AND NOT CYGWIN)
     set(LLVM_CONFIG_DEFAULT_EXTERNAL_LIT "${LLVM_CONFIG_TOOLS_BINARY_DIR}/llvm-lit.py")
   else()
     set(LLVM_CONFIG_DEFAULT_EXTERNAL_LIT "${LLVM_CONFIG_TOOLS_BINARY_DIR}/llvm-lit")
   endif()

 # are combined in one directory at install time.
 set(LLVM_CONFIG_INCLUDE_DIR "${LLVM_CONFIG_MAIN_INCLUDE_DIR}")
 set(LLVM_CONFIG_INCLUDE_DIRS
   "${LLVM_CONFIG_MAIN_INCLUDE_DIR}"
   "${LLVM_CONFIG_INCLUDE_DIR}"
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "\${LLVM_INSTALL_PREFIX}")
 extend_path(LLVM_CONFIG_CMAKE_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
 extend_path(LLVM_CONFIG_TOOLS_BINARY_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_TOOLS_INSTALL_DIR}")
+extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
 
 # Generate a default location for lit
 if (LLVM_INSTALL_UTILS AND LLVM_BUILD_UTILS)
   if (CMAKE_HOST_WIN32 AND NOT CYGWIN)
     set(LLVM_CONFIG_DEFAULT_EXTERNAL_LIT "${LLVM_CONFIG_TOOLS_BINARY_DIR}/llvm-lit.py")
   else()
     set(LLVM_CONFIG_DEFAULT_EXTERNAL_LIT "${LLVM_CONFIG_TOOLS_BINARY_DIR}/llvm-lit")
   endif()

llvm/cmake/modules/LLVMConfig.cmake.in

 set(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS @LLVM_EXPORT_SYMBOLS_FOR_PLUGINS@)
 set(LLVM_PLUGIN_EXT @LLVM_PLUGIN_EXT@)
 
 set(LLVM_ON_UNIX @LLVM_ON_UNIX@)
 
 set(LLVM_LIBDIR_SUFFIX @LLVM_LIBDIR_SUFFIX@)
 
 set(LLVM_INCLUDE_DIRS "@LLVM_CONFIG_INCLUDE_DIRS@")
-set(LLVM_LIBRARY_DIRS "@LLVM_CONFIG_LIBRARY_DIRS@")
 
 set(LLVM_APPEND_VC_REV "@LLVM_APPEND_VC_REV@")
 
 # These variables are duplicated for install tree but they have different
 # values for build tree.  LLVM_INCLUDE_DIRS contains both source
 # and generated include directories while the following variables have
 # them split.
 
-# These are the "main" dirs
+# This is the "main" dir
 set(LLVM_MAIN_INCLUDE_DIR "@LLVM_CONFIG_MAIN_INCLUDE_DIR@")
-set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 
 # This is a secondary one for generated files
 set(LLVM_INCLUDE_DIR "@LLVM_CONFIG_INCLUDE_DIR@")
 
 set(LLVM_DEFINITIONS "@LLVM_DEFINITIONS@")
 set(LLVM_BINARY_DIR "@LLVM_CONFIG_BINARY_DIR@")
 set(LLVM_CMAKE_DIR "@LLVM_CONFIG_CMAKE_DIR@")
 set(LLVM_TOOLS_BINARY_DIR "@LLVM_CONFIG_TOOLS_BINARY_DIR@")
+set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 set(LLVM_HAVE_OPT_VIEWER_MODULES @LLVM_HAVE_OPT_VIEWER_MODULES@)
 set(LLVM_CONFIGURATION_TYPES @CMAKE_CONFIGURATION_TYPES@)
 set(LLVM_ENABLE_SHARED_LIBS @BUILD_SHARED_LIBS@)
 
+# This is deprecated and just here for backwards compatibility. It will be
+# removed in the future.
+set(LLVM_LIBRARY_DIRS "${LLVM_LIBRARY_DIR}")
+
 set(LLVM_DEFAULT_EXTERNAL_LIT "@LLVM_CONFIG_DEFAULT_EXTERNAL_LIT@")
 set(LLVM_LIT_ARGS "@LLVM_LIT_ARGS@")
 
 set(LLVM_HAVE_LIBXAR "@LLVM_HAVE_LIBXAR@")
 
 if(NOT TARGET LLVMSupport)
   @LLVM_CONFIG_INCLUDE_EXPORTS@
   @llvm_config_include_buildtree_only_exports@