Skip to content

Commit

Permalink
apacheGH-38043: [R] Enable all features by default on macOS (apache#3…
Browse files Browse the repository at this point in the history
…8195)

### Rationale for this change

Previously GCS/S3 support would need to be explicitly enabled in source builds (when they are build without `NOT_CRAN`). As we want the macos binaries to be fully featured we should turn the features on when the dependencies exists. 

### What changes are included in this PR?

This PR enables this behavior for macOS only, on Linux setting `NOT_CRAN` or  `LIBARROW_MINIMAL=false` is still required.

### Are these changes tested?

Crossbow and locally (thanks @ paleolimbot )
* Closes: apache#38043

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
  • Loading branch information
3 people authored and loicalleyne committed Nov 13, 2023
1 parent 80ebea6 commit 673b0bb
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 31 deletions.
4 changes: 2 additions & 2 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -4086,6 +4086,7 @@ macro(build_crc32c_once)
${_CRC32C_STATIC_LIBRARY})
target_include_directories(Crc32c::crc32c BEFORE INTERFACE "${CRC32C_INCLUDE_DIR}")
add_dependencies(Crc32c::crc32c crc32c_ep)
list(APPEND ARROW_BUNDLED_STATIC_LIBS Crc32c::crc32c)
endif()
endmacro()

Expand Down Expand Up @@ -4316,8 +4317,7 @@ macro(build_google_cloud_cpp_storage)
absl::synchronization
absl::throw_delegate
absl::time
absl::time_zone
Crc32c::crc32c)
absl::time_zone)
endif()
endmacro()

Expand Down
1 change: 1 addition & 0 deletions dev/tasks/macros.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ on:
"arrow",
repos = c(getOption("arrow.dev_repo"), getOption("repos")),
verbose = TRUE,
type = "source",
INSTALL_opts = "--build"
)

Expand Down
36 changes: 19 additions & 17 deletions dev/tasks/r/github.packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -393,36 +393,38 @@ jobs:
read_parquet(system.file("v0.7.1.parquet", package = "arrow"))
print(arrow_info())
#TODO test macos source build?
test-linux-source:
test-source:
needs: source
name: Test linux source build
runs-on: ubuntu-latest
name: Test {{ '${{ matrix.platform.name }}' }} source build
runs-on: {{ '${{ matrix.platform.runs_on }}' }}
strategy:
fail-fast: false
matrix:
platform:
- {runs_on: "ubuntu-latest", name: "Linux"}
- {runs_on: ["self-hosted", "macos-10.13"] , name: "macOS"}
steps:
- name: Install R
if: matrix.platform.name == 'Linux'
uses: r-lib/actions/setup-r@v2
with:
install-r: false
{{ macros.github_setup_local_r_repo(false, false)|indent }}
{{ macros.github_checkout_arrow()|indent }}
{{ macros.github_checkout_arrow(action_v="3")|indent }}
- name: Install sccache
if: matrix.platform.name == 'Linux'
shell: bash
run: |
arrow/ci/scripts/install_sccache.sh unknown-linux-musl /usr/local/bin
- name: Install R package system dependencies
run: |
sudo arrow/ci/scripts/r_install_system_dependencies.sh
env:
ARROW_GCS: "ON"
ARROW_S3: "ON"
ARROW_SOURCE_HOME: arrow
- name: Install R package system dependencies (Linux)
if: matrix.platform.name == 'Linux'
run: sudo apt-get install -y libcurl4-openssl-dev libssl-dev
- name: Remove arrow/
run: |
rm -rf arrow/
- name: Enable parallel build
run: |
echo "MAKEFLAGS=-j$(nproc)" >> $GITHUB_ENV
- name: Install arrow from nightly repo
cores=`nproc || sysctl -n hw.logicalcpu`
echo "MAKEFLAGS=-j$cores" >> $GITHUB_ENV
- name: Install arrow source package
env:
# Test source build so be sure not to download a binary
LIBARROW_BINARY: "FALSE"
Expand All @@ -444,7 +446,7 @@ jobs:
upload-binaries:
# Only upload binaries if all tests pass.
needs: [r-packages, test-linux-source, test-linux-binary]
needs: [r-packages, test-source, test-linux-binary]
name: Upload artifacts
runs-on: ubuntu-latest
steps:
Expand Down
21 changes: 15 additions & 6 deletions r/configure
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,25 @@ else
ARROW_USE_PKG_CONFIG="false"
fi

# find openssl on macos. macOS ships with libressl. openssl is installable
# with brew, but it is generally not linked. We can over-ride this and find
# openssl but setting OPENSSL_ROOT_DIR (which cmake will pick up later in
# the installation process). FWIW, arrow's cmake process uses this
# same process to find openssl, but doing it now allows us to catch it in
# nixlibs.R and throw a nicer error.
## Find openssl
# Arrow's cmake process uses this same process to find openssl,
# but doing it now allows us to catch it in
# nixlibs.R and activate S3 and GCS support for the source build.

# macOS ships with libressl. openssl is installable with brew, but it is
# generally not linked. We can over-ride this and find
# openssl by setting OPENSSL_ROOT_DIR (which cmake will pick up later in
# the installation process).
if [ "${OPENSSL_ROOT_DIR}" = "" ] && brew --prefix openssl >/dev/null 2>&1; then
export OPENSSL_ROOT_DIR="`brew --prefix openssl`"
export PKG_CONFIG_PATH="${OPENSSL_ROOT_DIR}/lib/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}"
fi
# Look for openssl with pkg-config for non-brew sources(e.g. CRAN) and Linux
if [ "${OPENSSL_ROOT_DIR}" = "" -a "${PKG_CONFIG_AVAILABLE}" = "true" ]; then
if ${PKG_CONFIG} --exists openssl; then
export OPENSSL_ROOT_DIR="`${PKG_CONFIG} --variable=prefix openssl`"
fi
fi

#############
# Functions #
Expand Down
9 changes: 3 additions & 6 deletions r/tools/nixlibs.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ try_download <- function(from_url, to_file, hush = quietly) {
}

not_cran <- env_is("NOT_CRAN", "true")
if (not_cran) {
# enable full featured builds and binaries for macOS (or if the NOT_CRAN variable has been set)
if (not_cran || on_macos) {
# Set more eager defaults
if (env_is("LIBARROW_BINARY", "")) {
Sys.setenv(LIBARROW_BINARY = "true")
Expand Down Expand Up @@ -759,6 +760,7 @@ is_feature_requested <- function(env_varname, default = env_is("LIBARROW_MINIMAL
with_cloud_support <- function(env_var_list) {
arrow_s3 <- is_feature_requested("ARROW_S3")
arrow_gcs <- is_feature_requested("ARROW_GCS")

if (arrow_s3 || arrow_gcs) {
# User wants S3 or GCS support.
# Make sure that we have curl and openssl system libs
Expand All @@ -773,11 +775,6 @@ with_cloud_support <- function(env_var_list) {
cat("**** ", start_msg, " support ", msg, "; building with ", off_flags, "\n")
}

# Check the features
# This duplicates what we do with the test program above when we check
# capabilities for using binaries. We could consider consolidating this
# logic, though these use cmake in order to match exactly what we do in the
# libarrow build, and maybe that increases the fidelity.
if (!cmake_find_package("CURL", NULL, env_var_list)) {
# curl on macos should be installed, so no need to alter this for macos
# TODO: check for apt/yum/etc. and message the right thing?
Expand Down

0 comments on commit 673b0bb

Please sign in to comment.