GH-38430: [R] Add test + fix corner cases after nixlibs.R refactor (#…

…38534) ### Rationale for this change A few rough edges exist after #38236 including: - When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail - At least one CI job is pulling nightlies instead of using the version from the current commit ### What changes are included in this PR? - Clean up `find_latest_nightly()` + add test - Ensure all CI jobs are not using `find_latest_nightly()` ### Are these changes tested? Yes (test added) ### Are there any user-facing changes? No * Closes: #38430 Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jonathan Keane <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
apache · Nov 10, 2023 · c260a24 · c260a24
1 parent 8fcaba7
commit c260a24
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 14 deletions.
diff --git a/r/tools/nixlibs.R b/r/tools/nixlibs.R
@@ -35,7 +35,9 @@ exit <- function(..., .status = 1) {
 
 
 # checks the nightly repo for the latest nightly version X.Y.Z.100<dev>
-find_latest_nightly <- function(description_version) {
+find_latest_nightly <- function(description_version,
+                                list_uri = "https://nightlies.apache.org/arrow/r/src/contrib/PACKAGES",
+                                hush = quietly) {
   if (!startsWith(arrow_repo, "https://nightlies.apache.org/arrow/r")) {
     lg("Detected non standard dev repo: %s, not checking latest nightly version.", arrow_repo)
     return(description_version)
@@ -44,17 +46,28 @@ find_latest_nightly <- function(description_version) {
   res <- try(
     {
       # Binaries are only uploaded if all jobs pass so can just look at the source versions.
-      urls <- readLines("https://nightlies.apache.org/arrow/r/src/contrib")
-      versions <- grep("arrow_.*\\.tar\\.gz", urls, value = TRUE)
-      versions <- sub(".*arrow_(.*)\\.tar\\.gz.*", "\\1", x = versions)
-      versions <- sapply(versions, package_version)
-      versions <- data.frame(do.call(rbind, versions))
-      matching_major <- versions[versions$X1 == description_version[1, 1], ]
-      latest <- matching_major[which.max(matching_major$X4), ]
-      package_version(paste0(latest, collapse = "."))
+      urls <- readLines(list_uri)
+      versions <- grep("Version:\\s*.*?", urls, value = TRUE)
+      versions <- sort(package_version(sub("Version:\\s*", "\\1", versions)))
+      major_versions <- versions$major
+
+      description_version_major <- as.integer(description_version[1, 1])
+      matching_major <- major_versions == description_version_major
+      if (!any(matching_major)) {
+        lg(
+          "No nightly binaries were found for version %s: falling back to libarrow build from source",
+          description_version
+        )
+
+        return(description_version)
+      }
+
+      versions <- versions[matching_major]
+      max(versions)
     },
-    silent = quietly
+    silent = hush
   )
+
   if (inherits(res, "try-error")) {
     lg("Failed to find latest nightly for %s", description_version)
     latest <- description_version
@@ -832,16 +845,19 @@ quietly <- !env_is("ARROW_R_DEV", "true")
 
 not_cran <- env_is("NOT_CRAN", "true")
 
-if (is_release & !test_mode) {
+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 if(!test_mode) {
+} else {
   not_cran <- TRUE
   arrow_repo <- paste0(getOption("arrow.dev_repo", "https://nightlies.apache.org/arrow/r"), "/libarrow/")
+}
+
+if (!is_release && !test_mode) {
   VERSION <- find_latest_nightly(VERSION)
 }
 
-# To collect dirs to rm on exit, use del() to add dirs
+# To collect dirs to rm on exit, use cleanup() to add dirs
 # we reset it to avoid errors on reruns in the same session.
 options(.arrow.cleanup = character())
 on.exit(unlink(getOption(".arrow.cleanup"), recursive = TRUE), add = TRUE)
@@ -867,6 +883,8 @@ build_ok <- !env_is("LIBARROW_BUILD", "false")
 #  https://arrow.apache.org/docs/developers/cpp/building.html#offline-builds)
 download_ok <- !test_mode && !env_is("TEST_OFFLINE_BUILD", "true")
 
+download_libarrow_ok <- download_ok && !env_is("LIBARROW_DOWNLOAD", "false")
+
 # This "tools/thirdparty_dependencies" path, within the tar file, might exist if
 # create_package_with_all_dependencies() was run, or if someone has created it
 # manually before running make build.
@@ -904,7 +922,7 @@ if (!test_mode && !file.exists(api_h)) {
       lg("File not found: %s ($ARROW_DOWNLOADED_BINARIES)", bin_zip)
       bin_file <- NULL
     }
-  } else if (download_ok) {
+  } else if (download_libarrow_ok) {
     binary_flavor <- identify_binary()
     if (!is.null(binary_flavor)) {
       # The env vars say we can, and we've determined a lib that should work

diff --git a/r/tools/test-nixlibs.R b/r/tools/test-nixlibs.R
@@ -155,3 +155,68 @@ test_that("check_allowlist", {
   expect_true(check_allowlist("redhat", tempfile())) # remote allowlist doesn't exist, so we fall back to the default list, which contains redhat
   expect_false(check_allowlist("debian", tempfile()))
 })
+
+test_that("find_latest_nightly()", {
+  tf <- tempfile()
+  tf_uri <- paste0("file://", tf)
+  on.exit(unlink(tf))
+
+  writeLines(
+    c(
+      "Version: 13.0.0.100000333",
+      "Version: 13.0.0.100000334",
+      "Version: 13.0.0.100000335",
+      "Version: 14.0.0.100000001"
+    ),
+    tf
+  )
+
+  expect_output(
+    expect_identical(
+      find_latest_nightly(package_version("13.0.1.9000"), list_uri = tf_uri),
+      package_version("13.0.0.100000335")
+    ),
+    "Found latest nightly"
+  )
+
+  expect_output(
+    expect_identical(
+      find_latest_nightly(package_version("14.0.0.9000"), list_uri = tf_uri),
+      package_version("14.0.0.100000001")
+    ),
+    "Found latest nightly"
+  )
+
+  expect_output(
+    expect_identical(
+      find_latest_nightly(package_version("15.0.0.9000"), list_uri = tf_uri),
+      package_version("15.0.0.9000")
+    ),
+    "No nightly binaries were found for version"
+  )
+
+  # Check empty input
+  writeLines(character(), tf)
+  expect_output(
+    expect_identical(
+      find_latest_nightly(package_version("15.0.0.9000"), list_uri = tf_uri),
+      package_version("15.0.0.9000")
+    ),
+    "No nightly binaries were found for version"
+  )
+
+  # Check input that will throw an error
+  expect_output(
+    expect_identical(
+      suppressWarnings(
+        find_latest_nightly(
+          package_version("15.0.0.9000"),
+          list_uri = "this is not a URI",
+          hush = TRUE
+        )
+      ),
+      package_version("15.0.0.9000")
+    ),
+    "Failed to find latest nightly"
+  )
+})
diff --git a/r/vignettes/install.Rmd b/r/vignettes/install.Rmd
@@ -283,6 +283,7 @@ the bundled build script.  All boolean variables are case-insensitive.
 | --- | --- | :-: |
 | `LIBARROW_BUILD` | Allow building from source | `true` |
 | `LIBARROW_BINARY` | Try to install `libarrow` binary instead of building from source | (unset) |
+| `LIBARROW_DOWNLOAD` | Set to `false` to explicitly forbid fetching a `libarrow` binary | (unset) |
 | `LIBARROW_MINIMAL` | Build with minimal features enabled | (unset) |
 | `NOT_CRAN` | Set `LIBARROW_BINARY=true` and `LIBARROW_MINIMAL=false` | `false` |
 | `ARROW_R_DEV` | More verbose messaging and regenerates some code | `false` |