From d21d81073a2899d42bf4c27aca4eaa716b2a5fbb Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jul 2023 09:59:24 -0300 Subject: [PATCH] chore: Generalize option for static linking of Arrow C++ to link tests (#267) The previous option to use static linking was just a test for `MSVC`; however, the may be other times where using a shared library fails (e.g., #254). This PR generalizes that option as a workaround. --- .github/workflows/verify.yaml | 8 ++++---- CMakeLists.txt | 9 ++++----- extensions/nanoarrow_ipc/CMakeLists.txt | 7 ++++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/verify.yaml b/.github/workflows/verify.yaml index 41560d2ad..a4b1bb581 100644 --- a/.github/workflows/verify.yaml +++ b/.github/workflows/verify.yaml @@ -53,7 +53,7 @@ jobs: matrix: config: - {os: macOS-latest, label: macos} - - {os: windows-latest, label: windows} + - {os: windows-latest, label: windows, extra_cmake_install: "--config=Debug"} - {os: ubuntu-latest, label: ubuntu} steps: @@ -81,7 +81,7 @@ jobs: uses: actions/cache@v3 with: path: arrow - key: arrow-${{ runner.os }}-4 + key: arrow-${{ runner.os }}-5 - name: Build Arrow C++ if: steps.cache-arrow-build.outputs.cache-hit != 'true' @@ -92,14 +92,14 @@ jobs: mkdir arrow-build && cd arrow-build cmake ../apache-arrow-12.0.1/cpp -DCMAKE_INSTALL_PREFIX=../arrow cmake --build . - cmake --install . --prefix=../arrow + cmake --install . --prefix=../arrow ${{ matrix.config.extra_cmake_install }} cd .. - name: Set CMake options (Windows) if: matrix.config.label == 'windows' shell: bash run: | - echo "NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS} -DArrow_DIR=$(pwd -W)/arrow/lib/cmake/Arrow -Dgtest_force_shared_crt=ON" >> $GITHUB_ENV + echo "NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS} -DNANOARROW_ARROW_STATIC=ON -DArrow_DIR=$(pwd -W)/arrow/lib/cmake/Arrow -Dgtest_force_shared_crt=ON" >> $GITHUB_ENV - name: Set CMake options (POSIX) if: matrix.config.label != 'windows' diff --git a/CMakeLists.txt b/CMakeLists.txt index 04fcf3431..5691f3a90 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,8 @@ option(NANOARROW_BUILD_TESTS "Build tests" OFF) option(NANOARROW_BUNDLE "Create bundled nanoarrow.h and nanoarrow.c" OFF) option(NANOARROW_BUNDLE_AS_CPP "Bundle nanoarrow source file as nanoarrow.cc" OFF) option(NANOARROW_NAMESPACE "A prefix for exported symbols" OFF) +option(NANOARROW_ARROW_STATIC + "Use a statically-linked Arrow C++ build when linking tests" OFF) if(NANOARROW_NAMESPACE) set(NANOARROW_NAMESPACE_DEFINE "#define NANOARROW_NAMESPACE ${NANOARROW_NAMESPACE}") @@ -172,11 +174,8 @@ if(NANOARROW_BUILD_TESTS) cmake_policy(SET CMP0135 NEW) endif() - # On Windows, dynamically linking Arrow is difficult to get right, - # so use static linking by default. GTest needs to use shared C runtime - # when using MSVC. - if(MSVC) - set(gtest_force_shared_crt on) + # Give caller the option to link a static version of Arrow C++ + if(NANOARROW_ARROW_STATIC) set(NANOARROW_ARROW_TARGET arrow_static) else() set(NANOARROW_ARROW_TARGET arrow_shared) diff --git a/extensions/nanoarrow_ipc/CMakeLists.txt b/extensions/nanoarrow_ipc/CMakeLists.txt index bceda5011..e2dbb9de7 100644 --- a/extensions/nanoarrow_ipc/CMakeLists.txt +++ b/extensions/nanoarrow_ipc/CMakeLists.txt @@ -32,6 +32,8 @@ option(NANOARROW_IPC_FLATCC_ROOT_DIR "Root directory for flatcc include and lib directories" OFF) option(NANOARROW_IPC_FLATCC_INCLUDE_DIR "Include directory for flatcc includes" OFF) option(NANOARROW_IPC_FLATCC_LIB_DIR "Library directory that contains libflatccrt.a" OFF) +option(NANOARROW_ARROW_STATIC + "Use a statically-linked Arrow C++ build when linking tests" OFF) option(NANOARROW_IPC_CODE_COVERAGE "Enable coverage reporting" OFF) add_library(ipc_coverage_config INTERFACE) @@ -216,9 +218,8 @@ if(NANOARROW_IPC_BUILD_TESTS) target_link_libraries(nanoarrow_ipc PRIVATE ipc_coverage_config) endif() - # On Windows, dynamically linking Arrow is difficult to get right, - # so use static linking by default. - if(MSVC) + # Give caller the option to link a static version of Arrow C++ + if(NANOARROW_ARROW_STATIC) set(NANOARROW_IPC_ARROW_TARGET arrow_static) else() set(NANOARROW_IPC_ARROW_TARGET arrow_shared)