[GitHub] [arrow] kou commented on a change in pull request #11889: ARROW-14708: [C++] Adding missing abseil dependencies to enable static flight build

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

kou commented on a change in pull request #11889:
URL: https://github.com/apache/arrow/pull/11889#discussion_r765229149



##########
File path: cpp/src/arrow/ArrowConfig.cmake.in
##########
@@ -87,6 +87,10 @@ if(NOT (TARGET arrow_shared OR TARGET arrow_static))
       set_target_properties(
         arrow_static PROPERTIES INTERFACE_LINK_LIBRARIES
         "${arrow_static_interface_link_libraries};arrow_bundled_dependencies")
+        # Propagate dependencies like OpenSSL
+        set_target_properties(
+          arrow_bundled_dependencies PROPERTIES INTERFACE_LINK_LIBRARIES
+          "${arrow_static_interface_link_libraries}")

Review comment:
       I don't know why this is needed.
   Is there a use-case that uses `arrow_bundled_dependencies` without `arrow_static`? 
   
   BTW, indent for this code is broken.

##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -3476,13 +3476,28 @@ macro(build_grpc)
                                    INTERFACE_INCLUDE_DIRECTORIES "${GRPC_INCLUDE_DIR}")
 
   set(GRPC_GPR_ABSL_LIBRARIES
+      absl::bad_optional_access
       absl::base
-      absl::statusor
-      absl::status
       absl::cord
+      absl::debugging_internal
+      absl::demangle_internal
+      absl::graphcycles_internal
+      absl::int128
+      absl::log_severity
+      absl::malloc_internal
+      absl::raw_logging_internal
+      absl::spinlock_wait
+      absl::status
+      absl::statusor
+      absl::stacktrace
       absl::strings
+      absl::strings_internal
+      absl::str_format_internal
+      absl::symbolize
       absl::synchronization
-      absl::time)
+      absl::throw_delegate
+      absl::time
+      absl::time_zone)

Review comment:
       Can we create a one-linear(?) to collect this list like https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2659-L2667 ?
   
   It may be better that we have a shell script that generates the list and include it from this file:
   
   ```bash
   # cpp/build-support/list-grpc-absl-libraries.sh
   ...
   echo "set(GRPC_GPR_ABSL_LIBRARIES absl::...)"
   ```
   
   ```console
   $ cpp/build-support/list-grpc-absl-libraries.sh > cpp/cmake_modules/gRPCVariables.cmake
   ```
   
   ```diff
   diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   index 66d04acae..36565fdbf 100644
   --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
   +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   @@ -3475,14 +3475,7 @@ macro(build_grpc)
                            PROPERTIES IMPORTED_LOCATION "${GRPC_STATIC_LIBRARY_UPB}"
                                       INTERFACE_INCLUDE_DIRECTORIES "${GRPC_INCLUDE_DIR}")
    
   -  set(GRPC_GPR_ABSL_LIBRARIES
   -      absl::base
   -      absl::statusor
   -      absl::status
   -      absl::cord
   -      absl::strings
   -      absl::synchronization
   -      absl::time)
   +  include(gRPCVariables)
      add_library(gRPC::gpr STATIC IMPORTED)
      set_target_properties(gRPC::gpr
                            PROPERTIES IMPORTED_LOCATION "${GRPC_STATIC_LIBRARY_GPR}"
   ```




-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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