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-39697: [R] Source build should check if offline #39699

Merged
merged 5 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion dev/tasks/r/github.linux.offline.build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ jobs:
shell: Rscript {0}
- name: Install
env:
TEST_OFFLINE_BUILD: true
ARROW_OFFLINE_BUILD: true
LIBARROW_MINIMAL: false
{{ macros.github_set_sccache_envvars()|indent(8)}}
run: |
Expand Down
2 changes: 1 addition & 1 deletion dev/tasks/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1279,7 +1279,7 @@ tasks:
r_org: library
r_image: r-base
r_tag: latest
flags: '-e TEST_OFFLINE_BUILD=true'
flags: '-e ARROW_OFFLINE_BUILD=true'

test-r-dev-duckdb:
ci: github
Expand Down
2 changes: 1 addition & 1 deletion r/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

## Minor improvements and fixes

* Don't download cmake when TEST_OFFLINE_BUILD=true and update `SystemRequirements` (#39602).
* Don't download cmake when ARROW_OFFLINE_BUILD=true and update `SystemRequirements` (#39602).
* Fallback to source build gracefully if binary download fails (#39587).
* An error is now thrown instead of warning and pulling the data into R when any
of `sub`, `gsub`, `stringr::str_replace`, `stringr::str_replace_all` are
Expand Down
2 changes: 1 addition & 1 deletion r/configure
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ FORCE_BUNDLED_BUILD=`echo $FORCE_BUNDLED_BUILD | tr '[:upper:]' '[:lower:]'`
ARROW_USE_PKG_CONFIG=`echo $ARROW_USE_PKG_CONFIG | tr '[:upper:]' '[:lower:]'`
# Just used in testing: whether or not it is ok to download dependencies (in the
# bundled build)
TEST_OFFLINE_BUILD=`echo $TEST_OFFLINE_BUILD | tr '[:upper:]' '[:lower:]'`
ARROW_OFFLINE_BUILD=`echo $ARROW_OFFLINE_BUILD | tr '[:upper:]' '[:lower:]'`

VERSION=`grep '^Version' DESCRIPTION | sed s/Version:\ //`
UNAME=`uname -s`
Expand Down
36 changes: 24 additions & 12 deletions r/tools/nixlibs.R
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ validate_checksum <- function(binary_url, libfile, hush = quietly) {
# The warnings from system2 if it fails pop up later in the log and thus are
# more confusing than they are helpful (so we suppress them)
checksum_ok <- suppressWarnings(system2(
"shasum",
args = c("--status", "-a", "512", "-c", checksum_file),
stdout = ifelse(quietly, FALSE, ""),
stderr = ifelse(quietly, FALSE, "")
)) == 0
"shasum",
args = c("--status", "-a", "512", "-c", checksum_file),
stdout = ifelse(quietly, FALSE, ""),
stderr = ifelse(quietly, FALSE, "")
)) == 0
nealrichardson marked this conversation as resolved.
Show resolved Hide resolved

if (!checksum_ok) {
checksum_ok <- suppressWarnings(system2(
Expand Down Expand Up @@ -565,8 +565,8 @@ build_libarrow <- function(src_dir, dst_dir) {
env_var_list <- c(env_var_list, ARROW_DEPENDENCY_SOURCE = "BUNDLED")
}

# On macOS, if not otherwise set, let's override Boost_SOURCE to be bundled
# Necessary due to #39590 for CRAN
# On macOS, if not otherwise set, let's override Boost_SOURCE to be bundled
# Necessary due to #39590 for CRAN
nealrichardson marked this conversation as resolved.
Show resolved Hide resolved
if (on_macos) {
# Using lowercase (e.g. Boost_SOURCE) to match the cmake args we use already.
deps_to_bundle <- c("Boost", "lz4")
Expand Down Expand Up @@ -912,8 +912,6 @@ if (is_release) {
VERSION <- VERSION[1, 1:3]
arrow_repo <- paste0(getOption("arrow.repo", sprintf("https://apache.jfrog.io/artifactory/arrow/r/%s", VERSION)), "/libarrow/")
} else {
# Don't override explictily set NOT_CRAN env var, as it is used in CI.
not_cran <- !env_is("NOT_CRAN", "false")
assignUser marked this conversation as resolved.
Show resolved Hide resolved
arrow_repo <- paste0(getOption("arrow.dev_repo", "https://nightlies.apache.org/arrow/r"), "/libarrow/")
}

Expand All @@ -926,8 +924,7 @@ if (!is_release && !test_mode) {
options(.arrow.cleanup = character())
on.exit(unlink(getOption(".arrow.cleanup"), recursive = TRUE), add = TRUE)

# enable full featured builds for macOS in case of CRAN source builds.
if (not_cran || on_macos) {
assignUser marked this conversation as resolved.
Show resolved Hide resolved
if (not_cran) {
# Set more eager defaults
if (env_is("LIBARROW_BINARY", "")) {
Sys.setenv(LIBARROW_BINARY = "true")
Expand All @@ -943,7 +940,22 @@ if (not_cran || on_macos) {
build_ok <- !env_is("LIBARROW_BUILD", "false")

# Check if we're authorized to download
download_ok <- !test_mode && !env_is("TEST_OFFLINE_BUILD", "true")
download_ok <- !test_mode && !env_is("ARROW_OFFLINE_BUILD", "true")
if (!download_ok) {
lg("Dependency downloading disabled. Unset ARROW_OFFLINE_BUILD to enable", .indent = "***")
}
# If not forbidden from downloading, check if we are offline and turn off downloading.
# The default libarrow source build will download its source dependencies and fail
# if they can't be retrieved.
# But, don't do this if the user has requested a binary or a non-minimal build:
# we should error rather than silently succeeding with a minimal build.
if (download_ok && Sys.getenv("LIBARROW_BINARY") %in% c("false", "") && !env_is("LIBARROW_MINIMAL", "false")) {
download_ok <- try_download("https://apache.jfrog.io/artifactory/arrow/r/", tempfile())
if (!download_ok) {
lg("Network connection not available", .indent = "***")
}
}

nealrichardson marked this conversation as resolved.
Show resolved Hide resolved
download_libarrow_ok <- download_ok && !env_is("LIBARROW_DOWNLOAD", "false")

# This "tools/thirdparty_dependencies" path, within the tar file, might exist if
Expand Down
2 changes: 1 addition & 1 deletion r/vignettes/developers/setup.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ withr::with_makevars(list(CPPFLAGS = "", LDFLAGS = ""), remotes::install_github(
* See the user-facing [article on installation](../install.html) for a large number of
environment variables that determine how the build works and what features
get built.
* `TEST_OFFLINE_BUILD`: When set to `true`, the build script will not download
* `ARROW_OFFLINE_BUILD`: When set to `true`, the build script will not download
prebuilt the C++ library binary or, if needed, `cmake`.
It will turn off any features that require a download, unless they're available
in `ARROW_THIRDPARTY_DEPENDENCY_DIR` or the `tools/thirdparty_download/` subfolder.
Expand Down
Loading