Skip to content

ARROW-189: Build 3rd party with ExternalProject. #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
leifwalsh wants to merge 4 commits into from
Closed

ARROW-189: Build 3rd party with ExternalProject. #167

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8215abc
ARROW-189: Build 3rd party with ExternalProject.
leifwalsh Oct 8, 2016
8630428
ARROW-189: Addressed CR comments.
leifwalsh Oct 11, 2016
7892bae
ARROW-189: Fix darwin build.
leifwalsh Oct 11, 2016
e4fb63a
ARROW-189: Remove 3rd party from conda build.
leifwalsh Oct 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 93 additions & 18 deletions cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ project(arrow)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake_modules")

include(CMakeParseArguments)
include(ExternalProject)

set(BUILD_SUPPORT_DIR "${CMAKE_SOURCE_DIR}/build-support")
set(THIRDPARTY_DIR "${CMAKE_SOURCE_DIR}/thirdparty")

set(GTEST_VERSION "1.7.0")
set(GBENCHMARK_VERSION "1.0.0")
set(FLATBUFFERS_VERSION "1.3.0")

find_package(ClangTools)
if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR CLANG_TIDY_FOUND)
Expand Down Expand Up @@ -422,16 +426,6 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
endif()
endfunction()

## GTest
if ("$ENV{GTEST_HOME}" STREQUAL "")
set(GTest_HOME ${THIRDPARTY_DIR}/googletest-release-1.7.0)
endif()

## Google Benchmark
if ("$ENV{GBENCHMARK_HOME}" STREQUAL "")
set(GBENCHMARK_HOME ${THIRDPARTY_DIR}/installed)
endif()

# ----------------------------------------------------------------------
# Add Boost dependencies (code adapted from Apache Kudu (incubating))

Expand Down Expand Up @@ -476,18 +470,78 @@ include_directories(SYSTEM ${Boost_INCLUDE_DIR})

if(ARROW_BUILD_TESTS)
add_custom_target(unittest ctest -L unittest)
find_package(GTest REQUIRED)

if("$ENV{GTEST_HOME}" STREQUAL "")
if(APPLE)
set(GTEST_CMAKE_ARGS "-DCMAKE_CXX_FLAGS=\"-fPIC -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1_TUPLE=1 -Wno-unused-value -Wno-ignored-attributes\"")
else()
set(GTEST_CMAKE_ARGS "-DCMAKE_CXX_FLAGS=\"-fPIC\"")
endif()

ExternalProject_Add(googletest_ep
URL "https://github.com/google/googletest/archive/release-${GTEST_VERSION}.tar.gz"
CMAKE_ARGS ${GTEST_CMAKE_ARGS}
# googletest doesn't define install rules, so just build in the
# source dir and don't try to install. See its README for
# details.
BUILD_IN_SOURCE 1
INSTALL_COMMAND "/bin/true")

