jonkeane commented on code in PR #39587:
URL: https://github.com/apache/arrow/pull/39587#discussion_r1451559594


##########
r/tools/nixlibs.R:
##########
@@ -96,53 +96,76 @@ try_download <- function(from_url, to_file, hush = quietly) 
{
   !inherits(status, "try-error") && status == 0
 }
 
-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
-  }
+try_checksum <- function(binary_url, libfile, hush = quietly) {
   # Explicitly setting the env var to "false" will skip checksum validation
   # e.g. in case the included checksums are stale.
   skip_checksum <- env_is("ARROW_R_ENFORCE_CHECKSUM", "false")
   enforce_checksum <- env_is("ARROW_R_ENFORCE_CHECKSUM", "true")
   checksum_path <- Sys.getenv("ARROW_R_CHECKSUM_PATH", "tools/checksums")
   # validate binary checksum for CRAN release only
-  if (!skip_checksum && dir.exists(checksum_path) && is_release ||
-    enforce_checksum) {
-    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
-    )
+  # we do this in a try so that if it fails for any reason, we don't pollute 
the log

Review Comment:
   Like I said in a comment below: I'll clean this up and I'll also pull this 
level of nesting out too while I'm at it. 
   
   The main reason I kept it was the second checksum doesn't eat it's own 
output and the unlink step could fail. 
   
   Of those, the second is likely not possible / never going to happen given 
this structure, but the first might do something we don't want. I'll wrap that 
in a try + clean this up so that we're checking the output of the command there.



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to