jonkeane commented on code in PR #39587:
URL: https://github.com/apache/arrow/pull/39587#discussion_r1452608780
##########
r/tools/nixlibs.R:
##########
@@ -116,33 +105,66 @@ download_binary <- function(lib) {
# validate binary checksum for CRAN release only
if (!skip_checksum && dir.exists(checksum_path) && is_release ||
enforce_checksum) {
+ # Munge the path to the correct sha file which we include during the
+ # release process
checksum_file <- sub(".+/bin/(.+\\.zip)", "\\1\\.sha512", binary_url)
checksum_file <- file.path(checksum_path, checksum_file)
- checksum_cmd <- "shasum"
- checksum_args <- c("--status", "-a", "512", "-c", checksum_file)
-
- # shasum is not available on all linux versions
- status_shasum <- try(
- suppressWarnings(
- system2("shasum", args = c("--help"), stdout = FALSE, stderr = FALSE)
- ),
- silent = TRUE
- )
- if (inherits(status_shasum, "try-error") || is.integer(status_shasum) &&
status_shasum != 0) {
- checksum_cmd <- "sha512sum"
- checksum_args <- c("--status", "-c", checksum_file)
+ # Try `shasum`, and if that doesn't work, fall back to `sha512sum` if not
found
+ # system2 doesn't generate an R error, so we can't use a tryCatch to
+ # move from shasum to sha512sum.
+ # 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
+
+ if (!checksum_ok) {
+ checksum_ok <- suppressWarnings(system2(
+ "sha512sum",
+ args = c("--status", "-c", checksum_file),
+ stdout = ifelse(quietly, FALSE, ""),
+ stderr = ifelse(quietly, FALSE, "")
+ )) == 0
}
Review Comment:
OOPH this was fun to deal with. I tried a bunch of things (after forgetting
how system2 handles errors that it gets from the shell) and this is the most
clean way I could get it working given the oddity of shasum _existing_ when
checked, but not functional on some platforms (i.e. windows).
I tried to get as much detail as I could about the various oddities in the
comment for future-us, but LMK if there's more I should add here.
The binary job is clean 🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