set(GTEST_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/googletest_ep-prefix/src/googletest_ep")
set(GTEST_INCLUDE_DIR "${GTEST_PREFIX}/include")
set(GTEST_STATIC_LIB "${GTEST_PREFIX}/libgtest.a")
add_library(gtest STATIC IMPORTED)
set_target_properties(gtest PROPERTIES
IMPORTED_LOCATION ${GTEST_STATIC_LIB})
add_dependencies(gtest googletest_ep)
set(GTEST_VENDORED 1)
else()
find_package(GTest REQUIRED)
ADD_THIRDPARTY_LIB(gtest
STATIC_LIB ${GTEST_STATIC_LIB})
set(GTEST_VENDORED 0)
endif()

message(STATUS "GTest include dir: ${GTEST_INCLUDE_DIR}")
message(STATUS "GTest static library: ${GTEST_STATIC_LIB}")
include_directories(SYSTEM ${GTEST_INCLUDE_DIR})
ADD_THIRDPARTY_LIB(gtest
STATIC_LIB ${GTEST_STATIC_LIB})
endif()

if(ARROW_BUILD_BENCHMARKS)
add_custom_target(runbenchmark ctest -L benchmark)
find_package(GBenchmark REQUIRED)

if("$ENV{GBENCHMARK_HOME}" STREQUAL "")
if(APPLE)
set(GBENCHMARK_CMAKE_CXX_FLAGS "-std=c++11 -stdlib=libc++")
else()
set(GBENCHMARK_CMAKE_CXX_FLAGS "--std=c++11")
endif()

set(GBENCHMARK_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/gbenchmark_ep/src/gbenchmark_ep-install")
ExternalProject_Add(gbenchmark_ep
URL "https://github.com/google/benchmark/archive/v${GBENCHMARK_VERSION}.tar.gz"
CMAKE_ARGS
"-DCMAKE_BUILD_TYPE=Release"
"-DCMAKE_INSTALL_PREFIX:PATH=${GBENCHMARK_PREFIX}"
"-DCMAKE_CXX_FLAGS=-fPIC ${GBENCHMARK_CMAKE_CXX_FLAGS}")

set(GBENCHMARK_INCLUDE_DIR "${GBENCHMARK_PREFIX}/include")
set(GBENCHMARK_STATIC_LIB "${GBENCHMARK_PREFIX}/lib/libbenchmark.a")
add_library(benchmark STATIC IMPORTED)
set_target_properties(benchmark PROPERTIES
IMPORTED_LOCATION ${GBENCHMARK_STATIC_LIB})
add_dependencies(benchmark gbenchmark_ep)
set(GBENCHMARK_VENDORED 1)
else()
find_package(GBenchmark REQUIRED)
ADD_THIRDPARTY_LIB(benchmark
STATIC_LIB ${GBENCHMARK_STATIC_LIB})

@wesm wesm Oct 11, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possible to use ADD_THIRDPARTY_LIB in the ep portion?

@leifwalsh leifwalsh Oct 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, something else was broken when I switched away from it, it should work

set(GBENCHMARK_VENDORED 0)
endif()

message(STATUS "GBenchmark include dir: ${GBENCHMARK_INCLUDE_DIR}")
message(STATUS "GBenchmark static library: ${GBENCHMARK_STATIC_LIB}")
include_directories(SYSTEM ${GBENCHMARK_INCLUDE_DIR})
ADD_THIRDPARTY_LIB(benchmark
STATIC_LIB ${GBENCHMARK_STATIC_LIB})
endif()

## Google PerfTools
Expand Down Expand Up @@ -705,7 +759,24 @@ add_subdirectory(src/arrow/types)

## Flatbuffers
if(ARROW_IPC)
find_package(Flatbuffers REQUIRED)
if("$ENV{FLATBUFFERS_HOME}" STREQUAL "")
set(FLATBUFFERS_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/flatbuffers_ep-prefix/src/flatbuffers_ep-install")
ExternalProject_Add(flatbuffers_ep
URL "https://github.com/google/flatbuffers/archive/v${FLATBUFFERS_VERSION}.tar.gz"
CMAKE_ARGS
"-DCMAKE_CXX_FLAGS=-fPIC"
"-DCMAKE_INSTALL_PREFIX:PATH=${FLATBUFFERS_PREFIX}"
"-DFLATBUFFERS_BUILD_TESTS=OFF")

set(FLATBUFFERS_INCLUDE_DIR "${FLATBUFFERS_PREFIX}/include")
set(FLATBUFFERS_STATIC_LIB "${FLATBUFFERS_PREFIX}/libflatbuffers.a")
set(FLATBUFFERS_COMPILER "${FLATBUFFERS_PREFIX}/bin/flatc")
set(FLATBUFFERS_VENDORED 1)
else()
find_package(Flatbuffers REQUIRED)
set(FLATBUFFERS_VENDORED 0)
endif()

message(STATUS "Flatbuffers include dir: ${FLATBUFFERS_INCLUDE_DIR}")
message(STATUS "Flatbuffers static library: ${FLATBUFFERS_STATIC_LIB}")
message(STATUS "Flatbuffers compiler: ${FLATBUFFERS_COMPILER}")
Expand All @@ -714,5 +785,9 @@ if(ARROW_IPC)
set_target_properties(flatbuffers PROPERTIES
IMPORTED_LOCATION ${FLATBUFFERS_STATIC_LIB})

if(FLATBUFFERS_VENDORED)
add_dependencies(flatbuffers flatbuffers_ep)
endif()

add_subdirectory(src/arrow/ipc)
endif()
27 changes: 20 additions & 7 deletions cpp/src/arrow/ipc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ set(ARROW_IPC_SRCS
add_library(arrow_ipc SHARED
${ARROW_IPC_SRCS}
)
if(FLATBUFFERS_VENDORED)
add_dependencies(arrow_ipc flatbuffers_ep)
endif()
target_link_libraries(arrow_ipc
LINK_PUBLIC ${ARROW_IPC_LINK_LIBS}
LINK_PRIVATE ${ARROW_IPC_PRIVATE_LINK_LIBS})
Expand Down Expand Up @@ -84,13 +87,23 @@ foreach(FIL ${FBS_SRC})
list(APPEND ABS_FBS_SRC ${ABS_FIL})
endforeach()

add_custom_command(
OUTPUT ${FBS_OUTPUT_FILES}
COMMAND ${FLATBUFFERS_COMPILER} -c -o ${OUTPUT_DIR} ${ABS_FBS_SRC}
DEPENDS ${ABS_FBS_SRC}
COMMENT "Running flatc compiler on ${ABS_FBS_SRC}"
VERBATIM
)
if(FLATBUFFERS_VENDORED)
add_custom_command(
OUTPUT ${FBS_OUTPUT_FILES}
COMMAND ${FLATBUFFERS_COMPILER} -c -o ${OUTPUT_DIR} ${ABS_FBS_SRC}
DEPENDS ${ABS_FBS_SRC} flatbuffers_ep
COMMENT "Running flatc compiler on ${ABS_FBS_SRC}"
VERBATIM
)
else()
add_custom_command(
OUTPUT ${FBS_OUTPUT_FILES}
COMMAND ${FLATBUFFERS_COMPILER} -c -o ${OUTPUT_DIR} ${ABS_FBS_SRC}
DEPENDS ${ABS_FBS_SRC}
COMMENT "Running flatc compiler on ${ABS_FBS_SRC}"
VERBATIM
)

@wesm wesm Oct 11, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any issues with setting the dependencies in a cmake variable and having one add_custom_command call?

@leifwalsh leifwalsh Oct 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure

endif()

add_custom_target(metadata_fbs DEPENDS ${FBS_OUTPUT_FILES})
add_dependencies(arrow_objlib metadata_fbs)
Expand Down