Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-37923: [R] Move macOS build system to nixlibs.R #37684

Merged
merged 78 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 77 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
d043723
add macos libarrow build
assignUser Sep 5, 2023
5684883
use brewfile for deps
assignUser Sep 6, 2023
f54d112
use macos-11 instead of self-hosted 10.13
assignUser Sep 6, 2023
fffcaab
remove gcs from brew file
assignUser Sep 6, 2023
f020024
update brewfile
assignUser Sep 6, 2023
7e510fa
add macos binary test
assignUser Sep 6, 2023
0968bbc
add debug to configure
assignUser Sep 6, 2023
6ebbb19
force bundled builds
assignUser Sep 8, 2023
e48c4c9
escape gha macros
assignUser Sep 9, 2023
666c8af
Revert "add debug to configure"
assignUser Sep 13, 2023
0948def
use setup-r to avoid issues with unset libraries
assignUser Sep 13, 2023
ed105a8
fix typo
assignUser Sep 13, 2023
7dd95d2
fix file name
assignUser Sep 13, 2023
f224918
remove autobrew path in configure
assignUser Sep 13, 2023
03a318c
don't install brewfile
assignUser Sep 13, 2023
bf641ff
escape gha macro
assignUser Sep 13, 2023
9001053
force bundled build and additional flags to enable features
assignUser Sep 19, 2023
3bae0ee
remove formula pinning from jinja macro
assignUser Sep 20, 2023
f5eae5f
enable precompiled binaries via arrow.(dev_)repo on macos
assignUser Sep 20, 2023
6e12270
use precompiled binaries in nightly packages
assignUser Sep 20, 2023
8992a18
Revert "remove formula pinning from jinja macro"
assignUser Sep 20, 2023
1291fa2
add correct arch name to macro
assignUser Sep 20, 2023
82ccfad
fix select_binary
assignUser Sep 20, 2023
229116c
fix openssl include dir for test compile
assignUser Sep 20, 2023
1f6c155
ensure openssl
assignUser Sep 20, 2023
3ded21c
correctly return include dir
assignUser Sep 20, 2023
960fbbe
fix repo names
assignUser Sep 20, 2023
2f5cf07
update snappy version
assignUser Sep 21, 2023
ded46f1
fix typo
assignUser Sep 21, 2023
f68ddf3
disable -Werror for snappy
assignUser Sep 21, 2023
93d1a8f
patch snappy
assignUser Sep 21, 2023
84e3648
disable werror explicitly again
assignUser Sep 21, 2023
550c936
use patch
assignUser Sep 22, 2023
0bd9d3b
disable libcpp availability check
assignUser Sep 26, 2023
122bf4f
explicitly start sccache
assignUser Sep 26, 2023
e21f51b
Patch to add <functional> on macos 10.13
assignUser Sep 27, 2023
d72b6b6
Disable libcpp availability check on macos <11
assignUser Sep 27, 2023
2773faa
uncomment conditional
assignUser Sep 27, 2023
854a47c
disable libcpp checks for R package too
assignUser Sep 27, 2023
ed536b3
fix typo
assignUser Sep 27, 2023
b150b8a
make macos 10.13 check sh compatible
assignUser Sep 27, 2023
01e8e95
test another approach to version checking
assignUser Sep 27, 2023
3f555b5
fix openssl root dir
assignUser Sep 27, 2023
3b760a0
use cran style openssl
assignUser Sep 27, 2023
fbc4b87
disable centos binary test
assignUser Sep 27, 2023
5bb1965
fix needs errors
assignUser Sep 28, 2023
99b0231
print test program invocation
assignUser Sep 28, 2023
0bdef97
revert - diable windows build
assignUser Sep 28, 2023
7b84306
remove centos job from needs
assignUser Sep 28, 2023
53308de
adapt test program to libc++
assignUser Sep 28, 2023
0b1cdea
update artifact list
assignUser Sep 28, 2023
29396fe
Revert "revert - diable windows build"
assignUser Sep 28, 2023
22a0d9b
move openssl include before system include
assignUser Sep 28, 2023
80f4595
remove cran style openssl for now
assignUser Sep 28, 2023
cd473bc
format cmake
assignUser Sep 28, 2023
07a72d7
fix openssl version for libarrow
assignUser Sep 28, 2023
29a8264
remove debug print
assignUser Sep 28, 2023
2433995
add snappy patch to rat excludes
assignUser Sep 28, 2023
d2984ea
update comment
assignUser Sep 28, 2023
4bb76c8
escape gha macro
assignUser Sep 28, 2023
60635af
explicitly set openssl root dir
assignUser Sep 28, 2023
67528a2
address review comments
assignUser Sep 28, 2023
6a040aa
fix artifact names
assignUser Sep 28, 2023
c74a8af
add comment about snappy adding Werror on clang
assignUser Sep 29, 2023
6df174d
use macro to exclude libcpp test instead of paste
assignUser Sep 29, 2023
0887b2e
add libstdc++ check for macos
assignUser Sep 29, 2023
01b8213
add explaining comment
assignUser Sep 29, 2023
8f9d3e8
format
assignUser Sep 29, 2023
7d69673
Update dev/tasks/macros.jinja
assignUser Sep 29, 2023
4c19ac5
check for header in openssl root dir
assignUser Sep 29, 2023
6aa5ab3
remove libstdc++ check
assignUser Oct 3, 2023
2f11d2d
add missing endif
assignUser Oct 3, 2023
2e4d94e
Apply suggestions from code review
assignUser Oct 3, 2023
14ea558
Update r/configure
assignUser Oct 3, 2023
b5239d1
remove ws
assignUser Oct 4, 2023
e6a127e
remove werror hunk from snappy patch
assignUser Oct 4, 2023
99cfe8a
format
assignUser Oct 4, 2023
eca2134
Apply suggestions from code review
assignUser Oct 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cpp/Brewfile
Copy link
Member Author

