kou commented on code in PR #2416:
URL: https://github.com/apache/orc/pull/2416#discussion_r2415416507
##########
cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -54,50 +54,22 @@ string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_BUILD_TYPE)
if (DEFINED ENV{SNAPPY_HOME})
set (SNAPPY_HOME "$ENV{SNAPPY_HOME}")
-elseif (Snappy_ROOT)
Review Comment:
I'm OK with removing them as an Apache Arrow developer. But controlling
wheter `FindXXXAlt.cmake` or `FetchContent` by only this may not be enough...
I'll comment it on the
https://github.com/apache/orc/pull/2416/files#r2387031578 thread.
##########
cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -170,41 +303,68 @@ elseif (ORC_PACKAGE_KIND STREQUAL "vcpkg")
list (APPEND ORC_SYSTEM_DEPENDENCIES Snappy)
list (APPEND ORC_INSTALL_INTERFACE_TARGETS
"$<INSTALL_INTERFACE:Snappy::snappy>")
elseif (NOT "${SNAPPY_HOME}" STREQUAL "")
- find_package (Snappy REQUIRED)
+ find_package (SnappyAlt REQUIRED)
if (ORC_PREFER_STATIC_SNAPPY AND SNAPPY_STATIC_LIB)
orc_add_resolved_library (orc_snappy ${SNAPPY_STATIC_LIB}
${SNAPPY_INCLUDE_DIR})
else ()
orc_add_resolved_library (orc_snappy ${SNAPPY_LIBRARY}
${SNAPPY_INCLUDE_DIR})
endif ()
- list (APPEND ORC_SYSTEM_DEPENDENCIES Snappy)
+ list (APPEND ORC_SYSTEM_DEPENDENCIES SnappyAlt)
list (APPEND ORC_INSTALL_INTERFACE_TARGETS
"$<INSTALL_INTERFACE:Snappy::snappy>")
- orc_provide_find_module (Snappy)
+ orc_provide_find_module (SnappyAlt)
else ()
Review Comment:
Ah, you're right. Sorry.
How about the following?
```cmake
if (ORC_PACKAGE_KIND STREQUAL "conan")
# ...
elseif (ORC_PACKAGE_KIND STREQUAL "vcpkg")
# ...
elseif (TARGET Snappy::snappy)
# Use existing target if possible: This is for FetchContent use case like
Apache Arrow does
add_library(orc_snappy ALIAS Snappy::snappy)
elseif (TARGET Snappy::snappy-static)
# Use existing target if possible: This is for FetchContent use case like
Apache Arrow does
add_library(orc_snappy ALIAS Snappy::snappy-static)
elseif (ORC_PACKAGE_KIND_SNAPPY STREQUAL "system")
find_package(Snappy REQUIRED)
# ....
else ()
if (ORC_PACKAGE_KIND_SNAPPY STREQUAL "bundled")
set(Snappy_FOUND FALSE)
else ()
find_package(Snappy)
endif ()
if (Snappy_FOUND)
add_library(orc_snappy ALIAS Snappy::snappy)
else ()
# Use FetchContent
# ...
endif ()
endif ()
```
Apache Arrow will use the following part for bundled Apache ORC:
```cmake
elseif (TARGET Snappy::snappy)
# Use existing target if possible: This is for FetchContent use case like
Apache Arrow does
add_library(orc_snappy ALIAS Snappy::snappy)
elseif (TARGET Snappy::snappy-static)
# Use existing target if possible: This is for FetchContent use case like
Apache Arrow does
add_library(orc_snappy ALIAS Snappy::snappy-static)
```
If this works, we can remove `FindSnappyAlt.cmake` and `SNAPPY_HOME`
entirely.
```
--
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]
