GH-38043: [R] Enable all features by default on macOS (#38195)

### 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: #38043 Lead-authored-by: Jacob Wujciak-Jens <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jonathan Keane <[email protected]> Signed-off-by: Jacob Wujciak-Jens <[email protected]>
apache · Oct 13, 2023 · 6cc8214 · 6cc8214
1 parent e5bcfd3
commit 6cc8214
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 31 deletions.
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -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()
 
@@ -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()
 

diff --git a/dev/tasks/macros.jinja b/dev/tasks/macros.jinja
@@ -278,6 +278,7 @@ on:
     "arrow",
     repos = c(getOption("arrow.dev_repo"), getOption("repos")),
     verbose = TRUE,
+    type = "source",
     INSTALL_opts = "--build"
   )
 

diff --git a/dev/tasks/r/github.packages.yml b/dev/tasks/r/github.packages.yml
@@ -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"
@@ -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:

diff --git a/r/configure b/r/configure
@@ -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 #

diff --git a/r/tools/nixlibs.R b/r/tools/nixlibs.R
@@ -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")
@@ -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
@@ -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?