-
Notifications
You must be signed in to change notification settings - Fork 192
PARQUET-932: Add option to build parquet library with minimal dependency #279
Conversation
@@ -103,10 +103,14 @@ if ("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}") | |||
option(PARQUET_RPATH_ORIGIN | |||
"Build Parquet libraries with RATH set to \$ORIGIN" | |||
OFF) | |||
option(PARQUET_MINIMAL_DEPENDENCY | |||
"Depend only on Thirdparty headers to build libparquet. Always OFF if building binaries" | |||
OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also implicitly set this value to OFF
by default and set it to ON
if PARQUET_BUILD_TESTS
and PARQUET_BUILD_EXECUTABLES
and PARQUET_BUILD_BENCHMARKS
are all OFF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks OK to me (couple questions/comments), @xhochy can you also take a look?
cmake_modules/FindArrow.cmake
Outdated
set(ARROW_FOUND TRUE) | ||
set(ARROW_HEADER_NAME arrow/arrow.h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's called arrow/api.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -51,8 +51,10 @@ find_library(ARROW_IO_LIB_PATH NAMES arrow_io | |||
${ARROW_SEARCH_LIB_PATH} | |||
NO_DEFAULT_PATH) | |||
|
|||
if (ARROW_INCLUDE_DIR AND ARROW_LIB_PATH) | |||
if (ARROW_INCLUDE_DIR AND (PARQUET_MINIMAL_DEPENDENCY OR ARROW_LIB_PATH)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so I understand, you have a build environment where only the headers are available, and no libraries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct.
+1, merging this |
If users want to build only the parquet library, thirdparty headers are sufficient and GTEST and GBENCHMARK are not required.