From a126d74cd37422d2c0a9e8dec782173f945585eb Mon Sep 17 00:00:00 2001 From: pat-s Date: Sun, 9 Aug 2026 16:27:51 +0000 Subject: [PATCH] fix(build): keep source fallbacks out of the S3 package cache (#159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem This is the gap flagged in rpkgs/bincraft#106. `build_binary_package()` has a fast path that compares against `s3_package_cache` instead of querying S3 per package, and that cache is produced here: ```r s3_pkgs <- s3fs::s3_dir_ls(".../latest/src/contrib", recurse = TRUE) saveRDS(basename(s3_pkgs), "/mnt/cache/packages/s3_cache.rds") ``` A raw bucket listing cannot tell a binary from a package whose build failed and was published as its CRAN source — the two occupy the same key. So every source fallback reads as "already built" and is skipped for good. That is how `alpine324` accumulated ~13.5k of them. The same listing feeds `s3_dt`, which is subtracted from the build list at line 194 (`pkgs <- pkgs_no_error[!s3_dt]`). That one matters more: it excludes the very packages that need building, before `build_binary_package()` is even called. ## What this changes Drops from the listing every object the slot's own index reports as served from source. bincraft leaves the `Built` stamp off exactly those records (rpkgs/bincraft#105), so the index already carries the answer and no credentials, downloads or extra API calls are needed. Both consumers are fixed: the saved cache and `s3_dt`. The cache stays a plain filename vector, so the build container still needs no `s3fs`/reticulate — that was the point of saving it in the first place. Two deliberately conservative edges: - archived objects have no index record, so they are kept. Unknown means binary, never "rebuild it". - if the index cannot be read, the full listing is kept and a warning is printed, so a CDN blip cannot mass-schedule a rebuild. ## Verification The script parses, and the new block run against the live indices: ``` amd64/alpine324: index=24235 source-served=13542 e.g. AATtools_0.0.3.tar.gz, ABCDscores_7.0.0.tar.gz amd64/noble: index=24681 source-served=0 ``` `alpine324` is re-indexed by bincraft 5.1.1, so 13 542 objects drop out and those packages become buildable. `noble` has not been re-indexed yet, so every record still carries `Built`, nothing is dropped, and its behaviour is exactly what it is today — the safe failure mode this relies on. The new log line makes it visible per run: ``` S3 cache: N objects, M served as CRAN source, K usable binaries ``` ## Sequencing Needs rpkgs/bincraft#106 (and a release) before a rebuild actually builds: this fixes the bulk build path's list, #106 fixes the per-package pre-build skip. Reviewed-on: https://git.devxy.io/devxy/build-cran-binaries/pulls/159 --- local/packages-to-build.R | 59 +++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/local/packages-to-build.R b/local/packages-to-build.R index 887952d..3b508fb 100644 --- a/local/packages-to-build.R +++ b/local/packages-to-build.R @@ -89,14 +89,61 @@ s3_pkgs <- s3fs::s3_dir_ls( recurse = TRUE ) -# Save the raw S3 file listing for the build step to use as s3_package_cache +file_names <- basename(s3_pkgs) + +# 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 +# the cache, `build_binary_package()` reads it as "already built" and skips the +# package forever, which is how alpine324 accumulated ~13.5k source tarballs. +# +# bincraft stamps `Built` only on records it actually built, so the slot's own +# index distinguishes them. A slot last indexed by a bincraft that predates that +# fix stamps `Built` on everything, so the cache is then unchanged from before. +# Archived objects have no index record and are kept: unknown means binary, +# never "rebuild it". +index_url <- sprintf( + "https://cran.rpkgs.com/%s/%s/latest/src/contrib/PACKAGES.gz", + arch, + codename +) +source_served <- tryCatch( + { + con_idx <- gzcon(url(index_url, open = "rb")) + on.exit(close(con_idx), add = TRUE) + idx <- read.dcf(con_idx, fields = c("Package", "Version", "Built")) + sprintf( + "%s_%s.tar.gz", + idx[is.na(idx[, "Built"]), "Package"], + idx[is.na(idx[, "Built"]), "Version"] + ) + }, + error = function(e) { + cat(sprintf( + "WARNING: could not read %s (%s); keeping the full S3 cache\n", + index_url, + conditionMessage(e) + )) + character(0) + } +) + +binary_cache <- setdiff(file_names, source_served) +cat(sprintf( + "S3 cache: %d objects, %d served as CRAN source, %d usable binaries\n", + length(file_names), + length(file_names) - length(binary_cache), + length(binary_cache) +)) + +# Save the S3 file listing for the build step to use as s3_package_cache. # This avoids loading s3fs/reticulate in the build container, saving memory for # the dependency-installer subprocesses -saveRDS(basename(s3_pkgs), "/mnt/cache/packages/s3_cache.rds") - -file_names <- basename(s3_pkgs) -matches <- regexec("^([A-Za-z0-9.]+)_([0-9][^/]*)\\.tar\\.gz$", file_names) -parts <- regmatches(file_names, matches) +saveRDS(binary_cache, "/mnt/cache/packages/s3_cache.rds") +# Built from the filtered listing, not the raw one: `s3_dt` is subtracted from +# the build list below, so a source fallback left in here would exclude the very +# package that needs building. +matches <- regexec("^([A-Za-z0-9.]+)_([0-9][^/]*)\\.tar\\.gz$", binary_cache) +parts <- regmatches(binary_cache, matches) parts <- parts[sapply(parts, length) == 3] s3_dt <- data.table( Package = sapply(parts, `[`, 2),