Skip to content

Commit

Permalink
ARROW-17394: [C++][Parquet] Fix parquet_static dependencies (apache#1…
Browse files Browse the repository at this point in the history
…3863)

parquet_static needs Thrift because cpp/src/parquet/thrift_internal.h
uses Thrift.

See also:
microsoft/vcpkg#22552 (comment)

We can test this by the following command lines:

    $ cd cpp/examples/parquet/parquet_arrow
    $ export Arrow_DIR=${ARROW_INSTALL_PREFIX}/lib/cmake/arrow
    $ export Parquet_DIR=${ARROW_INSTALL_PREFIX}/lib/cmake/arrow
    $ cmake -S . -B build -DPARQUET_LINK_SHARED=OFF
    $ cmake --build build --verbose

I also noticed that OpenTelemetry related dependencies are also missed
because I usually use -DARROW_WITH_OPENTELEMETRY=ON option for Apache
Arrow development. The problem is also fixed in this.

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
  • Loading branch information
kou authored and ksuarez1423 committed Aug 15, 2022
1 parent 5982093 commit 980c77e
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
11 changes: 11 additions & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,17 @@ if(ARROW_WITH_OPENTELEMETRY)
opentelemetry-cpp::trace
opentelemetry-cpp::ostream_span_exporter
opentelemetry-cpp::otlp_http_exporter)
if(opentelemetry_SOURCE STREQUAL "SYSTEM")
list(APPEND
ARROW_STATIC_INSTALL_INTERFACE_LIBS
opentelemetry-cpp::trace
opentelemetry-cpp::ostream_span_exporter
opentelemetry-cpp::otlp_http_exporter)
endif()
if(Protobuf_SOURCE STREQUAL "SYSTEM")
list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS ${ARROW_PROTOBUF_LIBPROTOBUF})
endif()
list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS CURL::libcurl)
endif()

if(ARROW_WITH_UTF8PROC)
Expand Down
3 changes: 2 additions & 1 deletion cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,7 @@ endif()
macro(find_curl)
if(NOT TARGET CURL::libcurl)
find_package(CURL REQUIRED)
list(APPEND ARROW_SYSTEM_DEPENDENCIES CURL)
if(NOT TARGET CURL::libcurl)
# For CMake 3.11 or older
add_library(CURL::libcurl UNKNOWN IMPORTED)
Expand Down Expand Up @@ -4540,11 +4541,11 @@ macro(build_opentelemetry)

foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_LIBS})
add_dependencies(opentelemetry-cpp::${_OPENTELEMETRY_LIB} opentelemetry_ep)
list(APPEND ARROW_BUNDLED_STATIC_LIBS opentelemetry-cpp::${_OPENTELEMETRY_LIB})
endforeach()

# Work around https://gitlab.kitware.com/cmake/cmake/issues/15052
file(MAKE_DIRECTORY ${OPENTELEMETRY_INCLUDE_DIR})

endmacro()

if(ARROW_WITH_OPENTELEMETRY)
Expand Down
8 changes: 6 additions & 2 deletions cpp/examples/parquet/parquet_arrow/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ include(ExternalProject)
include(FindPkgConfig)
include(GNUInstallDirs)

set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake_modules")
option(PARQUET_LINK_SHARED "Link to the Parquet shared library" ON)

# This ensures that things like gnu++11 get passed correctly
if(NOT DEFINED CMAKE_CXX_STANDARD)
Expand All @@ -39,4 +39,8 @@ find_package(Arrow REQUIRED)
find_package(Parquet REQUIRED)

add_executable(parquet-arrow-example reader_writer.cc)
target_link_libraries(parquet-arrow-example parquet_shared arrow_shared)
if(PARQUET_LINK_SHARED)
target_link_libraries(parquet-arrow-example parquet_shared)
else()
target_link_libraries(parquet-arrow-example parquet_static)
endif()
13 changes: 10 additions & 3 deletions cpp/src/parquet/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,11 @@ endfunction()

if(ARROW_BUILD_STATIC)
set(PARQUET_STATIC_LINK_LIBS arrow_static ${ARROW_STATIC_LINK_LIBS})
set(PARQUET_STATIC_INTERFACE_INSTALL_LIBS arrow_static)
set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_static arrow_static
${ARROW_STATIC_LINK_LIBS})
else()
set(PARQUET_STATIC_INTERFACE_INSTALL_LIBS)
set(ARROW_LIBRARIES_FOR_STATIC_TESTS arrow_testing_shared arrow_shared)
endif()

Expand Down Expand Up @@ -218,8 +220,9 @@ if(NOT PARQUET_MINIMAL_DEPENDENCY)

# Link publicly with parquet_static (because internal users need to
# transitively link all dependencies)
set(PARQUET_STATIC_LINK_LIBS ${PARQUET_STATIC_LINK_LIBS} thrift::thrift)
endif(NOT PARQUET_MINIMAL_DEPENDENCY)
list(APPEND PARQUET_STATIC_LINK_LIBS thrift::thrift)
list(APPEND PARQUET_STATIC_INTERFACE_INSTALL_LIBS thrift::thrift)
endif()

if(CXX_LINKER_SUPPORTS_VERSION_SCRIPT)
set(PARQUET_SHARED_LINK_FLAGS
Expand All @@ -243,8 +246,12 @@ add_arrow_lib(parquet
${PARQUET_SHARED_LINK_LIBS}
SHARED_PRIVATE_LINK_LIBS
${PARQUET_SHARED_PRIVATE_LINK_LIBS}
SHARED_INSTALL_INTERFACE_LIBS
arrow_shared
STATIC_LINK_LIBS
${PARQUET_STATIC_LINK_LIBS})
${PARQUET_STATIC_LINK_LIBS}
STATIC_INSTALL_INTERFACE_LIBS
${PARQUET_STATIC_INTERFACE_INSTALL_LIBS})

if(WIN32 AND NOT (ARROW_TEST_LINKAGE STREQUAL "static"))
add_library(parquet_test_support STATIC
Expand Down

0 comments on commit 980c77e

Please sign in to comment.