kou commented on code in PR #46390:
URL: https://github.com/apache/arrow/pull/46390#discussion_r2083826234
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -2652,32 +2652,53 @@ if(ARROW_WITH_ZLIB)
endif()
macro(build_lz4)
- message(STATUS "Building LZ4 from source")
+ message(STATUS "Building LZ4 from source using FetchContent")
- set(LZ4_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/lz4_ep-install")
+ # Set LZ4 as vendored
+ set(LZ4_VENDORED TRUE)
- set(LZ4_STATIC_LIB
-
"${LZ4_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}lz4${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ # Declare the content
+ fetchcontent_declare(lz4_ep
+ URL ${LZ4_SOURCE_URL}
+ URL_HASH "SHA256=${ARROW_LZ4_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR "build/cmake")
- set(LZ4_CMAKE_ARGS ${EP_COMMON_CMAKE_ARGS}
-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
- -DLZ4_BUILD_CLI=OFF -DLZ4_BUILD_LEGACY_LZ4C=OFF)
+ # Prepare fetch content environment
+ prepare_fetchcontent()
- # We need to copy the header in lib to directory outside of the build
- externalproject_add(lz4_ep
- ${EP_COMMON_OPTIONS}
- CMAKE_ARGS ${LZ4_CMAKE_ARGS}
- SOURCE_SUBDIR "build/cmake"
- INSTALL_DIR ${LZ4_PREFIX}
- URL ${LZ4_SOURCE_URL}
- URL_HASH "SHA256=${ARROW_LZ4_BUILD_SHA256_CHECKSUM}"
- BUILD_BYPRODUCTS ${LZ4_STATIC_LIB})
+ # Set LZ4-specific build options as cache variables
+ set(LZ4_BUILD_CLI
+ OFF
+ CACHE BOOL "Don't build LZ4 CLI" FORCE)
+ set(LZ4_BUILD_LEGACY_LZ4C
+ OFF
+ CACHE BOOL "Don't build legacy LZ4 tools" FORCE)
+
+ # Make the dependency available - this will actually perform the download
and configure
+ fetchcontent_makeavailable(lz4_ep)
- file(MAKE_DIRECTORY "${LZ4_PREFIX}/include")
- add_library(LZ4::lz4 STATIC IMPORTED)
- set_target_properties(LZ4::lz4 PROPERTIES IMPORTED_LOCATION
"${LZ4_STATIC_LIB}")
- target_include_directories(LZ4::lz4 BEFORE INTERFACE "${LZ4_PREFIX}/include")
- add_dependencies(LZ4::lz4 lz4_ep)
+ # Get the source and binary directories after fetching content
+ fetchcontent_getproperties(lz4_ep
+ SOURCE_DIR LZ4_SOURCE_DIR
+ BINARY_DIR LZ4_BINARY_DIR)
+
+ # Set up the imported library for compatibility with the rest of the build
system
+ set(LZ4_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/_deps/lz4_ep-build")
+ set(LZ4_INCLUDE_DIR "${LZ4_SOURCE_DIR}/lib")
+
+ # Since lz4 is now part of our build system, we can directly use the target
+ add_library(LZ4::lz4 ALIAS lz4_static)
+
+ # Add lz4_static to the orc export set
+ if(NOT TARGET lz4_static)
+ message(FATAL_ERROR "Expected lz4_static target to be created by
FetchContent")
+ endif()
+ if(NOT DEFINED CMAKE_EXPORT_NO_PACKAGE_REGISTRY)
+ set(CMAKE_EXPORT_NO_PACKAGE_REGISTRY ON)
+ endif()
+ install(TARGETS lz4_static EXPORT orc_targets)
Review Comment:
How about this?
```diff
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake
b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 6cfcdc84c0..5541721ec5 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2678,10 +2678,11 @@ macro(build_lz4)
fetchcontent_makeavailable(lz4_ep)
# Since lz4 is now part of our build system, we can directly use the
target
- add_library(LZ4::lz4 ALIAS lz4_static)
+ add_library(LZ4::lz4 INTERFACE IMPORTED)
+ target_link_libraries(LZ4::lz4 INTERFACE lz4_static)
# Add to bundled static libs
- list(APPEND ARROW_BUNDLED_STATIC_LIBS LZ4::lz4)
+ list(APPEND ARROW_BUNDLED_STATIC_LIBS lz4_static)
endmacro()
if(ARROW_WITH_LZ4)
```
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
