raulcd commented on code in PR #50195:
URL: https://github.com/apache/arrow/pull/50195#discussion_r4024805277


##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -1042,21 +1027,61 @@ if(ARROW_FILESYSTEM)
     endforeach()
   endif()
   if(ARROW_S3)
-    foreach(ARROW_FILESYSTEM_TARGET ${ARROW_FILESYSTEM_TARGETS})
-      target_link_libraries(${ARROW_FILESYSTEM_TARGET} PRIVATE 
${AWSSDK_LINK_LIBRARIES})
-    endforeach()
-
-    if(ARROW_S3_MODULE)
-      if(NOT ARROW_BUILD_SHARED)
-        message(FATAL_ERROR "ARROW_S3_MODULE without shared libarrow 
(-DARROW_BUILD_SHARED=ON) is not supported"
-        )
+    # If libarrow_s3.a is only built, "pkg-config --cflags --libs
+    # arrow-s3" outputs build flags for static linking not shared
+    # linking. ARROW_S3_PC_* except ARROW_S3_PC_*_PRIVATE are for the
+    # static linking case.
+    if(NOT ARROW_BUILD_SHARED AND ARROW_BUILD_STATIC)
+      string(APPEND ARROW_S3_PC_CFLAGS "${ARROW_S3_PC_CFLAGS_PRIVATE}")
+      set(ARROW_S3_PC_CFLAGS_PRIVATE "")
+      set(ARROW_S3_PC_LIBS "${ARROW_S3_PC_LIBS_PRIVATE}")
+      set(ARROW_S3_PC_LIBS_PRIVATE "")
+      set(ARROW_S3_PC_REQUIRES "${ARROW_S3_PC_REQUIRES_PRIVATE}")
+      set(ARROW_S3_PC_REQUIRES_PRIVATE "")
+    else()
+      set(ARROW_S3_PC_LIBS "")
+      set(ARROW_S3_PC_REQUIRES "")
+    endif()
+    list(APPEND ARROW_S3_LIB_SRCS filesystem/s3fs_module.cc filesystem/s3fs.cc)
+    set(ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS Arrow::arrow_static)
+    if(AWSSDK_SOURCE STREQUAL "SYSTEM")
+      list(APPEND ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS 
${AWSSDK_LINK_LIBRARIES})
+    elseif(AWSSDK_SOURCE STREQUAL "BUNDLED")
+      if(UNIX)
+        list(APPEND ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS CURL::libcurl)
       endif()
-
-      add_library(arrow_s3fs MODULE filesystem/s3fs_module.cc 
filesystem/s3fs.cc)
-      target_link_libraries(arrow_s3fs PRIVATE ${AWSSDK_LINK_LIBRARIES} 
arrow_shared)
-      set_source_files_properties(filesystem/s3fs.cc filesystem/s3fs_module.cc
-                                  PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
     endif()
+    add_arrow_lib(arrow_s3
+                  CMAKE_PACKAGE_NAME
+                  ArrowS3
+                  PKG_CONFIG_NAME
+                  arrow-s3
+                  SOURCES
+                  ${ARROW_S3_LIB_SRCS}

Review Comment:
   I think this premise is wrong `unlike the previous in-libarrow layout.` as 
there are no S3 symbols there either on `filesystem.cc` so this mechanism 
hasn't changed and if there's an issue to be fixed it was pre-existing.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to