@assignUser assignUser Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially used the brew file to install dependencies but due to linking issues (#37716 and #37717) I reverted to all bundled deps for now. I think it still makes sense to add the missing deps to the brewfile though ^^

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ brew "aws-sdk-cpp"
brew "bash"
brew "boost"
brew "brotli"
brew "bzip2"
brew "c-ares"
brew "curl"
brew "ccache"
brew "cmake"
brew "flatbuffers"
Expand All @@ -29,14 +31,18 @@ brew "googletest"
brew "grpc"
brew "llvm@14"
brew "lz4"
brew "mimalloc"
brew "ninja"
brew "node"
brew "openssl@3"
brew "pkg-config"
brew "protobuf"
brew "python"
brew "rapidjson"
brew "re2"
brew "snappy"
brew "thrift"
brew "utf8proc"
brew "wget"
brew "xsimd"
brew "zstd"
13 changes: 10 additions & 3 deletions cpp/cmake_modules/SetupCxxFlags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -456,11 +456,18 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STRE
# Don't complain about optimization passes that were not possible
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-pass-failed")

# Avoid clang / libc++ error about C++17 aligned allocation on macOS.
# See https://chromium.googlesource.com/chromium/src/+/eee44569858fc650b635779c4e34be5cb0c73186%5E%21/#F0
# for details.
if(APPLE)
# Avoid clang / libc++ error about C++17 aligned allocation on macOS.
# See https://chromium.googlesource.com/chromium/src/+/eee44569858fc650b635779c4e34be5cb0c73186%5E%21/#F0
# for details.
set(CXX_ONLY_FLAGS "${CXX_ONLY_FLAGS} -fno-aligned-new")
assignUser marked this conversation as resolved.
Show resolved Hide resolved

if(CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20)
# Avoid C++17 std::get 'not available' issue on macOS 10.13
# This will be required until atleast R 4.4 is released and
# CRAN (hopefully) stops checking on 10.13
string(APPEND CXX_ONLY_FLAGS " -D_LIBCPP_DISABLE_AVAILABILITY")
endif()
endif()
endif()

Expand Down
21 changes: 21 additions & 0 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1308,13 +1308,34 @@ macro(build_snappy)
set(SNAPPY_CMAKE_ARGS
${EP_COMMON_CMAKE_ARGS} -DSNAPPY_BUILD_TESTS=OFF -DSNAPPY_BUILD_BENCHMARKS=OFF
"-DCMAKE_INSTALL_PREFIX=${SNAPPY_PREFIX}")
# Snappy unconditionaly enables Werror when building with clang this can lead
# to build failues by way of new compiler warnings. This adds a flag to disable
# Werror to the very end of the invocation to override the snappy internal setting.
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO)
list(APPEND
SNAPPY_CMAKE_ARGS
"-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} -Wno-error"
)
endforeach()
endif()

