Skip to content
This repository was archived by the owner on May 10, 2024. It is now read-only.

PARQUET-932: Add option to build parquet library with minimal dependency #279

Closed
wants to merge 4 commits into from

Conversation

majetideepak
Copy link

@majetideepak majetideepak commented Mar 30, 2017

If users want to build only the parquet library, thirdparty headers are sufficient and GTEST and GBENCHMARK are not required.

@@ -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)
Copy link
Author

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

@majetideepak
Copy link
Author

@xhochy, @wesm Can you give your feedback on this? I would like this patch to be committed soon.Thanks!

Copy link
Member

@wesm wesm left a 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?

set(ARROW_FOUND TRUE)
set(ARROW_HEADER_NAME arrow/arrow.h)
Copy link
Member

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

Copy link
Author

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))
Copy link
Member

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct.

@wesm
Copy link
Member

wesm commented Apr 1, 2017

+1, merging this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants