GH-37941: [R][CI][Release] Add checksum verification for pre-compiled binaries by assignUser · Pull Request #38115 · apache/arrow

assignUser · 2023-10-07T01:02:54Z

Rationale for this change

This change is to restore parity with the previous solution on macOS (brew does cs validation) and improve security for windows and linux. This also align with CRAN policy.

What changes are included in this PR?

This PR adds a script that can be run after the arrow release (once all files have been pushed to the artifactory) before the CRAN submission to download the checksum files for the pre-compiled binaries which are already added through the usual release. *libs.R have been extended to use these checksum files to validate the downloaded binaries.

Are these changes tested?

The r-binary-packages nightlies generate checksums and use them when building binary packages, this way the code path is tested. They do not modify the actual src package though.

Are there any user-facing changes?

no (outside of log messages)

Closes: [R][Release] Add checksum verification to pre-compiled binaries #37941

assignUser · 2023-10-07T01:03:16Z

@github-actions crossbow submit r-binary-packages

assignUser · 2023-10-07T02:22:30Z

@github-actions crossbow submit r-binary-packages

github-actions · 2023-10-08T01:49:45Z

Revision: 4dae43c

Submitted crossbow builds: ursacomputing/crossbow @ actions-88d6d2b34e

Task	Status
conda-linux-aarch64-cpu-r42
conda-linux-aarch64-cpu-r43
conda-linux-x64-cpu-r42
conda-linux-x64-cpu-r43
conda-osx-arm64-cpu-r42
conda-osx-arm64-cpu-r43
conda-osx-x64-cpu-r42
conda-osx-x64-cpu-r43
conda-win-x64-cpu-r41
homebrew-r-autobrew
homebrew-r-brew
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

paleolimbot

Two small comments...thank you for taking this on!

paleolimbot · 2023-10-10T20:11:32Z

Also, it is probably worth rebasing to clear up the CI.

assignUser · 2023-10-11T01:13:18Z

Also, it is probably worth rebasing to clear up the CI.

I merged to keep the crossbow job shas valid. I will run another round of validation but then this should be merge ready imo.

assignUser · 2023-10-11T01:14:33Z

@github-actions crossbow submit -g r

github-actions · 2023-10-11T01:18:06Z

Revision: dd292b6

Submitted crossbow builds: ursacomputing/crossbow @ actions-d4c5399398

Task	Status
conda-linux-aarch64-cpu-r42
conda-linux-aarch64-cpu-r43
conda-linux-x64-cpu-r42
conda-linux-x64-cpu-r43
conda-osx-arm64-cpu-r42
conda-osx-arm64-cpu-r43
conda-osx-x64-cpu-r42
conda-osx-x64-cpu-r43
conda-win-x64-cpu-r41
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

paleolimbot

Pending green CI, naturally. Thank you!

nealrichardson

A few notes, apologies for the late review here @assignUser

nealrichardson · 2023-10-11T12:51:59Z

+ checksum_cmd <- "shasum"
+ checksum_args <- c("--status", "-a", "512", "-c", checksum_file)
+
+ # shasum is not available on all linux versions


Use sys.which() to see if it's present?

Ah nice 👍

conbench-apache-arrow · 2023-10-14T01:53:37Z

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

There were no benchmark performance regressions. 🎉

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

…mpiled binaries (apache#38115) ### Rationale for this change This change is to restore parity with the previous solution on macOS (brew does cs validation) and improve security for windows and linux. This also align with CRAN policy. ### What changes are included in this PR? This PR adds a script that can be run after the arrow release (once all files have been pushed to the artifactory) before the CRAN submission to download the checksum files for the pre-compiled binaries which are already added through the usual release. *libs.R have been extended to use these checksum files to validate the downloaded binaries. ### Are these changes tested? The r-binary-packages nightlies generate checksums and use them when building binary packages, this way the code path is tested. They do not modify the actual src package though. ### Are there any user-facing changes? no (outside of log messages) * Closes: apache#37941 Authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: Nic Crane <thisisnic@gmail.com>

assignUser added 9 commits October 6, 2023 05:56

add script to fetch checksum files from artifactory
8974e19

remove rwinlib
357f62a

validate windows binary checksum
b80ee6d

fix error with lineendings on windows
f0cfd43

use versioned filename to match checksum file
eceadb4

add checksum validation for nixlibs.R
f1219f2

use same logic for checksum for *libs.R
cc0d876

allow setting checksum path via envvar
0beda0e

add checksum validation to nightly builds
16ae261

assignUser requested review from kou, paleolimbot, raulcd and thisisnic as code owners October 7, 2023 01:02

github-actionsBot added Component: R awaiting review Awaiting review labels Oct 7, 2023

This comment was marked as outdated.
Sign in to view

add sudo to overcome docker permission
9fbd82a

This comment was marked as outdated.
Sign in to view

fix typo
271574c

This comment was marked as outdated.
Sign in to view

assignUser added 3 commits October 7, 2023 04:04

chown docker build files to prevent permission issues
b4e324f

use absolute checksum path
0428ad9

fix log message and checksum command
28ce9e7

This comment was marked as outdated.
Sign in to view

assignUser added 4 commits October 7, 2023 04:35

chown needs sudo
5e01e62

validate checksum with linux binary
ed758f6

fix log msg
6ef1495

fix file name
3aad002

assignUser mentioned this pull request Oct 9, 2023
GH-38079: [R] Allow override of version used to download libarrow static library #38080
Closed

paleolimbot approved these changes Oct 10, 2023
View reviewed changes

Comment threadr/tools/nixlibs.R Outdated
Comment threadr/.gitignore

github-actionsBot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 10, 2023

assignUser added 3 commits October 11, 2023 03:05

allow checksum bypass via envvar
7a6c452

add checksum download to release checklist
4ff5d4c

Merge branch 'main' into add-checksum-rbinaries
77fe890

github-actionsBot added the Component: Documentation label Oct 11, 2023

assignUser added 2 commits October 11, 2023 03:11

fix name of update script
4c8511e

update usage
dd292b6

paleolimbot approved these changes Oct 11, 2023
View reviewed changes

Comment threadr/tools/nixlibs.R

github-actionsBot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Oct 11, 2023

thisisnic merged commit b20e0ae into apache:mainOct 11, 2023

thisisnic removed the awaiting changes Awaiting changes label Oct 11, 2023

nealrichardson reviewed Oct 11, 2023
View reviewed changes

github-actionsBot added the awaiting committer review Awaiting committer review label Oct 11, 2023

assignUser mentioned this pull request Oct 11, 2023
[R][Release] Add update-checksums.R to r/Makefile #38205
Closed

github-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Oct 11, 2023

assignUser mentioned this pull request Oct 24, 2023
[R] Update command search for checksum check in nixlibs.R #38429
Open

Uh oh!

Conversation

assignUser commented Oct 7, 2023• edited by github-actions BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

assignUser commented Oct 7, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

assignUser commented Oct 7, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

github-actionsBot commented Oct 8, 2023

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paleolimbot commented Oct 10, 2023

Uh oh!

assignUser commented Oct 11, 2023

Uh oh!

assignUser commented Oct 11, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Oct 11, 2023

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nealrichardson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nealrichardsonOct 11, 2023

Choose a reason for hiding this comment

Uh oh!

assignUserOct 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

conbench-apache-arrowBot commented Oct 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

assignUser commented Oct 7, 2023•
edited by github-actions Bot
Loading

assignUser commented Oct 7, 2023•
edited
Loading

assignUser commented Oct 7, 2023•
edited
Loading

assignUser commented Oct 11, 2023•
edited
Loading