-
Notifications
You must be signed in to change notification settings - Fork 191
PARQUET-932: Add option to build parquet library with minimal dependency #279
Conversation
| 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
| if (ARROW_INCLUDE_DIR AND ARROW_LIB_PATH) | ||
| if (ARROW_INCLUDE_DIR AND (PARQUET_MINIMAL_DEPENDENCY OR ARROW_LIB_PATH)) | ||
| 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.
| 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.