GH-39584: [R] fallback to source gracefully #39587

jonkeane · 2024-01-12T20:16:08Z

Rationale for this change

Resolves #39584

What changes are included in this PR?

We now only check the checksum after the download succeeded, and try to be quieter about it when we do. We also use bundled boost and lz4 source on macos by default (to avoid system versions of each on cran that seem to have issues)

Are these changes tested?

I submitted a download-malignant (and verbose) build to CRAN's macbuilder and it succeeds.

Are there any user-facing changes?

In principle the macos source build is slightly altered + we have a cleaner path when file downloads fail. But both of these should be relatively non-impactful since most macos users are getting binaries from CRAN. Most importantly it helps us stay on CRAN.

This PR contains a "Critical Fix".

Closes: [R] Macos build cleanups #39584

jonkeane · 2024-01-12T20:17:14Z

r/tools/nixlibs.R

-download_binary <- function(lib) {
-  libfile <- paste0("arrow-", VERSION, ".zip")
-  binary_url <- paste0(arrow_repo, "bin/", lib, "/arrow-", VERSION, ".zip")
-  if (try_download(binary_url, libfile)) {
-      lg("Successfully retrieved libarrow (%s)", lib)
-  } else {
-      lg(
-        "Downloading libarrow failed for version %s (%s)\n    at %s",
-        VERSION, lib, binary_url
-      )
-    libfile <- NULL
-  }


The way that GH is rendering this change is awkward because I factored the checksumming out into a function. It might be easier to review in split diff and not unified

r/tools/nixlibs.R

thisisnic

Looks good to me; changes make sense and additional comments are helpful for clarification, though would like another set of eyes on it before merging.

r/inst/build_arrow_static.sh

assignUser · 2024-01-12T23:04:25Z

@github-actions crossbow submit -g r

assignUser

Thanks, this is great! I have kicked of a round of crossbow jobs but don't anticipate any issues.

r/tools/nixlibs.R

github-actions · 2024-01-12T23:06:55Z

Revision: bacfdbe

Submitted crossbow builds: ursacomputing/crossbow @ actions-f46bb07c50

Task	Status
r-binary-packages
test-fedora-r-clang-sanitizer
test-r-arrow-backwards-compatibility
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-install-local-minsizerel
test-r-library-r-base-latest
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-valgrind
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-gcc-release-latest
test-r-rstudio-r-base-4.1-opensuse153
test-r-rstudio-r-base-4.2-centos7-devtoolset-8
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions
test-ubuntu-r-sanitizer

r/tools/nixlibs.R

jonkeane · 2024-01-15T14:23:06Z

@github-actions crossbow submit r-binary-packages

github-actions · 2024-01-15T14:25:19Z

Revision: 5acf772

Submitted crossbow builds: ursacomputing/crossbow @ actions-82ca8f9466

Task	Status
r-binary-packages

jonkeane · 2024-01-15T14:57:27Z

@github-actions crossbow submit r-binary-packages

github-actions · 2024-01-15T14:59:40Z

Revision: bdf08ec

Submitted crossbow builds: ursacomputing/crossbow @ actions-5b5bb65a33

Task	Status
r-binary-packages

jonkeane · 2024-01-15T15:37:06Z

@github-actions crossbow submit r-binary-packages

github-actions · 2024-01-15T15:39:26Z

Revision: e2e8658

Submitted crossbow builds: ursacomputing/crossbow @ actions-af24ef801f

Task	Status
r-binary-packages

jonkeane · 2024-01-15T16:09:49Z

@github-actions crossbow submit r-binary-packages

github-actions · 2024-01-15T16:12:10Z

Revision: 5d3508c

Submitted crossbow builds: ursacomputing/crossbow @ actions-1c0597fe03

Task	Status
r-binary-packages

r/tools/nixlibs.R

assignUser

Looks good, thanks! I will add the minor comment changes and merge in a bit unless there are objections.

r/tools/nixlibs.R

### Rationale for this change Resolves #39584 ### What changes are included in this PR? We now only check the checksum after the download succeeded, and try to be quieter about it when we do. We also use bundled boost and lz4 source on macos by default (to avoid system versions of each on cran that seem to have issues) ### Are these changes tested? I submitted a download-malignant (and verbose) build to [CRAN's macbuilder](https://mac.r-project.org/macbuilder/results/1705088784-991a5beacf4ec26e/) and it succeeds. ### Are there any user-facing changes? In principle the macos source build is slightly altered + we have a cleaner path when file downloads fail. But both of these should be relatively non-impactful since most macos users are getting binaries from CRAN. Most importantly it helps us stay on CRAN. **This PR contains a "Critical Fix".** * Closes: #39584 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>

conbench-apache-arrow · 2024-01-16T13:24:30Z

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 1df2e4a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

### Rationale for this change Resolves #39584 ### What changes are included in this PR? We now only check the checksum after the download succeeded, and try to be quieter about it when we do. We also use bundled boost and lz4 source on macos by default (to avoid system versions of each on cran that seem to have issues) ### Are these changes tested? I submitted a download-malignant (and verbose) build to [CRAN's macbuilder](https://mac.r-project.org/macbuilder/results/1705088784-991a5beacf4ec26e/) and it succeeds. ### Are there any user-facing changes? In principle the macos source build is slightly altered + we have a cleaner path when file downloads fail. But both of these should be relatively non-impactful since most macos users are getting binaries from CRAN. Most importantly it helps us stay on CRAN. **This PR contains a "Critical Fix".** * Closes: #39584 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>

### Rationale for this change Resolves apache#39584 ### What changes are included in this PR? We now only check the checksum after the download succeeded, and try to be quieter about it when we do. We also use bundled boost and lz4 source on macos by default (to avoid system versions of each on cran that seem to have issues) ### Are these changes tested? I submitted a download-malignant (and verbose) build to [CRAN's macbuilder](https://mac.r-project.org/macbuilder/results/1705088784-991a5beacf4ec26e/) and it succeeds. ### Are there any user-facing changes? In principle the macos source build is slightly altered + we have a cleaner path when file downloads fail. But both of these should be relatively non-impactful since most macos users are getting binaries from CRAN. Most importantly it helps us stay on CRAN. **This PR contains a "Critical Fix".** * Closes: apache#39584 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>

jonkeane requested review from assignUser, paleolimbot and thisisnic as code owners January 12, 2024 20:16

github-actions bot added Component: R awaiting committer review Awaiting committer review labels Jan 12, 2024

jonkeane commented Jan 12, 2024

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jan 12, 2024

jonkeane commented Jan 12, 2024

View reviewed changes

r/tools/nixlibs.R Show resolved Hide resolved

jonkeane commented Jan 12, 2024

View reviewed changes

r/tools/nixlibs.R Outdated Show resolved Hide resolved

thisisnic approved these changes Jan 12, 2024

View reviewed changes

github-actions bot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting merge Awaiting merge labels Jan 12, 2024

kou reviewed Jan 12, 2024

View reviewed changes

r/inst/build_arrow_static.sh Outdated Show resolved Hide resolved

assignUser approved these changes Jan 12, 2024

View reviewed changes

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Show resolved Hide resolved

assignUser reviewed Jan 12, 2024

View reviewed changes

r/tools/nixlibs.R Outdated Show resolved Hide resolved

nealrichardson reviewed Jan 13, 2024

View reviewed changes

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Jan 14, 2024

jonkeane added 3 commits January 14, 2024 11:08

Don't fail on checksum, bundle boost source

50e2924

fall back to bundled boost and lz4 on macos

e24fcd3

PR comments

97c3787

jonkeane force-pushed the macos-build-fixes branch from 24e1173 to 97c3787 Compare January 14, 2024 17:08

github-actions bot removed the awaiting changes Awaiting changes label Jan 14, 2024

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 15, 2024

oops, that's not how system2 works

bdf08ec

another way

e2e8658

One last time?

5d3508c

remove the extra edit

c530814

jonkeane commented Jan 15, 2024

View reviewed changes

r/tools/nixlibs.R Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jan 15, 2024

assignUser approved these changes Jan 16, 2024

View reviewed changes

r/tools/nixlibs.R Outdated Show resolved Hide resolved

r/tools/nixlibs.R Outdated Show resolved Hide resolved

Update comments

79d5b94

assignUser merged commit 1df2e4a into apache:main Jan 16, 2024
8 checks passed

assignUser removed the awaiting changes Awaiting changes label Jan 16, 2024

github-actions bot added the awaiting review Awaiting review label Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-39584: [R] fallback to source gracefully #39587

GH-39584: [R] fallback to source gracefully #39587

jonkeane commented Jan 12, 2024 •

edited by github-actions bot

jonkeane Jan 12, 2024

thisisnic left a comment

assignUser commented Jan 12, 2024

assignUser left a comment

github-actions bot commented Jan 12, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

assignUser left a comment

conbench-apache-arrow bot commented Jan 16, 2024

GH-39584: [R] fallback to source gracefully #39587

GH-39584: [R] fallback to source gracefully #39587

Conversation

jonkeane commented Jan 12, 2024 • edited by github-actions bot

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

jonkeane Jan 12, 2024

Choose a reason for hiding this comment

thisisnic left a comment

Choose a reason for hiding this comment

assignUser commented Jan 12, 2024

assignUser left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 12, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

jonkeane commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

assignUser left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Jan 16, 2024

jonkeane commented Jan 12, 2024 •

edited by github-actions bot