From 94e6c697cf831965de5464391c54db571901ba0f Mon Sep 17 00:00:00 2001 From: pat-s Date: Mon, 31 Aug 2026 21:36:11 +0000 Subject: [PATCH] fix(build): stop per-minor objects masking the per-minor candidate list (#189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation `arm64/alpine324` (pipeline 11953) finished in minutes having uploaded 57 packages, and reported: ``` Skipped 0 package versions already attempted under R 4.4; 0 remaining Skipped 0 package versions already attempted under R 4.6; 3 remaining ``` The same run's index step dropped **2407** packages as missing for 4.4 and **2436** for 4.6. Nothing to build, and thousands missing — the candidate list is wrong. ## Two omissions **1. Per-minor objects mask the per-minor candidates.** ```r s3_pkgs <- s3fs::s3_dir_ls(".../latest/src/contrib", recurse = TRUE) file_names <- basename(s3_pkgs) ``` `recurse = TRUE` walks `4.4/`, `4.5/`, `4.6/`; `basename()` throws the directory away. `4.5/curl_1.0.tar.gz` and `curl_1.0.tar.gz` collapse to one name, so a package present under **any** R minor counts as built for **all** of them — pruning exactly the packages a per-minor pass exists to build. Per-minor objects are now excluded, and presence in a specific minor is decided downstream where the running R version is known: `build-all.R` filters on it, and `build_binary_package()` checks the per-minor path per package and skips what is already there. `Archive/` is kept. Those are versions built and later superseded; dropping them would make every archived version look unbuilt. Validated against real path shapes: | path | | |---|---| | `curl_1.0.tar.gz` | keep | | `4.4/curl_1.0.tar.gz` | exclude | | `4.6/rlang_1.3.0.tar.gz` | exclude | | `Archive/curl/curl_0.9.tar.gz` | keep | | `PACKAGES.gz` | keep | **2. The error query ignores `r_version`.** ```sql SELECT error_occurred FROM single_builds WHERE name = $1 AND tag = $2 AND platform = $3 AND arch = $4 ``` A failure under the primary minor drops the package from every other minor's candidate list. This is the same omission fixed in `local/build-all.R` (#187) and in bincraft's `check_package_error()` (rpkgs/bincraft#119). This is the third and last consumer of that table — I have grepped the rest; `bincraft::R/cran-internal.R` also reads it, but to list packages present rather than to skip, where the R minor does not apply. ## Expected effect The per-minor passes get real candidate lists. Expect slots that reported "0 remaining" to report thousands, and correspondingly long runs. There is a cost: the list is no longer pruned by per-minor presence, so each pass asks `build_binary_package()` about packages that may already exist, and it answers `already exists in S3 ... Skipping build` per package. Slower per pass, and correct — the pruning it replaces was removing the wrong things. Reviewed-on: https://git.devxy.io/devxy/build-cran-binaries/pulls/189 --- local/packages-to-build.R | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/local/packages-to-build.R b/local/packages-to-build.R index aa30390..9cc1ed6 100644 --- a/local/packages-to-build.R +++ b/local/packages-to-build.R @@ -116,7 +116,28 @@ s3_pkgs <- s3fs::s3_dir_ls( recurse = TRUE ) -file_names <- basename(s3_pkgs) +# `recurse = TRUE` walks the per-minor slots as well, and `basename()` throws +# the directory away - so `4.5/curl_1.0.tar.gz` and `curl_1.0.tar.gz` collapse +# to one name and a package present under *any* R minor counts as built for +# *all* of them. The candidate list then prunes exactly the packages a +# per-minor pass exists to build: arm64/alpine324 reported "0 remaining" for +# both 4.4 and 4.6 while its indexes were dropping 2400+ packages as missing. +# +# Per-minor objects are therefore excluded here. Presence in a specific minor +# is decided downstream, where the running R version is known: build-all.R +# filters on it, and `build_binary_package()` checks the per-minor path per +# package and skips what is already there. +# +# Archive/ is kept. Those are versions that were built and then superseded; +# dropping them would make every archived version look unbuilt. +per_minor_object <- grepl("/[0-9]+\\.[0-9]+/[^/]+$", s3_pkgs) +if (any(per_minor_object)) { + cat(sprintf( + "Excluding %d per-minor object(s) from the presence check; those are decided per pass\n", + sum(per_minor_object) + )) +} +file_names <- basename(s3_pkgs[!per_minor_object]) # An object occupying a key is not proof a binary was built: a package whose # build failed has its CRAN source published under exactly that name. Left in @@ -179,11 +200,22 @@ s3_dt <- data.table( ### Get all packages with build errors +# Scoped to the R minor this snapshot is computed under. A failure is a fact +# about one interpreter: without the scope a package that failed under the +# primary minor is dropped from the candidate list for every other minor too, +# which is the same omission fixed in local/build-all.R and in bincraft's +# check_package_error(). +snapshot_r_minor <- paste( + R.version$major, + strsplit(R.version$minor, ".", fixed = TRUE)[[1L]][1L], + sep = "." +) sql_query <- paste0( # nolint "SELECT error_occurred FROM ", "single_builds", - " WHERE name = $1 AND tag = $2 AND platform = $3 AND arch = $4" + " WHERE name = $1 AND tag = $2 AND platform = $3 AND arch = $4", + " AND substring(r_version from '^[0-9]+[.][0-9]+') = $5" ) # Function to query for a single package-version query_error <- function(pkg, ver) { @@ -191,7 +223,7 @@ query_error <- function(pkg, ver) { ~ DBI::dbGetQuery( con, sql_query, - params = list(pkg, ver, platform, arch) + params = list(pkg, ver, platform, arch, snapshot_r_minor) ), rate = purrr::rate_backoff( pause_base = 1L,