This is an automated email from the ASF dual-hosted git repository. wesm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push: new 4fe873e ARROW-4903: [C++] Fix static/shared-only builds 4fe873e is described below commit 4fe873e58ee4355e6d093c9ea897f4b02c5ab19b Author: Uwe L. Korn <uw...@xhochy.com> AuthorDate: Sat Mar 16 18:12:33 2019 -0500 ARROW-4903: [C++] Fix static/shared-only builds This also add two new docker configurations (`cpp-static-only` and `cpp-shared-only`) to test for these issues. This should also fix https://issues.apache.org/jira/browse/ARROW-4848 Author: Uwe L. Korn <uw...@xhochy.com> Closes #3928 from xhochy/ARROW-4903 and squashes the following commits: a8070e35f <Uwe L. Korn> clang-format 20b779525 <Uwe L. Korn> cmake-format d16900d2b <Uwe L. Korn> ARROW-4903: Fix static/shared-only builds --- ci/docker_build_cpp.sh | 3 ++ cpp/examples/parquet/CMakeLists.txt | 7 ++++- cpp/src/arrow/flight/CMakeLists.txt | 34 ++++++++++---------- cpp/src/arrow/ipc/test-common.h | 63 +++++++++++++++++++------------------ cpp/src/parquet/CMakeLists.txt | 13 +++++--- cpp/src/plasma/CMakeLists.txt | 22 ++++++++----- docker-compose.yml | 23 ++++++++++++++ 7 files changed, 106 insertions(+), 59 deletions(-) diff --git a/ci/docker_build_cpp.sh b/ci/docker_build_cpp.sh index 782267a..78e14b5 100755 --- a/ci/docker_build_cpp.sh +++ b/ci/docker_build_cpp.sh @@ -50,6 +50,9 @@ cmake -GNinja \ -DARROW_BUILD_UTILITIES=${ARROW_BUILD_UTILITIES:-ON} \ -DARROW_INSTALL_NAME_RPATH=${ARROW_INSTALL_NAME_RPATH:-ON} \ -DARROW_EXTRA_ERROR_CONTEXT=ON \ + -DARROW_BUILD_SHARED=${ARROW_BUILD_SHARED:-ON} \ + -DARROW_BUILD_STATIC=${ARROW_BUILD_STATIC:-ON} \ + -DARROW_TEST_LINKAGE=${ARROW_TEST_LINKAGE:-shared} \ -DCMAKE_CXX_FLAGS=$CXXFLAGS \ ${CMAKE_ARGS} \ ${source_dir} diff --git a/cpp/examples/parquet/CMakeLists.txt b/cpp/examples/parquet/CMakeLists.txt index db172a2..f2722b1 100644 --- a/cpp/examples/parquet/CMakeLists.txt +++ b/cpp/examples/parquet/CMakeLists.txt @@ -23,7 +23,12 @@ target_link_libraries(parquet-low-level-example parquet_static) target_link_libraries(parquet-low-level-example2 parquet_static) add_executable(parquet-arrow-example parquet-arrow/reader-writer.cc) -target_link_libraries(parquet-arrow-example parquet_shared) +# Prefer shared linkage but use static if shared build is deactivated +if (ARROW_BUILD_SHARED) + target_link_libraries(parquet-arrow-example parquet_shared) +else() + target_link_libraries(parquet-arrow-example parquet_static) +endif() add_dependencies(parquet parquet-low-level-example diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index f3a3d80..b749342 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -31,11 +31,19 @@ if(GRPC_HAS_ADDRESS_SORTING) list(APPEND ARROW_FLIGHT_STATIC_LINK_LIBS gRPC::address_sorting) endif() -set(ARROW_FLIGHT_TEST_LINK_LIBS - arrow_flight_shared - arrow_flight_testing_shared - ${ARROW_TEST_SHARED_LINK_LIBS} - ${ARROW_FLIGHT_STATIC_LINK_LIBS}) +if(ARROW_TEST_LINKAGE STREQUAL "static") + set(ARROW_FLIGHT_TEST_LINK_LIBS + arrow_flight_static + arrow_flight_testing_static + ${ARROW_TEST_STATIC_LINK_LIBS} + ${ARROW_FLIGHT_STATIC_LINK_LIBS}) +else() + set(ARROW_FLIGHT_TEST_LINK_LIBS + arrow_flight_shared + arrow_flight_testing_shared + ${ARROW_TEST_SHARED_LINK_LIBS} + ${ARROW_FLIGHT_STATIC_LINK_LIBS}) +endif() # TODO(wesm): Protobuf shared vs static linking @@ -149,20 +157,12 @@ if(ARROW_BUILD_BENCHMARKS) DEPENDS ${PROTO_DEPENDS}) add_executable(arrow-flight-perf-server perf-server.cc perf.pb.cc) - target_link_libraries(arrow-flight-perf-server - arrow_flight_shared - arrow_flight_testing_shared - ${ARROW_FLIGHT_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} - GTest::GTest) + target_link_libraries(arrow-flight-perf-server ${ARROW_FLIGHT_TEST_LINK_LIBS} + ${GFLAGS_LIBRARIES} GTest::GTest) add_executable(arrow-flight-benchmark flight-benchmark.cc perf.pb.cc) - target_link_libraries(arrow-flight-benchmark - arrow_flight_static - arrow_testing_static - ${ARROW_FLIGHT_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} - GTest::GTest) + target_link_libraries(arrow-flight-benchmark ${ARROW_FLIGHT_TEST_LINK_LIBS} + ${GFLAGS_LIBRARIES} GTest::GTest) add_dependencies(arrow-flight-benchmark arrow-flight-perf-server) diff --git a/cpp/src/arrow/ipc/test-common.h b/cpp/src/arrow/ipc/test-common.h index 3421360..8593fbc 100644 --- a/cpp/src/arrow/ipc/test-common.h +++ b/cpp/src/arrow/ipc/test-common.h @@ -67,8 +67,9 @@ static inline void CompareBatchColumnsDetailed(const RecordBatch& result, const auto kListInt32 = list(int32()); const auto kListListInt32 = list(kListInt32); -Status MakeRandomInt32Array(int64_t length, bool include_nulls, MemoryPool* pool, - std::shared_ptr<Array>* out, uint32_t seed = 0) { +static inline Status MakeRandomInt32Array(int64_t length, bool include_nulls, + MemoryPool* pool, std::shared_ptr<Array>* out, + uint32_t seed = 0) { random::RandomArrayGenerator rand(seed); const double null_probability = include_nulls ? 0.5 : 0.0; @@ -77,9 +78,9 @@ Status MakeRandomInt32Array(int64_t length, bool include_nulls, MemoryPool* pool return Status::OK(); } -Status MakeRandomListArray(const std::shared_ptr<Array>& child_array, int num_lists, - bool include_nulls, MemoryPool* pool, - std::shared_ptr<Array>* out) { +static inline Status MakeRandomListArray(const std::shared_ptr<Array>& child_array, + int num_lists, bool include_nulls, + MemoryPool* pool, std::shared_ptr<Array>* out) { // Create the null list values std::vector<uint8_t> valid_lists(num_lists); const double null_percent = include_nulls ? 0.1 : 0; @@ -123,8 +124,8 @@ Status MakeRandomListArray(const std::shared_ptr<Array>& child_array, int num_li typedef Status MakeRecordBatch(std::shared_ptr<RecordBatch>* out); -Status MakeRandomBooleanArray(const int length, bool include_nulls, - std::shared_ptr<Array>* out) { +static inline Status MakeRandomBooleanArray(const int length, bool include_nulls, + std::shared_ptr<Array>* out) { std::vector<uint8_t> values(length); random_null_bytes(length, 0.5, values.data()); std::shared_ptr<Buffer> data; @@ -142,7 +143,8 @@ Status MakeRandomBooleanArray(const int length, bool include_nulls, return Status::OK(); } -Status MakeBooleanBatchSized(const int length, std::shared_ptr<RecordBatch>* out) { +static inline Status MakeBooleanBatchSized(const int length, + std::shared_ptr<RecordBatch>* out) { // Make the schema auto f0 = field("f0", boolean()); auto f1 = field("f1", boolean()); @@ -155,12 +157,12 @@ Status MakeBooleanBatchSized(const int length, std::shared_ptr<RecordBatch>* out return Status::OK(); } -Status MakeBooleanBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeBooleanBatch(std::shared_ptr<RecordBatch>* out) { return MakeBooleanBatchSized(1000, out); } -Status MakeIntBatchSized(int length, std::shared_ptr<RecordBatch>* out, - uint32_t seed = 0) { +static inline Status MakeIntBatchSized(int length, std::shared_ptr<RecordBatch>* out, + uint32_t seed = 0) { // Make the schema auto f0 = field("f0", int32()); auto f1 = field("f1", int32()); @@ -175,7 +177,7 @@ Status MakeIntBatchSized(int length, std::shared_ptr<RecordBatch>* out, return Status::OK(); } -Status MakeIntRecordBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeIntRecordBatch(std::shared_ptr<RecordBatch>* out) { return MakeIntBatchSized(10, out); } @@ -215,8 +217,8 @@ Status MakeBinaryArrayWithUniqueValues(int64_t length, bool include_nulls, return builder.Finish(out); } -Status MakeStringTypesRecordBatch(std::shared_ptr<RecordBatch>* out, - bool with_nulls = true) { +static inline Status MakeStringTypesRecordBatch(std::shared_ptr<RecordBatch>* out, + bool with_nulls = true) { const int64_t length = 500; auto string_type = utf8(); auto binary_type = binary(); @@ -243,11 +245,12 @@ Status MakeStringTypesRecordBatch(std::shared_ptr<RecordBatch>* out, return Status::OK(); } -Status MakeStringTypesRecordBatchWithNulls(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeStringTypesRecordBatchWithNulls( + std::shared_ptr<RecordBatch>* out) { return MakeStringTypesRecordBatch(out, true); } -Status MakeNullRecordBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeNullRecordBatch(std::shared_ptr<RecordBatch>* out) { const int64_t length = 500; auto f0 = field("f0", null()); auto schema = ::arrow::schema({f0}); @@ -256,7 +259,7 @@ Status MakeNullRecordBatch(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeListRecordBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeListRecordBatch(std::shared_ptr<RecordBatch>* out) { // Make the schema auto f0 = field("f0", kListInt32); auto f1 = field("f1", kListListInt32); @@ -279,7 +282,7 @@ Status MakeListRecordBatch(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeZeroLengthRecordBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeZeroLengthRecordBatch(std::shared_ptr<RecordBatch>* out) { // Make the schema auto f0 = field("f0", kListInt32); auto f1 = field("f1", kListListInt32); @@ -299,7 +302,7 @@ Status MakeZeroLengthRecordBatch(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeNonNullRecordBatch(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeNonNullRecordBatch(std::shared_ptr<RecordBatch>* out) { // Make the schema auto f0 = field("f0", kListInt32); auto f1 = field("f1", kListListInt32); @@ -322,7 +325,7 @@ Status MakeNonNullRecordBatch(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeDeeplyNestedList(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeDeeplyNestedList(std::shared_ptr<RecordBatch>* out) { const int batch_length = 5; auto type = int32(); @@ -342,7 +345,7 @@ Status MakeDeeplyNestedList(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeStruct(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeStruct(std::shared_ptr<RecordBatch>* out) { // reuse constructed list columns std::shared_ptr<RecordBatch> list_batch; RETURN_NOT_OK(MakeListRecordBatch(&list_batch)); @@ -372,7 +375,7 @@ Status MakeStruct(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeUnion(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeUnion(std::shared_ptr<RecordBatch>* out) { // Define schema std::vector<std::shared_ptr<Field>> union_types( {field("u0", int32()), field("u1", uint8())}); @@ -438,7 +441,7 @@ Status MakeUnion(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeDictionary(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeDictionary(std::shared_ptr<RecordBatch>* out) { const int64_t length = 6; std::vector<bool> is_valid = {true, true, false, true, true, true}; @@ -519,7 +522,7 @@ Status MakeDictionary(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeDictionaryFlat(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeDictionaryFlat(std::shared_ptr<RecordBatch>* out) { const int64_t length = 6; std::vector<bool> is_valid = {true, true, false, true, true, true}; @@ -557,7 +560,7 @@ Status MakeDictionaryFlat(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeDates(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeDates(std::shared_ptr<RecordBatch>* out) { std::vector<bool> is_valid = {true, true, true, false, true, true, true}; auto f0 = field("f0", date32()); auto f1 = field("f1", date64()); @@ -577,7 +580,7 @@ Status MakeDates(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeTimestamps(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeTimestamps(std::shared_ptr<RecordBatch>* out) { std::vector<bool> is_valid = {true, true, true, false, true, true, true}; auto f0 = field("f0", timestamp(TimeUnit::MILLI)); auto f1 = field("f1", timestamp(TimeUnit::NANO, "America/New_York")); @@ -596,7 +599,7 @@ Status MakeTimestamps(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeTimes(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeTimes(std::shared_ptr<RecordBatch>* out) { std::vector<bool> is_valid = {true, true, true, false, true, true, true}; auto f0 = field("f0", time32(TimeUnit::MILLI)); auto f1 = field("f1", time64(TimeUnit::NANO)); @@ -631,7 +634,7 @@ void AppendValues(const std::vector<bool>& is_valid, const std::vector<T>& value } } -Status MakeFWBinary(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeFWBinary(std::shared_ptr<RecordBatch>* out) { std::vector<bool> is_valid = {true, true, true, false}; auto f0 = field("f0", fixed_size_binary(4)); auto f1 = field("f1", fixed_size_binary(0)); @@ -655,7 +658,7 @@ Status MakeFWBinary(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeDecimal(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeDecimal(std::shared_ptr<RecordBatch>* out) { constexpr int kDecimalPrecision = 38; auto type = decimal(kDecimalPrecision, 4); auto f0 = field("f0", type); @@ -684,7 +687,7 @@ Status MakeDecimal(std::shared_ptr<RecordBatch>* out) { return Status::OK(); } -Status MakeNull(std::shared_ptr<RecordBatch>* out) { +static inline Status MakeNull(std::shared_ptr<RecordBatch>* out) { auto f0 = field("f0", null()); // Also put a non-null field to make sure we handle the null array buffers properly diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt index f448064..7ae8ddf 100644 --- a/cpp/src/parquet/CMakeLists.txt +++ b/cpp/src/parquet/CMakeLists.txt @@ -46,7 +46,7 @@ function(ADD_PARQUET_TEST REL_TEST_NAME) # By default we prefer shared linking with libparquet, as it's faster # and uses less disk space, but in some cases we need to force static # linking (see rationale below). - if(ARG_USE_STATIC_LINKING) + if(ARG_USE_STATIC_LINKING OR ARROW_TEST_LINKAGE STREQUAL "static") add_test_case(${REL_TEST_NAME} STATIC_LINK_LIBS ${PARQUET_STATIC_TEST_LINK_LIBS} ${TEST_ARGUMENTS}) else() @@ -110,10 +110,10 @@ set(PARQUET_SHARED_TEST_LINK_LIBS parquet_shared Thrift::thrift) -set(PARQUET_STATIC_TEST_LINK_LIBS ${PARQUET_MIN_TEST_LIBS} - ${ARROW_LIBRARIES_FOR_STATIC_TESTS} parquet_static) +set(PARQUET_STATIC_TEST_LINK_LIBS ${PARQUET_MIN_TEST_LIBS} parquet_static + ${ARROW_LIBRARIES_FOR_STATIC_TESTS}) -if(WIN32) +if(WIN32 OR NOT ARROW_BUILD_SHARED) # The benchmarks depend on some static Thrift symbols set(PARQUET_BENCHMARK_LINK_OPTION STATIC_LINK_LIBS benchmark::benchmark_main parquet_static) @@ -238,6 +238,11 @@ add_arrow_lib(parquet STATIC_LINK_LIBS ${PARQUET_STATIC_LINK_LIBS}) +if(ARROW_BUILD_STATIC AND WIN32) + # ARROW-4848: Static Parquet lib needs to import static symbols on Windows + target_compile_definitions(parquet_static PUBLIC ARROW_STATIC) +endif() + add_dependencies(parquet ${PARQUET_LIBRARIES}) # Thrift requires these definitions for some types that we use diff --git a/cpp/src/plasma/CMakeLists.txt b/cpp/src/plasma/CMakeLists.txt index 07c67a6..2a93a63 100644 --- a/cpp/src/plasma/CMakeLists.txt +++ b/cpp/src/plasma/CMakeLists.txt @@ -126,7 +126,12 @@ list(APPEND PLASMA_EXTERNAL_STORE_SOURCES "external_store.cc" "hash_table_store. # We use static libraries for the plasma_store_server executable so that it can # be copied around and used in different locations. add_executable(plasma_store_server ${PLASMA_EXTERNAL_STORE_SOURCES} store.cc) -target_link_libraries(plasma_store_server plasma_static ${PLASMA_STATIC_LINK_LIBS}) +if(ARROW_BUILD_STATIC) + target_link_libraries(plasma_store_server plasma_static ${PLASMA_STATIC_LINK_LIBS}) +else() + # Fallback to shared libs in the case that static libraries are not build. + target_link_libraries(plasma_store_server plasma_shared ${PLASMA_LINK_LIBS}) +endif() add_dependencies(plasma plasma_store_server) if(ARROW_RPATH_ORIGIN) @@ -237,17 +242,20 @@ function(ADD_PLASMA_TEST REL_TEST_NAME) ${ARG_UNPARSED_ARGUMENTS}) endfunction() -add_plasma_test(test/serialization_tests EXTRA_LINK_LIBS plasma_shared - ${PLASMA_LINK_LIBS}) +if(ARROW_BUILD_SHARED) + set(PLASMA_TEST_LIBS plasma_shared ${PLASMA_LINK_LIBS}) +else() + set(PLASMA_TEST_LIBS plasma_static ${PLASMA_STATIC_LINK_LIBS}) +endif() + +add_plasma_test(test/serialization_tests EXTRA_LINK_LIBS ${PLASMA_TEST_LIBS}) add_plasma_test(test/client_tests EXTRA_LINK_LIBS - plasma_shared - ${PLASMA_LINK_LIBS} + ${PLASMA_TEST_LIBS} EXTRA_DEPENDENCIES plasma_store_server) add_plasma_test(test/external_store_tests EXTRA_LINK_LIBS - plasma_shared - ${PLASMA_LINK_LIBS} + ${PLASMA_TEST_LIBS} EXTRA_DEPENDENCIES plasma_store_server) diff --git a/docker-compose.yml b/docker-compose.yml index e7ed56c..0217e74 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -94,6 +94,29 @@ services: PARQUET_TEST_DATA: /arrow/cpp/submodules/parquet-testing/data volumes: *ubuntu-volumes + cpp-static-only: + # Usage: + # docker-compose build cpp + # docker-compose run cpp-static-only + image: arrow:cpp + shm_size: 2G + environment: + ARROW_BUILD_SHARED: "OFF" + ARROW_TEST_LINKAGE: "static" + PARQUET_TEST_DATA: /arrow/cpp/submodules/parquet-testing/data + volumes: *ubuntu-volumes + + cpp-shared-only: + # Usage: + # docker-compose build cpp + # docker-compose run cpp-static-only + image: arrow:cpp + shm_size: 2G + environment: + ARROW_BUILD_STATIC: "OFF" + PARQUET_TEST_DATA: /arrow/cpp/submodules/parquet-testing/data + volumes: *ubuntu-volumes + cpp-cmake32: # Usage: # docker-compose build cpp-cmake32