Skip to content

Commit f4d52af

Browse files
jonkeanekou
andauthored
GH-46487: [C++] Refactor lz4 from ExternalProject to FetchContent (#46390)
### Rationale for this change `FetchContent` is easier to maintain than `ExternalProject` because we don't need to create import CMake targets manually with `FetchContent`. ### What changes are included in this PR? Use `FetchContent` instead of `ExternalProject` for bundled LZ4. ### Are these changes tested? Yes ### Are there any user-facing changes? No * GitHub Issue: #46487 Lead-authored-by: Jonathan Keane <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
1 parent da74ae6 commit f4d52af

File tree

1 file changed

+49
-36
lines changed

1 file changed

+49
-36
lines changed

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,33 +2650,40 @@ if(ARROW_WITH_ZLIB)
26502650
endif()
26512651

26522652
macro(build_lz4)
2653-
message(STATUS "Building LZ4 from source")
2653+
message(STATUS "Building LZ4 from source using FetchContent")
26542654

2655-
set(LZ4_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/lz4_ep-install")
2655+
# Set LZ4 as vendored
2656+
set(LZ4_VENDORED TRUE)
26562657

2657-
set(LZ4_STATIC_LIB
2658-
"${LZ4_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}lz4${CMAKE_STATIC_LIBRARY_SUFFIX}")
2658+
# Declare the content
2659+
fetchcontent_declare(lz4
2660+
URL ${LZ4_SOURCE_URL}
2661+
URL_HASH "SHA256=${ARROW_LZ4_BUILD_SHA256_CHECKSUM}"
2662+
SOURCE_SUBDIR "build/cmake")
26592663

2660-
set(LZ4_CMAKE_ARGS ${EP_COMMON_CMAKE_ARGS} -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
2661-
-DLZ4_BUILD_CLI=OFF -DLZ4_BUILD_LEGACY_LZ4C=OFF)
2664+
# Prepare fetch content environment
2665+
prepare_fetchcontent()
26622666

2663-
# We need to copy the header in lib to directory outside of the build
2664-
externalproject_add(lz4_ep
2665-
${EP_COMMON_OPTIONS}
2666-
CMAKE_ARGS ${LZ4_CMAKE_ARGS}
2667-
SOURCE_SUBDIR "build/cmake"
2668-
INSTALL_DIR ${LZ4_PREFIX}
2669-
URL ${LZ4_SOURCE_URL}
2670-
URL_HASH "SHA256=${ARROW_LZ4_BUILD_SHA256_CHECKSUM}"
2671-
BUILD_BYPRODUCTS ${LZ4_STATIC_LIB})
2672-
2673-
file(MAKE_DIRECTORY "${LZ4_PREFIX}/include")
2674-
add_library(LZ4::lz4 STATIC IMPORTED)
2675-
set_target_properties(LZ4::lz4 PROPERTIES IMPORTED_LOCATION "${LZ4_STATIC_LIB}")
2676-
target_include_directories(LZ4::lz4 BEFORE INTERFACE "${LZ4_PREFIX}/include")
2677-
add_dependencies(LZ4::lz4 lz4_ep)
2678-
2679-
list(APPEND ARROW_BUNDLED_STATIC_LIBS LZ4::lz4)
2667+
# Set LZ4-specific build options as cache variables
2668+
set(LZ4_BUILD_CLI
2669+
OFF
2670+
CACHE BOOL "Don't build LZ4 CLI" FORCE)
2671+
set(LZ4_BUILD_LEGACY_LZ4C
2672+
OFF
2673+
CACHE BOOL "Don't build legacy LZ4 tools" FORCE)
2674+
2675+
# Make the dependency available - this will actually perform the download and configure
2676+
fetchcontent_makeavailable(lz4)
2677+
2678+
# Use LZ4::lz4 as an imported library not an alias of lz4_static so other targets such as orc
2679+
# can depend on it as an external library. External libraries are ignored in
2680+
# install(TARGETS orc EXPORT orc_targets) and install(EXPORT orc_targets).
2681+
add_library(LZ4::lz4 INTERFACE IMPORTED)
2682+
target_link_libraries(LZ4::lz4 INTERFACE lz4_static)
2683+
2684+
# Add to bundled static libs.
2685+
# We must use lz4_static (not imported target) not LZ4::lz4 (imported target).
2686+
list(APPEND ARROW_BUNDLED_STATIC_LIBS lz4_static)
26802687
endmacro()
26812688

26822689
if(ARROW_WITH_LZ4)
@@ -4587,6 +4594,17 @@ target_include_directories(arrow::hadoop INTERFACE "${HADOOP_HOME}/include")
45874594
function(build_orc)
45884595
message(STATUS "Building Apache ORC from source")
45894596

4597+
if(LZ4_VENDORED)
4598+
set(ORC_LZ4_TARGET lz4_static)
4599+
set(ORC_LZ4_ROOT "${lz4_SOURCE_DIR}")
4600+
set(ORC_LZ4_INCLUDE_DIR "${lz4_SOURCE_DIR}/lib")
4601+
else()
4602+
set(ORC_LZ4_TARGET LZ4::lz4)
4603+
get_target_property(ORC_LZ4_INCLUDE_DIR ${ORC_LZ4_TARGET}
4604+
INTERFACE_INCLUDE_DIRECTORIES)
4605+
get_filename_component(ORC_LZ4_ROOT "${ORC_LZ4_INCLUDE_DIR}" DIRECTORY)
4606+
endif()
4607+
45904608
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.29)
45914609
fetchcontent_declare(orc
45924610
${FC_DECLARE_COMMON_OPTIONS}
@@ -4599,16 +4617,14 @@ function(build_orc)
45994617
set(ORC_PREFER_STATIC_LZ4
46004618
OFF
46014619
CACHE BOOL "" FORCE)
4602-
get_target_property(LZ4_INCLUDE_DIR LZ4::lz4 INTERFACE_INCLUDE_DIRECTORIES)
4603-
if(NOT LZ4_INCLUDE_DIR)
4604-
find_path(LZ4_INCLUDE_DIR NAMES lz4.h)
4605-
endif()
4606-
get_filename_component(LZ4_ROOT "${LZ4_INCLUDE_DIR}" DIRECTORY)
46074620
set(LZ4_HOME
4608-
"${LZ4_ROOT}"
4621+
"${ORC_LZ4_ROOT}"
4622+
CACHE STRING "" FORCE)
4623+
set(LZ4_INCLUDE_DIR
4624+
"${ORC_LZ4_INCLUDE_DIR}"
46094625
CACHE STRING "" FORCE)
46104626
set(LZ4_LIBRARY
4611-
LZ4::lz4
4627+
${ORC_LZ4_TARGET}
46124628
CACHE STRING "" FORCE)
46134629

46144630
set(ORC_PREFER_STATIC_PROTOBUF
@@ -4707,9 +4723,6 @@ function(build_orc)
47074723
INTERFACE_INCLUDE_DIRECTORIES)
47084724
get_filename_component(ORC_SNAPPY_ROOT "${ORC_SNAPPY_INCLUDE_DIR}" DIRECTORY)
47094725

4710-
get_target_property(ORC_LZ4_ROOT LZ4::lz4 INTERFACE_INCLUDE_DIRECTORIES)
4711-
get_filename_component(ORC_LZ4_ROOT "${ORC_LZ4_ROOT}" DIRECTORY)
4712-
47134726
get_target_property(ORC_ZSTD_ROOT ${ARROW_ZSTD_LIBZSTD} INTERFACE_INCLUDE_DIRECTORIES)
47144727
get_filename_component(ORC_ZSTD_ROOT "${ORC_ZSTD_ROOT}" DIRECTORY)
47154728

@@ -4733,9 +4746,9 @@ function(build_orc)
47334746
"-DSNAPPY_HOME=${ORC_SNAPPY_ROOT}"
47344747
"-DSNAPPY_LIBRARY=$<TARGET_FILE:${Snappy_TARGET}>"
47354748
"-DLZ4_HOME=${ORC_LZ4_ROOT}"
4736-
"-DLZ4_LIBRARY=$<TARGET_FILE:LZ4::lz4>"
4737-
"-DLZ4_STATIC_LIB=$<TARGET_FILE:LZ4::lz4>"
4738-
"-DLZ4_INCLUDE_DIR=${ORC_LZ4_ROOT}/include"
4749+
"-DLZ4_LIBRARY=$<TARGET_FILE:${ORC_LZ4_TARGET}>"
4750+
"-DLZ4_STATIC_LIB=$<TARGET_FILE:${ORC_LZ4_TARGET}>"
4751+
"-DLZ4_INCLUDE_DIR=${ORC_LZ4_INCLUDE_DIR}"
47394752
"-DSNAPPY_INCLUDE_DIR=${ORC_SNAPPY_INCLUDE_DIR}"
47404753
"-DZSTD_HOME=${ORC_ZSTD_ROOT}"
47414754
"-DZSTD_INCLUDE_DIR=$<TARGET_PROPERTY:${ARROW_ZSTD_LIBZSTD},INTERFACE_INCLUDE_DIRECTORIES>"

0 commit comments

Comments
 (0)