kou commented on code in PR #46390:
URL: https://github.com/apache/arrow/pull/46390#discussion_r2085811020
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -2652,33 +2652,37 @@ 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})
-
- 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)
-
- list(APPEND ARROW_BUNDLED_STATIC_LIBS LZ4::lz4)
+ # 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)
+
+ # Add lz4 as an interfece library so other targets can depend on it
Review Comment:
```suggestion
# Use LZ4::lz4 as an imported library not an alias of lz4_static so other
targets such as orc
# can depend on it as an external library. External libraries are ignored
in
# install(TARGETS orc EXPORT orc_targets) and install(EXPORT orc_targets).
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -2652,33 +2652,37 @@ 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})
-
- 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)
-
- list(APPEND ARROW_BUNDLED_STATIC_LIBS LZ4::lz4)
+ # 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)
+
+ # Add lz4 as an interfece library so other targets can depend on it
+ add_library(LZ4::lz4 INTERFACE IMPORTED)
+ target_link_libraries(LZ4::lz4 INTERFACE lz4_static)
+
+ # Add to bundled static libs
Review Comment:
```suggestion
# Add to bundled static libs.
# We must use lz4_static (not imported target) not LZ4::lz4 (imported
target).
```
--
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]
