From 351f384dfcb207c687f8dea3db4dfc9fb8e35adb Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 4 Dec 2020 08:21:27 -0800 Subject: [PATCH 01/14] Add crossbot job to test package on older R versions --- dev/tasks/r/github.linux.versions.yml | 72 +++++++++++++++++++++++++++ dev/tasks/tasks.yml | 6 +++ 2 files changed, 78 insertions(+) create mode 100644 dev/tasks/r/github.linux.versions.yml diff --git a/dev/tasks/r/github.linux.versions.yml b/dev/tasks/r/github.linux.versions.yml new file mode 100644 index 00000000000..2e72f24d250 --- /dev/null +++ b/dev/tasks/r/github.linux.versions.yml @@ -0,0 +1,72 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# NOTE: must set "Crossbow" as name to have the badge links working in the +# github comment reports! +name: Crossbow + +on: + push: + branches: + - "*-github-*" + +jobs: + r-versions: + name: "rstudio/r-base:{{ MATRIX }}-bionic" + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # See https://hub.docker.com/r/rstudio/r-base + r_version: + # We test devel, release, and oldrel in regular CI. + # This is for older versions + - "3.1" + - "3.2" + - "3.3" + - "3.4" + - "3.5" + env: + R_ORG: "rstudio" + R_IMAGE: "r-base" + R_TAG: "{{ MATRIX }}-bionic" + ARROW_R_DEV: "FALSE" + steps: + - name: Checkout Arrow + run: | + git clone --no-checkout {{ arrow.remote }} arrow + git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }} + git -C arrow checkout FETCH_HEAD + git -C arrow submodule update --init --recursive + - name: Free Up Disk Space + shell: bash + run: arrow/ci/scripts/util_cleanup.sh + - name: Fetch Submodules and Tags + shell: bash + run: cd arrow && ci/scripts/util_checkout.sh + - name: Docker Pull + shell: bash + run: cd arrow && docker-compose pull --ignore-pull-failures r + - name: Docker Build + shell: bash + run: cd arrow && docker-compose build r + - name: Docker Run + shell: bash + run: cd arrow && docker-compose run r + - name: Dump install logs + run: cat arrow/r/check/arrow.Rcheck/00install.out + if: always() diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml index 569e59f80dd..10547820b56 100644 --- a/dev/tasks/tasks.yml +++ b/dev/tasks/tasks.yml @@ -1753,6 +1753,12 @@ tasks: params: MATRIX: "${{ matrix.r_image }}" + test-r-versions: + ci: github + template: r/github.linux.versions.yml + params: + MATRIX: "${{ matrix.r_version }}" + test-r-rhub-ubuntu-gcc-release: ci: azure template: r/azure.linux.yml From 2810b1f7e55de777dcac2d682c7f4562824b5fbc Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 4 Dec 2020 08:39:58 -0800 Subject: [PATCH 02/14] Try test script via stdin --- ci/scripts/r_test.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/scripts/r_test.sh b/ci/scripts/r_test.sh index 2aabf21818b..0a15f663ae9 100755 --- a/ci/scripts/r_test.sh +++ b/ci/scripts/r_test.sh @@ -60,7 +60,7 @@ fi # Make sure we aren't writing to the home dir (CRAN _hates_ this but there is no official check) BEFORE=$(ls -alh ~/) -${R_BIN} -e "as_cran <- !identical(tolower(Sys.getenv('NOT_CRAN')), 'true') +SCRIPT="as_cran <- !identical(tolower(Sys.getenv('NOT_CRAN')), 'true') if (as_cran) { rcmdcheck::rcmdcheck(args = c('--as-cran', '--run-donttest'), error_on = 'warning', check_dir = 'check') } else { @@ -73,6 +73,7 @@ ${R_BIN} -e "as_cran <- !identical(tolower(Sys.getenv('NOT_CRAN')), 'true') } rcmdcheck::rcmdcheck(build_args = '--no-build-vignettes', args = c('--no-manual', '--ignore-vignettes', '--run-donttest'), error_on = 'warning', check_dir = 'check') }" +${R_BIN} < $SCRIPT AFTER=$(ls -alh ~/) if [ "$NOT_CRAN" != "true" ] && [ "$BEFORE" != "$AFTER" ]; then From a4b39a3d9360f94c0c5647a14354ee16a4bdad73 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 4 Dec 2020 08:49:51 -0800 Subject: [PATCH 03/14] Iterate --- ci/scripts/r_test.sh | 2 +- dev/tasks/r/github.linux.versions.yml | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ci/scripts/r_test.sh b/ci/scripts/r_test.sh index 0a15f663ae9..b33dfd2c97c 100755 --- a/ci/scripts/r_test.sh +++ b/ci/scripts/r_test.sh @@ -73,7 +73,7 @@ SCRIPT="as_cran <- !identical(tolower(Sys.getenv('NOT_CRAN')), 'true') } rcmdcheck::rcmdcheck(build_args = '--no-build-vignettes', args = c('--no-manual', '--ignore-vignettes', '--run-donttest'), error_on = 'warning', check_dir = 'check') }" -${R_BIN} < $SCRIPT +echo "$SCRIPT" | ${R_BIN} AFTER=$(ls -alh ~/) if [ "$NOT_CRAN" != "true" ] && [ "$BEFORE" != "$AFTER" ]; then diff --git a/dev/tasks/r/github.linux.versions.yml b/dev/tasks/r/github.linux.versions.yml index 2e72f24d250..37606ff8dd3 100644 --- a/dev/tasks/r/github.linux.versions.yml +++ b/dev/tasks/r/github.linux.versions.yml @@ -35,8 +35,9 @@ jobs: r_version: # We test devel, release, and oldrel in regular CI. # This is for older versions - - "3.1" - - "3.2" + # - "3.1" + # - "3.2" + # rcmdcheck package depends on digest, which depends on R >= 3.3 - "3.3" - "3.4" - "3.5" From 0686ef695f13d6dc92424f80f67a7163816a14c4 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 4 Dec 2020 09:00:36 -0800 Subject: [PATCH 04/14] pedantry --- ci/scripts/r_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/r_test.sh b/ci/scripts/r_test.sh index b33dfd2c97c..151e71b8a7c 100755 --- a/ci/scripts/r_test.sh +++ b/ci/scripts/r_test.sh @@ -73,7 +73,7 @@ SCRIPT="as_cran <- !identical(tolower(Sys.getenv('NOT_CRAN')), 'true') } rcmdcheck::rcmdcheck(build_args = '--no-build-vignettes', args = c('--no-manual', '--ignore-vignettes', '--run-donttest'), error_on = 'warning', check_dir = 'check') }" -echo "$SCRIPT" | ${R_BIN} +echo "$SCRIPT" | ${R_BIN} --no-save AFTER=$(ls -alh ~/) if [ "$NOT_CRAN" != "true" ] && [ "$BEFORE" != "$AFTER" ]; then From 9418f8f295ee71fa753ed288a0eaaaafec5ed712 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 08:09:15 -0800 Subject: [PATCH 05/14] DATAPTR fix --- r/src/arrow_types.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/r/src/arrow_types.h b/r/src/arrow_types.h index e6c2362017a..909ccfb217a 100644 --- a/r/src/arrow_types.h +++ b/r/src/arrow_types.h @@ -53,6 +53,17 @@ std::shared_ptr Array__from_vector(SEXP x, SEXP type); std::shared_ptr RecordBatch__from_arrays(SEXP, SEXP); arrow::MemoryPool* gc_memory_pool(); +#if (R_VERSION < R_Version(3, 5, 0)) +#define LOGICAL_RO(x) ((const int*)LOGICAL(x)) +#define INTEGER_RO(x) ((const int*)INTEGER(x)) +#define REAL_RO(x) ((const double*)REAL(x)) +#define COMPLEX_RO(x) ((const Rcomplex*)COMPLEX(x)) +#define STRING_PTR_RO(x) ((const SEXP*)STRING_PTR(x)) +#define RAW_RO(x) ((const Rbyte*)RAW(x)) +#define DATAPTR_RO(x) ((const void*)STRING_PTR(x)) +#define DATAPTR(x) (void*)STRING_PTR(x) +#endif + namespace arrow { static inline void StopIfNotOk(const Status& status) { From 30a91a6e04058ea8fce66c26de58b328acac2c96 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Fri, 8 Jan 2021 11:14:52 +0100 Subject: [PATCH 06/14] include Rdynload, ARROW-10803 --- r/src/imports.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/r/src/imports.cpp b/r/src/imports.cpp index a85aa23e838..e50dc5b0fd0 100644 --- a/r/src/imports.cpp +++ b/r/src/imports.cpp @@ -16,6 +16,7 @@ // under the License. #include +#include // for R_GetCCallable namespace vctrs { struct vctrs_api_ptrs_t { From 36d0efc61c4a6171ef6c72940264e4456c70a1a4 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 08:13:02 -0800 Subject: [PATCH 07/14] rlang and vctrs depend on R >= 3.3 so we must too --- dev/tasks/r/github.linux.versions.yml | 4 +--- r/DESCRIPTION | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dev/tasks/r/github.linux.versions.yml b/dev/tasks/r/github.linux.versions.yml index 37606ff8dd3..f4954ec99c8 100644 --- a/dev/tasks/r/github.linux.versions.yml +++ b/dev/tasks/r/github.linux.versions.yml @@ -35,9 +35,7 @@ jobs: r_version: # We test devel, release, and oldrel in regular CI. # This is for older versions - # - "3.1" - # - "3.2" - # rcmdcheck package depends on digest, which depends on R >= 3.3 + # rlang and vctrs depend on R >= 3.3 - "3.3" - "3.4" - "3.5" diff --git a/r/DESCRIPTION b/r/DESCRIPTION index 06d50d44806..f87ea4e563d 100644 --- a/r/DESCRIPTION +++ b/r/DESCRIPTION @@ -14,7 +14,7 @@ Description: 'Apache' 'Arrow' is a cross-language language-independent columnar memory format for flat and hierarchical data, organized for efficient analytic operations on modern hardware. This package provides an interface to the 'Arrow C++' library. -Depends: R (>= 3.1) +Depends: R (>= 3.3) License: Apache License (>= 2.0) URL: https://github.com/apache/arrow/, https://arrow.apache.org/docs/r/ BugReports: https://issues.apache.org/jira/projects/ARROW/issues From d9a838cb35d148b0bc09d0c3db0a00751232a706 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 10:00:50 -0800 Subject: [PATCH 08/14] run as not cran --- dev/tasks/r/github.linux.versions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tasks/r/github.linux.versions.yml b/dev/tasks/r/github.linux.versions.yml index f4954ec99c8..9ddeb1e9401 100644 --- a/dev/tasks/r/github.linux.versions.yml +++ b/dev/tasks/r/github.linux.versions.yml @@ -43,7 +43,7 @@ jobs: R_ORG: "rstudio" R_IMAGE: "r-base" R_TAG: "{{ MATRIX }}-bionic" - ARROW_R_DEV: "FALSE" + ARROW_R_DEV: "TRUE" steps: - name: Checkout Arrow run: | From f853a07dec7e25159bfaea46dd4ff7580197f412 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 13:28:47 -0800 Subject: [PATCH 09/14] download.file.method on R 3.3 --- r/tools/linuxlibs.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/r/tools/linuxlibs.R b/r/tools/linuxlibs.R index 9068a754bc7..c37cd2b2829 100644 --- a/r/tools/linuxlibs.R +++ b/r/tools/linuxlibs.R @@ -21,6 +21,11 @@ dst_dir <- paste0("libarrow/arrow-", VERSION) arrow_repo <- "https://dl.bintray.com/ursalabs/arrow-r/libarrow/" +if (getRversion() < 3.4 && is.null(getOption("download.file.method"))) { + # default method doesn't work on R 3.3; default changed to libcurl in 3.4 + options(download.file.method = "libcurl") +} + options(.arrow.cleanup = character()) # To collect dirs to rm on exit on.exit(unlink(getOption(".arrow.cleanup"))) From 8ea76e0a7a7f2009d1518a0295c91105f35e7d64 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 14:56:01 -0800 Subject: [PATCH 10/14] Try wget download method --- r/tools/linuxlibs.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/tools/linuxlibs.R b/r/tools/linuxlibs.R index c37cd2b2829..94e53806b76 100644 --- a/r/tools/linuxlibs.R +++ b/r/tools/linuxlibs.R @@ -22,8 +22,8 @@ dst_dir <- paste0("libarrow/arrow-", VERSION) arrow_repo <- "https://dl.bintray.com/ursalabs/arrow-r/libarrow/" if (getRversion() < 3.4 && is.null(getOption("download.file.method"))) { - # default method doesn't work on R 3.3; default changed to libcurl in 3.4 - options(download.file.method = "libcurl") + # default method doesn't work on R 3.3, nor does libcurl + options(download.file.method = "wget") } options(.arrow.cleanup = character()) # To collect dirs to rm on exit From d5c884f8fb7828874a6554a48eeae990abb42e7b Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 15:27:21 -0800 Subject: [PATCH 11/14] Fix CXX11 var names on R 3.3; also bump default cmake version while we're here --- r/tools/linuxlibs.R | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/r/tools/linuxlibs.R b/r/tools/linuxlibs.R index 94e53806b76..c418e355922 100644 --- a/r/tools/linuxlibs.R +++ b/r/tools/linuxlibs.R @@ -301,8 +301,12 @@ build_libarrow <- function(src_dir, dst_dir) { options(.arrow.cleanup = c(getOption(".arrow.cleanup"), build_dir)) R_CMD_config <- function(var) { - # cf. tools::Rcmd, introduced R 3.3 - system2(file.path(R.home("bin"), "R"), c("CMD", "config", var), stdout = TRUE) + if (getRversion() < 3.4) { + # var names were called CXX1X instead of CXX11 + var <- sub("^CXX11", "CXX1X", var) + } + # tools::Rcmd introduced R 3.3 + tools::Rcmd(paste("config", var), stdout = TRUE) } env_var_list <- c( SOURCE_DIR = src_dir, @@ -339,7 +343,7 @@ ensure_cmake <- function() { if (is.null(cmake)) { # If not found, download it cat("**** cmake\n") - CMAKE_VERSION <- Sys.getenv("CMAKE_VERSION", "3.18.1") + CMAKE_VERSION <- Sys.getenv("CMAKE_VERSION", "3.19.2") cmake_binary_url <- paste0( "https://github.com/Kitware/CMake/releases/download/v", CMAKE_VERSION, "/cmake-", CMAKE_VERSION, "-Linux-x86_64.tar.gz" From 56c61370ddeb1b30ae7c26fb26efb13d52c96e0f Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 16:07:42 -0800 Subject: [PATCH 12/14] Hacks to configure for R 3.3 --- r/configure | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/r/configure b/r/configure index c50a7586dc7..77f4451a8f4 100755 --- a/r/configure +++ b/r/configure @@ -151,12 +151,6 @@ else fi fi -# Find compiler -CXXCPP="`${R_HOME}/bin/R CMD config CXX11` -E" -CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX11FLAGS` -CXX11STD=`"${R_HOME}"/bin/R CMD config CXX11STD` -CPPFLAGS=`"${R_HOME}"/bin/R CMD config CPPFLAGS` - # If libarrow uses the old GLIBCXX ABI, so we have to use it too if [ "$ARROW_USE_OLD_CXXABI" ]; then PKG_CFLAGS="$PKG_CFLAGS -D_GLIBCXX_USE_CXX11_ABI=0" @@ -168,6 +162,17 @@ if [ "$ARROW_R_CXXFLAGS" ]; then fi # Test configuration +CXXCPP="`${R_HOME}/bin/R CMD config CXX11` -E" +if [ $? -eq 0 ]; then + CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX11FLAGS` + CXX11STD=`"${R_HOME}"/bin/R CMD config CXX11STD` +else + # For compatibility with R < 3.4, when these were called CXX1X + CXXCPP="`${R_HOME}/bin/R CMD config CXX1X` -E" + CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX1XFLAGS` + CXX11STD=`"${R_HOME}"/bin/R CMD config CXX1XSTD` +fi +CPPFLAGS=`"${R_HOME}"/bin/R CMD config CPPFLAGS` TEST_CMD="${CXXCPP} ${CPPFLAGS} ${PKG_CFLAGS} ${CXX11FLAGS} ${CXX11STD} -xc++ -" echo "#include $PKG_TEST_HEADER" | ${TEST_CMD} >/dev/null 2>&1 From 5d8fa78253984ffa2b2b39da5a9d2b399660015d Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 8 Jan 2021 16:42:42 -0800 Subject: [PATCH 13/14] Lint --- r/src/imports.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/src/imports.cpp b/r/src/imports.cpp index e50dc5b0fd0..5c77d6cb5b0 100644 --- a/r/src/imports.cpp +++ b/r/src/imports.cpp @@ -15,8 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include // for R_GetCCallable #include -#include // for R_GetCCallable namespace vctrs { struct vctrs_api_ptrs_t { From 4d4656cb9a1185e8c7a8226fc49d0c3f3cc3b7d8 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Mon, 11 Jan 2021 11:24:19 -0800 Subject: [PATCH 14/14] Add comment to configure --- r/configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/r/configure b/r/configure index 77f4451a8f4..18aee86e17d 100755 --- a/r/configure +++ b/r/configure @@ -164,6 +164,8 @@ fi # Test configuration CXXCPP="`${R_HOME}/bin/R CMD config CXX11` -E" if [ $? -eq 0 ]; then + # Great, CXX11 exists for this version of R (current); + # now let's set the other two variables CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX11FLAGS` CXX11STD=`"${R_HOME}"/bin/R CMD config CXX11STD` else