if(APPLE AND CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20)
# On macOS 10.13 we need to explicitly add <functional> to avoid a missing include error
# This can be removed once CRAN no longer checks on macOS 10.13
find_program(PATCH patch REQUIRED)
set(SNAPPY_PATCH_COMMAND ${PATCH} -p1 -i ${CMAKE_CURRENT_LIST_DIR}/snappy.diff)
assignUser marked this conversation as resolved.
Show resolved Hide resolved
else()
set(SNAPPY_PATCH_COMMAND)
endif()

externalproject_add(snappy_ep
${EP_COMMON_OPTIONS}
BUILD_IN_SOURCE 1
INSTALL_DIR ${SNAPPY_PREFIX}
URL ${SNAPPY_SOURCE_URL}
URL_HASH "SHA256=${ARROW_SNAPPY_BUILD_SHA256_CHECKSUM}"
PATCH_COMMAND ${SNAPPY_PATCH_COMMAND}
CMAKE_ARGS ${SNAPPY_CMAKE_ARGS}
BUILD_BYPRODUCTS "${SNAPPY_STATIC_LIB}")

Expand Down
12 changes: 12 additions & 0 deletions cpp/cmake_modules/snappy.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
diff --git a/snappy.cc b/snappy.cc
index d414718..5b0d0d6 100644
--- a/snappy.cc
+++ b/snappy.cc
@@ -83,6 +83,7 @@
#include <string>
#include <utility>
#include <vector>
+#include <functional>
assignUser marked this conversation as resolved.
Show resolved Hide resolved

