jonkeane commented on code in PR #46390:
URL: https://github.com/apache/arrow/pull/46390#discussion_r2083775871
##########
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")
+
Review Comment:
Yup, they are cruft I should have removed. Done now
--
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]
