[GitHub] [arrow] tobim commented on a change in pull request #7842: ARROW-9412: [C++] Add non-bundled dependencies to INTERFACE_LINK_LIBRARIES of static libarrow

[GitBox <gi...@apache.org>]

tobim commented on a change in pull request #7842:
URL: https://github.com/apache/arrow/pull/7842#discussion_r462285704



##########
File path: cpp/cmake_modules/FindLz4.cmake
##########
@@ -24,13 +24,14 @@ set(LZ4_STATIC_LIB_SUFFIX "${LZ4_MSVC_STATIC_LIB_SUFFIX}${CMAKE_STATIC_LIBRARY_S
 set(LZ4_LIB_NAMES lz4 liblz4)
 
 if(NOT ARROW_LZ4_USE_SHARED)
+  set(static_names_)

Review comment:
       This is not necessary.

##########
File path: cpp/src/arrow/ArrowConfig.cmake.in
##########
@@ -38,45 +38,42 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowOptions.cmake")
 
 include(CMakeFindDependencyMacro)
 
-if(ARROW_USE_STATIC)
-  set(LIB_PATH_SUFFIXES
-      "${CMAKE_LIBRARY_ARCHITECTURE}"
-      "lib/${CMAKE_LIBRARY_ARCHITECTURE}"
-      "lib64"
-      "lib32"
-      "lib"
-      "bin"
-      "Library"
-      "Library/lib"
-      "Library/bin")
-  set(INCLUDE_PATH_SUFFIXES "include" "Library" "Library/include")
-
-  set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
-  set(THREADS_PREFER_PTHREAD_FLAG TRUE)
-  find_dependency(Threads)
-
-  if(DEFINED CMAKE_MODULE_PATH)
-    set(_CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
-  endif()
-  set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
-
-@ARROW_FIND_DEPENDENCY_LIST@
-
-  if(DEFINED _CMAKE_MODULE_PATH_OLD)
-    set(CMAKE_MODULE_PATH ${_CMAKE_MODULE_PATH_OLD})
-    unset(_CMAKE_MODULE_PATH_OLD)
-  else()
-    unset(CMAKE_MODULE_PATH)
-  endif()
-endif()
-
-
 # Load targets only once. If we load targets multiple times, CMake reports
 # already existent target error.
 if(NOT (TARGET arrow_shared OR TARGET arrow_static))
   include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
 
   if(TARGET arrow_static)
+    set(LIB_PATH_SUFFIXES

Review comment:
       This change means that the static dependencies need to be resolved even if `arrow_static` is never used.

##########
File path: cpp/cmake_modules/Findzstd.cmake
##########
@@ -54,10 +54,11 @@ else()
   if(ZSTD_PC_FOUND)
     set(ZSTD_INCLUDE_DIR "${ZSTD_PC_INCLUDEDIR}")
 
-    if(NOT ARROW_ZSTD_USE_SHARED)
+    if(ARROW_ZSTD_USE_SHARED)
+      list(APPEND ZSTD_PC_LIBRARY_DIRS "${ZSTD_PC_LIBDIR}")
+    else()

Review comment:
       Some libzstd.pc versions are written in a way that only ${ZSTD_PC_STATIC_LIBDIR} contains a path. I started out with essentially this, but it didn't work during testing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org