From 2462492389a8f2ca286c481852c84ba1f0d0eff9 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Fri, 4 Mar 2022 06:08:42 +0900 Subject: [PATCH] ARROW-15667: [R] Test development build with ARROW_BUILD_STATIC=OFF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I found that on Windows, the build would fail if `ARROW_BUILD_STATIC=OFF`, even though it was supposed to be using shared libraries. To make things simpler, I changed `configure.win` to just use the pkg-config files. This works for dynamic libraries. However, if you try to do a static-only build, it fails. I started to look into why, but am feeling exhausted debugging linking errors. 😩 At the very least, I think this change means if someone fixes the pkg-config file to support static linking, then the R static-only build should (hopefully) be fixed automatically. Closes #12406 from wjones127/ARROW-15667-windows-shared-lib Authored-by: Will Jones Signed-off-by: Sutou Kouhei --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 14 ++++++++++- r/configure.win | 23 +++++++----------- r/vignettes/developers/setup.Rmd | 27 +++++++++++---------- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index dde6ab76b38f1..81524653db6cb 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1825,6 +1825,11 @@ if(ARROW_MIMALLOC) IMPORTED_LOCATION "${MIMALLOC_STATIC_LIB}" INTERFACE_INCLUDE_DIRECTORIES "${MIMALLOC_INCLUDE_DIR}") + if(WIN32) + set_property(TARGET mimalloc::mimalloc + APPEND + PROPERTY INTERFACE_LINK_LIBRARIES "bcrypt.lib" "psapi.lib") + endif() add_dependencies(mimalloc::mimalloc mimalloc_ep) add_dependencies(toolchain mimalloc_ep) @@ -2389,7 +2394,14 @@ if(ARROW_WITH_RE2) # source not uses C++ 11. resolve_dependency(re2 HAVE_ALT TRUE) if(${re2_SOURCE} STREQUAL "SYSTEM") - get_target_property(RE2_LIB re2::re2 IMPORTED_LOCATION) + get_target_property(RE2_LIB re2::re2 IMPORTED_LOCATION_${UPPERCASE_BUILD_TYPE}) + if(NOT RE2_LIB) + get_target_property(RE2_LIB re2::re2 IMPORTED_LOCATION_RELEASE) + endif() + if(NOT RE2_LIB) + get_target_property(RE2_LIB re2::re2 IMPORTED_LOCATION) + endif() + string(APPEND ARROW_PC_LIBS_PRIVATE " ${RE2_LIB}") endif() add_definitions(-DARROW_WITH_RE2) diff --git a/r/configure.win b/r/configure.win index 589689851e165..6c31be4521b2b 100644 --- a/r/configure.win +++ b/r/configure.win @@ -85,36 +85,31 @@ function configure_dev() { echo "*** Using locally built Arrow at $ARROW_HOME" RWINLIB=$(cygpath $ARROW_HOME) - PKG_CFLAGS="-I${RWINLIB}/include -DARROW_R_WITH_ARROW" - PKG_LIBS=" -L${RWINLIB}/lib -L$MSYSTEM_PREFIX/lib -larrow -larrow_bundled_dependencies -lz -lole32 ${MIMALLOC_LIBS} ${OPENSSL_LIBS}" - # These are already in arrow_bundled_dependencies: - # -lutf8proc -lthrift -lsnappy -lzstd -llz4 + export PKG_CONFIG_PATH=$(cygpath $ARROW_HOME)/lib/pkgconfig:$(cygpath $MSYSTEM_PREFIX)/lib + PKG_CONFIG_PACKAGES="arrow" + + PKG_CFLAGS="-DARROW_R_WITH_ARROW" if [ $(cmake_option ARROW_PARQUET) -eq 1 ]; then PKG_CFLAGS="$PKG_CFLAGS -DARROW_R_WITH_PARQUET" - PKG_LIBS="-lparquet $PKG_LIBS" - # NOTE: parquet is assumed to have the same -L flag as arrow - # so there is no need to add its location to PKG_DIRS + PKG_CONFIG_PACKAGES="$PKG_CONFIG_PACKAGES parquet" fi if [ $(cmake_option ARROW_DATASET) -eq 1 ]; then PKG_CFLAGS="$PKG_CFLAGS -DARROW_R_WITH_DATASET" - PKG_LIBS="-larrow_dataset $PKG_LIBS" - # NOTE: arrow-dataset is assumed to have the same -L flag as arrow - # so there is no need to add its location to PKG_DIRS + PKG_CONFIG_PACKAGES="$PKG_CONFIG_PACKAGES arrow-dataset" fi if [ $(cmake_option ARROW_S3) -eq 1 ]; then PKG_CFLAGS="$PKG_CFLAGS -DARROW_R_WITH_S3" - if [ "$BUNDLED_LIBS" != "" ]; then - # We're depending on openssl/curl from the system, so they're not in the bundled deps - BUNDLED_LIBS="$BUNDLED_LIBS -lssl -lcrypto -lcurl" - fi fi if [ $(cmake_option ARROW_JSON) -eq 1 ]; then PKG_CFLAGS="$PKG_CFLAGS -DARROW_R_WITH_JSON" fi + + PKG_CFLAGS="$(pkg-config --cflags $PKG_CONFIG_PACKAGES) $PKG_CFLAGS" + PKG_LIBS=$(pkg-config --libs $PKG_CONFIG_PACKAGES) } diff --git a/r/vignettes/developers/setup.Rmd b/r/vignettes/developers/setup.Rmd index 8fdcb0ac339c3..3a8727b70af9e 100644 --- a/r/vignettes/developers/setup.Rmd +++ b/r/vignettes/developers/setup.Rmd @@ -110,13 +110,11 @@ The package can be built on Windows using [RTools 4](https://cran.r-project.org/ Open the corresponding RTools Bash, for example "Rtools MinGW 64-bit" for mingw64. -Install CMake and Ninja with: +Install CMake, ccache, and Ninja with: ```{bash, save=run & windows} pacman --sync --refresh --noconfirm \ - ${MINGW_PACKAGE_PREFIX}-cmake \ - ${MINGW_PACKAGE_PREFIX}-ninja \ - ${MINGW_PACKAGE_PREFIX}-openssl + ${MINGW_PACKAGE_PREFIX}-{ccache,cmake,ninja,openssl} export CMAKE_GENERATOR=Ninja ``` @@ -128,9 +126,10 @@ export PATH=~/Documents/R/R-4.1.2/bin/x64:$PATH You can install additional dependencies like so. Note that you are limited to the packages in [the RTools repo](https://github.com/r-windows/rtools-packages), which does not contain every dependency used by Arrow. -```{bash} +```{bash, save=run & windows} pacman --sync --refresh --noconfirm \ ${MINGW_PACKAGE_PREFIX}-boost \ + ${MINGW_PACKAGE_PREFIX}-brotli \ ${MINGW_PACKAGE_PREFIX}-lz4 \ ${MINGW_PACKAGE_PREFIX}-protobuf \ ${MINGW_PACKAGE_PREFIX}-snappy \ @@ -210,18 +209,11 @@ cmake \ -DARROW_DATASET=ON \ -DARROW_EXTRA_ERROR_CONTEXT=ON \ -DARROW_FILESYSTEM=ON \ - -DARROW_INSTALL_NAME_RPATH=OFF \ + -DARROW_MIMALLOC=ON \ -DARROW_JSON=OFF \ -DARROW_PARQUET=ON \ - -DARROW_WITH_LZ4=OFF \ -DARROW_WITH_SNAPPY=OFF \ -DARROW_WITH_ZLIB=ON \ - -DARROW_MIMALLOC=OFF \ - -DARROW_S3=OFF \ - -DARROW_WITH_BROTLI=ON \ - -DARROW_WITH_BZ2=ON \ - -DARROW_WITH_SNAPPY=ON \ - -DARROW_WITH_ZSTD=ON \ .. ``` @@ -231,6 +223,15 @@ cmake \ **For Windows**: some options, including `-DARROW_JEMALLOC`, are not supported on Windows. + +```{bash, save=run & !sys_install, hide=TRUE} +# For testing purposes, build with only shared libraries +cmake \ + -DARROW_BUILD_SHARED=ON \ + -DARROW_BUILD_STATIC=OFF \ + .. +``` + #### Enabling more Arrow features To enable optional features including: S3 support, an alternative memory allocator, and additional compression libraries, add some or all of these flags to your call to `cmake` (the trailing `\` makes them easier to paste into a bash shell on a new line):