Skip to content

Commit

Permalink
ARROW-15667: [R] Test development build with ARROW_BUILD_STATIC=OFF
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
wjones127 authored and kou committed Mar 3, 2022
1 parent e26a88f commit 2462492
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 28 deletions.
14 changes: 13 additions & 1 deletion cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
23 changes: 9 additions & 14 deletions r/configure.win
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}


Expand Down
27 changes: 14 additions & 13 deletions r/vignettes/developers/setup.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand All @@ -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 \
Expand Down Expand Up @@ -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 \
..
```

Expand All @@ -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):
Expand Down

0 comments on commit 2462492

Please sign in to comment.