fix(repair-built-stamp): patch the published PACKAGES.rds once bincraft drops cranlike (#236)

Follow-up to https://codefloe.com/rpkgs/bincraft/pulls/228 (bincraft#33), which drops the cranlike and desc forks.

## Why

After that change, `upload_package_index()` no longer keeps a `PACKAGES.db`.
It reuses the records of the slot's published `PACKAGES.rds`, as long as each tarball is still listed with the same ETag.
`local/repair-built-stamp.R` patched `PACKAGES.db`, so it would stop having any effect.

## Changes

- `local/repair-built-stamp.R` now patches `Built` in each slot's `PACKAGES.rds` and then re-emits the index.
  - Only the slot's own rows are patched: the generic slot's rows without a `Path`, or a per-minor slot's rows whose `Path` is its minor.
  - A missing stamp is not counted as broken, since bincraft clears it on purpose for source fallbacks.
  - With a bincraft release that still loads cranlike, the script patches `PACKAGES.db` as before, so merge order does not matter.
  - `bincraft::built_stamp()` was never exported, so the script failed at that line before this change; it now calls `bincraft:::built_stamp()`.
- The `GITHUB_PAT` comments in `reindex`, `weekly-rebuild-reindex` and `backfill-flat-safety` now give the current reason: bincraft reads DESCRIPTIONs from raw.githubusercontent.com.
- The README section on index files describes `upload_package_index()` instead of the forks.

## Testing

Tested against a moto S3 bucket, with bincraft installed from the bincraft#228 branch:

- A slot was indexed, then 3 of its 5 `Built` stamps were set to `R 4.5.0; NA; ...`.
- The dry run reported 5 entries, 3 broken.
- `--apply` rewrote the 3 stamps, and the re-emit logged `Index records: 5 reused, 0 read, 0 unreadable.`
- The published `PACKAGES` then had a valid stamp on all 5 records.
- With the released bincraft, which still loads cranlike, the script chose `PACKAGES.db`.

The stale `PACKAGES.db` objects in the slots can be deleted once a bincraft release with #228 is in use.

Reviewed-on: #236

.crow/backfill-flat-safety.yaml

@@ -140,12 +140,11 @@ steps:
         from_secret: PGPASS
       REPO_RO_TOKEN:
         from_secret: REPO_RO_TOKEN
-      # uvr resolves bincraft's dependencies through the GitHub API, including
-      # the pat-s/cranlike@s3 fork. Unauthenticated it is rate limited, and the
-      # failure is not loud: uvr warns, falls back to the registry copy of
-      # cranlike, and the index write then dies on `unused argument (built =
-      # built)` because upstream has no such parameter. Intermittent by nature,
-      # since it depends on the rate-limit window.
+      # bincraft reads the DESCRIPTION of every new or replaced tarball from
+      # the CRAN mirror on raw.githubusercontent.com, and authenticates with
+      # this token when it is set. A slot whose published index cannot be read
+      # is re-read in full, ~25k requests, which the anonymous rate limit would
+      # cut short.
       GITHUB_PAT:
         from_secret: GITHUB_PAT
       GIT_USER: pat-s

.crow/reindex.yaml

@@ -141,12 +141,11 @@ steps:
         from_secret: PGPASS
       REPO_RO_TOKEN:
         from_secret: REPO_RO_TOKEN
-      # uvr resolves bincraft's dependencies through the GitHub API, including
-      # the pat-s/cranlike@s3 fork. Unauthenticated it is rate limited, and the
-      # failure is not loud: uvr warns, falls back to the registry copy of
-      # cranlike, and the index write then dies on `unused argument (built =
-      # built)` because upstream has no such parameter. Intermittent by nature,
-      # since it depends on the rate-limit window.
+      # bincraft reads the DESCRIPTION of every new or replaced tarball from
+      # the CRAN mirror on raw.githubusercontent.com, and authenticates with
+      # this token when it is set. A slot whose published index cannot be read
+      # is re-read in full, ~25k requests, which the anonymous rate limit would
+      # cut short.
       GITHUB_PAT:
         from_secret: GITHUB_PAT
       GIT_USER: pat-s

.crow/repair-built-stamp.yaml

@@ -3,8 +3,8 @@
 #
 # uvr matches the stamp's platform triple plus R minor to decide binary vs
 # source, so an unusable triple turns a whole slot source-only. See
-# local/repair-built-stamp.R for why this patches PACKAGES.db in place instead
-# of forcing a full reparse.
+# local/repair-built-stamp.R for why this patches the published PACKAGES.rds in
+# place instead of forcing a full reparse.
 #
 # Run with `dry_run: true` first: it reports how many entries are broken per
 # slot and changes nothing. Pick the R version the slot should advertise, which

.crow/weekly-rebuild-reindex.yaml

@@ -4,8 +4,8 @@
 # objects in place, so the slot's index still advertises the old MD5 and, for
 # anything that had been served from source, no Built stamp. Re-indexing from
 # inside a shard would mean three concurrent `upload_package_index()` calls on
-# one prefix: `cranlike::update_PACKAGES()` lists the live bucket, so an early
-# lister that uploads last publishes an index missing its siblings' work.
+# one prefix: each lists the live bucket, so an early lister that uploads last
+# publishes an index missing its siblings' work.
 #
 # So it happens exactly once per slot, here, after every shard has finished.
 # `runs_on: [success, failure]` keeps that true when a shard fails; only an
@@ -144,12 +144,11 @@ steps:
         from_secret: PGPASS
       REPO_RO_TOKEN:
         from_secret: REPO_RO_TOKEN
-      # uvr resolves bincraft's dependencies through the GitHub API, including
-      # the pat-s/cranlike@s3 fork. Unauthenticated it is rate limited, and the
-      # failure is not loud: uvr warns, falls back to the registry copy of
-      # cranlike, and the index write then dies on `unused argument (built =
-      # built)` because upstream has no such parameter. Intermittent by nature,
-      # since it depends on the rate-limit window.
+      # bincraft reads the DESCRIPTION of every new or replaced tarball from
+      # the CRAN mirror on raw.githubusercontent.com, and authenticates with
+      # this token when it is set. A slot whose published index cannot be read
+      # is re-read in full, ~25k requests, which the anonymous rate limit would
+      # cut short.
       GITHUB_PAT:
         from_secret: GITHUB_PAT
       GIT_USER: pat-s

README.md

@@ -103,11 +103,13 @@ Preview the selection without building or accessing storage or build metadata:
 
 ### Creating/Updating the PACKAGES Index Files
 
-Currently, the {cranlike} and {desc} packages only work with files on a local file system.
+Tools such as `tools::write_PACKAGES()` and {cranlike} only index files on a local file system.
 This is infeasible if the goal is to store binaries in S3.
 Storing binaries permanently on a disk-based file system would incur significantly higher costs, especially when operating in the cloud.
 
-Hence, [{cranlike}](https://devxy.gitlab.io/r-package-binaries/bincraft/) and [{desc}](https://github.com/pat-s/desc/tree/description-from-remote) were forked to handle files in S3 (through {s3fs}).
+Hence, `bincraft::upload_package_index()` builds the index from a listing of the slot in S3 (through {s3fs}).
+Each record is read from the package's CRAN `DESCRIPTION` on the GitHub mirror of CRAN, falling back to the `DESCRIPTION` inside the tarball for a package the mirror does not have.
+The previously published `PACKAGES.rds` serves as a cache, so only new or replaced tarballs are read.
 
 ### Resources
 

local/repair-built-stamp.R

@@ -9,21 +9,22 @@
 ### fail wherever a system `-dev` library is missing.
 ###
 ### Rewriting it is not a matter of re-running the normal index update.
-### `upload_package_index()` reuses the slot's remote `PACKAGES.db`, and
-### cranlike's `update_db()` only reparses files whose md5 changed, so entries
-### already in the database keep the stamp they were written with. Dropping
-### `PACKAGES.db` to force a full reparse does work, but for an S3 repo cranlike
-### reads each package's metadata from the CRAN *source* mirror on GitHub, so a
-### 25k-entry slot means 25k requests to raw.githubusercontent.com and a real
-### risk of being rate-limited part-way through.
+### `upload_package_index()` uses the slot's published `PACKAGES.rds` as its
+### cache: a record whose tarball is still listed with the same ETag is reused,
+### stamp included, and only new or replaced tarballs are read and stamped. A
+### full re-read would mean 25k requests to raw.githubusercontent.com per slot
+### and a real risk of being rate-limited part-way through.
 ###
 ### Only the `Built` column is wrong, so correct it in place instead: patch the
-### column in `PACKAGES.db`, put the database back, and let
-### `upload_package_index()` re-emit `PACKAGES*` from it. `update_db()` always
-### rewrites the index files even when nothing was reparsed, so no tarball is
-### re-read and nothing is fetched from GitHub.
+### column in the published `PACKAGES.rds`, put it back, and let
+### `upload_package_index()` re-emit `PACKAGES*` from it. No tarball is re-read
+### and nothing is fetched from GitHub.
 ###
-### The replacement comes from `bincraft::built_stamp()` under the R running
+### bincraft releases before the switch away from cranlike (bincraft#33) read
+### the slot's `PACKAGES.db` instead, so with one of those installed the script
+### patches that database, as it always did.
+###
+### The replacement comes from bincraft's `built_stamp()` under the R running
 ### this script, so run it under the R version the slot should advertise (the
 ### `R_VERSION` its entry in `.crow/process-updates.yaml` uses). That is what a
 ### healthy `upload_package_index()` run would have written.
@@ -61,8 +62,9 @@ if (is.null(codename) || is.na(codename) || !nzchar(codename)) {
 }
 
 # built_stamp() refuses an unusable platform, so a broken build image fails here
-# rather than writing a second bad stamp over the first one.
-stamp <- bincraft::built_stamp()
+# rather than writing a second bad stamp over the first one. It is internal to
+# bincraft, hence `:::`.
+stamp <- bincraft:::built_stamp()
 message(sprintf("Slot:        %s/%s", arch, codename))
 message(sprintf("New stamp:   %s", stamp))
 
@@ -76,7 +78,12 @@ s3fs::s3_file_system(
 
 base_dir <- file.path(bucket, arch, codename, "latest", "src", "contrib")
 
-# A stamp is broken when its platform component is absent or literally "NA".
+# A stamp is broken when its platform component is literally "NA" or empty.
+broken_stamp <- function(built) {
+  !is.na(built) & grepl("; NA;|;  ;", built)
+}
+# The same test for PACKAGES.db, which also counts a missing stamp as broken:
+# the database never holds a deliberately cleared one.
 broken_stamp_where <- paste(
   "Built IS NULL",
   "OR Built LIKE '%; NA;%'",
@@ -91,9 +98,70 @@ r_minors <- sub(
   list.dirs("/opt/R", recursive = FALSE)
 )
 r_minors <- unique(grep("^[0-9]+\\.[0-9]+$", r_minors, value = TRUE))
-slots <- c(base_dir, file.path(base_dir, r_minors))
+slots <- c(
+  stats::setNames(base_dir, NA_character_),
+  stats::setNames(file.path(base_dir, r_minors), r_minors)
+)
 
-repair_slot <- function(slot) {
+# bincraft imports cranlike for as long as it keeps the index in PACKAGES.db.
+uses_packages_db <- "cranlike" %in% loadedNamespaces()
+
+repair_packages_rds <- function(slot, r_minor) {
+  rds_remote <- file.path(slot, "PACKAGES.rds")
+  if (!s3fs::s3_file_exists(rds_remote)) {
+    message(sprintf("  %s: no PACKAGES.rds, skipping", slot))
+    return(invisible(NULL))
+  }
+
+  rds_local <- tempfile(fileext = ".rds")
+  on.exit(unlink(rds_local), add = TRUE)
+  s3fs::s3_file_download(rds_remote, rds_local, overwrite = TRUE)
+  records <- readRDS(rds_local)
+
+  # A per-minor index is published as a union with the generic slot, and only
+  # the rows carrying the slot's own `Path` are reused from it; the generic ones
+  # are re-read from the generic slot's index, which is repaired on its own.
+  own <- if (!("Path" %in% colnames(records))) {
+    rep(TRUE, nrow(records))
+  } else if (is.na(r_minor)) {
+    is.na(records[, "Path"])
+  } else {
+    records[, "Path"] %in% r_minor
+  }
+  # A missing stamp is not broken here: bincraft clears it on purpose for a
+  # package whose build failed and that is served as its CRAN source.
+  broken <- own & broken_stamp(records[, "Built"])
+
+  message(sprintf(
+    "  %s: %s entries, %s broken",
+    slot,
+    sum(own),
+    sum(broken)
+  ))
+  if (!any(broken)) {
+    return(invisible(NULL))
+  }
+  if (!apply_changes) {
+    message("    (dry run, pass --apply to rewrite)")
+    return(invisible(NULL))
+  }
+
+  records[broken, "Built"] <- stamp
+  saveRDS(records, rds_local, compress = "xz")
+  s3fs::s3_file_upload(
+    rds_local,
+    rds_remote,
+    overwrite = TRUE,
+    CacheControl = "no-store"
+  )
+  message(sprintf(
+    "    rewrote %s entries and uploaded PACKAGES.rds",
+    sum(broken)
+  ))
+  invisible(NULL)
+}
+
+repair_packages_db <- function(slot, r_minor) {
   db_remote <- file.path(slot, "PACKAGES.db")
   if (!s3fs::s3_file_exists(db_remote)) {
     message(sprintf("  %s: no PACKAGES.db, skipping", slot))
@@ -134,16 +202,21 @@ repair_slot <- function(slot) {
   invisible(NULL)
 }
 
-invisible(lapply(slots, repair_slot))
+repair_slot <- if (uses_packages_db) repair_packages_db else repair_packages_rds
+message(sprintf(
+  "Patching:    %s",
+  if (uses_packages_db) "PACKAGES.db" else "PACKAGES.rds"
+))
+invisible(Map(repair_slot, slots, names(slots)))
 
 if (!apply_changes) {
   message("Dry run complete; nothing was changed.")
   quit(save = "no")
 }
 
-# Re-emit PACKAGES/PACKAGES.gz/PACKAGES.rds from the corrected database. Nothing
-# is reparsed, because no tarball's md5 changed.
-message("Re-emitting index files from the corrected database...")
+# Re-emit PACKAGES/PACKAGES.gz/PACKAGES.rds from the corrected records. Nothing
+# is reparsed, because no tarball changed.
+message("Re-emitting index files from the corrected records...")
 bincraft::upload_package_index(
   codename = codename,
   s3_endpoint = endpoint,