namespace snappy {

5 changes: 2 additions & 3 deletions cpp/thirdparty/versions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ ARROW_RAPIDJSON_BUILD_VERSION=232389d4f1012dddec4ef84861face2d2ba85709
ARROW_RAPIDJSON_BUILD_SHA256_CHECKSUM=b9290a9a6d444c8e049bd589ab804e0ccf2b05dc5984a19ed5ae75d090064806
ARROW_RE2_BUILD_VERSION=2022-06-01
ARROW_RE2_BUILD_SHA256_CHECKSUM=f89c61410a072e5cbcf8c27e3a778da7d6fd2f2b5b1445cd4f4508bee946ab0f
# 1.1.9 is patched to implement https://github.com/google/snappy/pull/148 if this is bumped, remove the patch
assignUser marked this conversation as resolved.
Show resolved Hide resolved
ARROW_SNAPPY_BUILD_VERSION=1.1.9
ARROW_SNAPPY_BUILD_SHA256_CHECKSUM=75c1fbb3d618dd3a0483bff0e26d0a92b495bbe5059c8b4f1c962b478b6e06e7
ARROW_SNAPPY_BUILD_VERSION=1.1.10
ARROW_SNAPPY_BUILD_SHA256_CHECKSUM=49d831bffcc5f3d01482340fe5af59852ca2fe76c3e05df0e67203ebbe0f1d90
ARROW_SUBSTRAIT_BUILD_VERSION=v0.27.0
ARROW_SUBSTRAIT_BUILD_SHA256_CHECKSUM=4ed375f69d972a57fdc5ec406c17003a111831d8640d3f1733eccd4b3ff45628
ARROW_S2N_TLS_BUILD_VERSION=v1.3.35
Expand Down
1 change: 1 addition & 0 deletions dev/release/rat_exclude_files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ cpp/build-support/iwyu/*
cpp/cmake_modules/FindPythonLibsNew.cmake
cpp/cmake_modules/SnappyCMakeLists.txt
cpp/cmake_modules/SnappyConfig.h
cpp/cmake_modules/snappy.diff
cpp/examples/parquet/parquet-arrow/cmake_modules/FindArrow.cmake
cpp/src/parquet/.parquetcppversion
cpp/src/generated/parquet_constants.cpp
Expand Down
14 changes: 13 additions & 1 deletion dev/tasks/macros.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ on:
stopifnot(packageVersion("arrow") == {{ '"${{needs.source.outputs.pkg_version}}"' }})
{% endmacro %}

{%- macro github_setup_local_r_repo(get_nix, get_win) -%}
{%- macro github_setup_local_r_repo(get_nix, get_win, get_mac=False) -%}
# TODO: improve arg handling
- name: Setup local repo
shell: bash
run: mkdir repo
Expand All @@ -327,6 +328,17 @@ on:
path: repo/libarrow/bin/linux-openssl-{{ openssl_version }}
{% endfor %}
{% endif %}
{% if get_mac %}
{% for openssl_version in ["1.1", "3.0"] %}
{% for arch in ["x86_64", "arm64"] %}
- name: Get macOS {{ arch }} OpenSSL {{ openssl_version }} binary
uses: actions/download-artifact@v3
with:
name: r-lib__libarrow__bin__darwin-{{arch}}-openssl-{{ openssl_version }}
path: repo/libarrow/bin/darwin-{{ arch }}-openssl-{{ openssl_version }}
{% endfor %}
{% endfor %}
{% endif %}
- name: Get src pkg
uses: actions/download-artifact@v3
with:
Expand Down
80 changes: 68 additions & 12 deletions dev/tasks/r/github.packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,56 @@ jobs:
name: r-pkg__src__contrib
path: arrow/r/arrow_*.tar.gz

macos-cpp:
name: C++ Binary macOS OpenSSL {{ '${{ matrix.openssl }}' }} {{ '${{ matrix.platform.arch }}'}}
assignUser marked this conversation as resolved.
Show resolved Hide resolved
runs-on: {{ '${{ matrix.platform.runs_on }}'}}
assignUser marked this conversation as resolved.
Show resolved Hide resolved
needs: source
strategy:
fail-fast: false
matrix:
platform:
- { runs_on: ["self-hosted", "macos-10.13"], arch: "x86_64" }
assignUser marked this conversation as resolved.
Show resolved Hide resolved
assignUser marked this conversation as resolved.
Show resolved Hide resolved
assignUser marked this conversation as resolved.
Show resolved Hide resolved
- { runs_on: ["self-hosted", "macOS", "arm64", "devops-managed"], arch: "arm64" }
openssl: ['3.0', '1.1']

steps:
{{ macros.github_checkout_arrow(action_v="3")|indent }}
{{ macros.github_change_r_pkg_version(is_fork, '${{ needs.source.outputs.pkg_version }}')|indent }}
- name: Install Deps
if: {{ "${{ !contains(matrix.platform.runs_on, 'macos-10.13') }}" }}
run: |
brew install sccache ninja
brew install openssl@{{ '${{ matrix.openssl }}' }}
- name: Build libarrow
shell: bash
env:
{{ macros.github_set_sccache_envvars()|indent(8) }}
MACOSX_DEPLOYMENT_TARGET: "10.13"
ARROW_S3: ON
ARROW_GCS: ON
ARROW_DEPENDENCY_SOURCE: BUNDLED
CMAKE_GENERATOR: Ninja
LIBARROW_MINIMAL: false
run: |
sccache --start-server
export EXTRA_CMAKE_FLAGS="-DOPENSSL_ROOT_DIR=$(brew --prefix openssl@{{ '${{ matrix.openssl }}' }})"
cd arrow
r/inst/build_arrow_static.sh
- name: Bundle libarrow
shell: bash
env:
PKG_FILE: arrow-{{ '${{ needs.source.outputs.pkg_version }}' }}.zip
VERSION: {{ '${{ needs.source.outputs.pkg_version }}' }}
run: |
cd arrow/r/libarrow/dist
zip -r $PKG_FILE lib/ include/

- name: Upload binary artifact
uses: actions/upload-artifact@v3
with:
name: r-lib__libarrow__bin__darwin-{{ '${{ matrix.platform.arch }}' }}-openssl-{{ '${{ matrix.openssl }}' }}
path: arrow/r/libarrow/dist/arrow-*.zip

linux-cpp:
name: C++ Binary Linux OpenSSL {{ '${{ matrix.openssl }}' }}
runs-on: ubuntu-latest
Expand Down Expand Up @@ -135,7 +185,7 @@ jobs:
path: build/arrow-*.zip

r-packages:
needs: [source, windows-cpp]
needs: [source, windows-cpp, macos-cpp]
name: {{ '${{ matrix.platform.name }} ${{ matrix.r_version.r }}' }}
runs-on: {{ '${{ matrix.platform.runs_on }}' }}
strategy:
Expand Down Expand Up @@ -167,7 +217,7 @@ jobs:

rig system setup-user-lib
rig system add-pak
{{ macros.github_setup_local_r_repo(false, true)|indent }}
{{ macros.github_setup_local_r_repo(false, true, true)|indent }}
- name: Prepare Dependency Installation

shell: bash
Expand All @@ -178,18 +228,19 @@ jobs:
with:
working-directory: 'arrow'
extra-packages: cpp11
- name: Install sccache
if: startsWith(matrix.platform, 'macos')
run: brew install sccache
- name: Set CRAN like openssl
if: contains(matrix.platform.runs_on, 'arm64')
run: |
# The arm64 runners contain openssl 1.1.1t in this path that is always included first so we need to override the
# default setting of the brew --prefix as root dir to avoid version conflicts.
echo "OPENSSL_ROOT_DIR=/opt/R/arm64" >> $GITHUB_ENV
- name: Build Binary
id: build
shell: Rscript {0}
env:
NOT_CRAN: "true" # actions/setup-r sets this implicitly
NOT_CRAN: "false" # actions/setup-r sets this implicitly
assignUser marked this conversation as resolved.
Show resolved Hide resolved
ARROW_R_DEV: "true"
FORCE_AUTOBREW: "true" # this is ignored on windows
# sccache for macos
{{ macros.github_set_sccache_envvars()|indent(8) }}
LIBARROW_BINARY: "true" # has to be set as long as allowlist not updated
run: |
on_windows <- tolower(Sys.info()[["sysname"]]) == "windows"

Expand All @@ -213,8 +264,10 @@ jobs:
INSTALL_opts = INSTALL_opts
)


# Test
library(arrow)
arrow_info()
read_parquet(system.file("v0.7.1.parquet", package = "arrow"))

# encode contrib.url for artifact name
Expand All @@ -233,7 +286,6 @@ jobs:
with:
name: r-pkg{{ '${{ steps.build.outputs.path }}' }}
path: arrow_*

test-linux-binary:
needs: [source, linux-cpp]
name: Test binary {{ '${{ matrix.config.image }}' }}
Expand Down Expand Up @@ -291,7 +343,10 @@ jobs:
with:
name: r-pkg_centos7
path: arrow_*

assignUser marked this conversation as resolved.
Show resolved Hide resolved
test-centos-binary:
# arrow binary package not on ppm currently see #37922
if: false
needs: test-linux-binary
runs-on: ubuntu-latest
container: "rstudio/r-base:4.2-centos7"
Expand All @@ -317,7 +372,8 @@ jobs:
read_parquet(system.file("v0.7.1.parquet", package = "arrow"))
print(arrow_info())

test-source:
#TODO test macos source build?
test-linux-source:
needs: source
name: Test linux source build
runs-on: ubuntu-latest
Expand Down Expand Up @@ -367,7 +423,7 @@ jobs:

upload-binaries:
# Only upload binaries if all tests pass.
needs: [r-packages, test-source, test-linux-binary, test-centos-binary]
needs: [r-packages, test-linux-source, test-linux-binary]
name: Upload artifacts
runs-on: ubuntu-latest
steps:
Expand Down
4 changes: 4 additions & 0 deletions dev/tasks/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,10 @@ tasks:
- r-lib__libarrow__bin__linux-openssl-1.0__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__linux-openssl-1.1__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__linux-openssl-3.0__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__darwin-arm64-openssl-1.1__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__darwin-arm64-openssl-3.0__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__darwin-x86_64-openssl-1.1__arrow-{no_rc_r_version}\.zip
- r-lib__libarrow__bin__darwin-x86_64-openssl-3.0__arrow-{no_rc_r_version}\.zip
assignUser marked this conversation as resolved.
Show resolved Hide resolved
- r-pkg__bin__windows__contrib__4.1__arrow_{no_rc_r_version}\.zip
- r-pkg__bin__windows__contrib__4.2__arrow_{no_rc_r_version}\.zip
- r-pkg__bin__macosx__contrib__4.1__arrow_{no_rc_r_version}\.tgz
Expand Down
Loading
Loading