jonkeane commented on code in PR #46390:
URL: https://github.com/apache/arrow/pull/46390#discussion_r2084550015
##########
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:
Yup! And ah ok, thanks, lemme try that. I also updated the comment, but I
might not be describing that in the best way, so very open to suggestions.
--
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]
