diff --git a/.crow/prune-stale-flat-binaries.yaml b/.crow/prune-stale-flat-binaries.yaml index a759daf..500f008 100644 --- a/.crow/prune-stale-flat-binaries.yaml +++ b/.crow/prune-stale-flat-binaries.yaml @@ -1,11 +1,18 @@ -# Remove flat-slot binaries belonging to ABI-risky packages, then re-index. +# Remove flat-slot binaries recorded unable to load under some R minor, then +# re-index. # -# `upload_single_binary()` writes an ABI-risky package only to the per-minor -# slot, so a risky package should have no flat binary at all. Packages built -# before they were classified risky still do, and the flat slot is what every -# client that cannot be routed by R minor receives. That binary was built under -# whichever R was primary at the time, so it dies at load under any minor that -# dropped a symbol it uses: +# The list comes from `abi_flat_safety`, so it names the objects the binaries +# themselves report as broken, not the ones a source heuristic suspects. That +# distinction is the whole cost of the operation: the heuristic calls 93% of +# compiled packages risky and would delete roughly 3000 objects per slot to fix +# about 230. +# +# `upload_single_binary()` has written a non-portable package only to the +# per-minor slot since v5.3.0, so this is a one-time cleanup of what was +# uploaded before that. The flat slot is what every client that cannot be +# routed by R minor receives, and such a binary was built under whichever R was +# primary at the time, so it dies at load under any minor that dropped a symbol +# it uses: # # base64enc.so: undefined symbol: SETLENGTH # diff --git a/local/prune-stale-flat-binaries.R b/local/prune-stale-flat-binaries.R index dbb0ab1..aea97b4 100644 --- a/local/prune-stale-flat-binaries.R +++ b/local/prune-stale-flat-binaries.R @@ -1,14 +1,23 @@ -### Remove flat-slot binaries of packages that are ABI-risky. +### Remove flat-slot binaries recorded as unable to load under some R minor. # # Usage: # Rscript local/prune-stale-flat-binaries.R [--apply] [pkg ...] # -# `upload_single_binary()` writes an ABI-risky package only to the per-minor -# slot, never to the flat one, so a risky package should have no flat binary at -# all. Packages built before they were classified risky still do, and the flat -# slot is what every client that cannot be routed by R minor receives. That -# binary was built under whichever R was primary at the time, so it fails at -# load under any minor that dropped a symbol it uses: +# The list comes from `abi_flat_safety`, written by the flat-safety backfill +# from the binaries' own symbols, so it names exactly the objects that fail at +# dyn.load() somewhere. It deliberately does *not* use +# `abi_cache_risky_packages()`: that source heuristic calls 93% of compiled +# packages risky where the binaries say 4.3%, and pruning on it would delete +# roughly 3000 working objects per slot to fix about 230 broken ones. Selecting +# per object rather than per package is the same correction #129 made to the +# index writer. +# +# `upload_single_binary()` has written a non-portable package only to the +# per-minor slot since v5.3.0, so this is a one-time cleanup of objects +# uploaded before that, not a recurring sweep. The flat slot is what every +# client that cannot be routed by R minor receives, and such a binary was built +# under whichever R was primary at the time, so it fails at load under any +# minor that dropped a symbol it uses: # # base64enc.so: undefined symbol: SETLENGTH # @@ -60,27 +69,57 @@ flat_files <- relative[is_flat] flat_pkg <- sub("_.*$", "", flat_files) if (length(explicit) > 0L) { - risky <- explicit - cat(sprintf("Restricted to %d named package(s)\n", length(risky))) + doomed <- flat_files[flat_pkg %in% explicit] + cat(sprintf( + "%s/%s: %d flat binaries, %d belong to the %d named package(s)\n", + arch, + codename, + length(flat_files), + length(doomed), + length(explicit) + )) } else { - risky <- bincraft:::abi_cache_risky_packages() - cat(sprintf("Classifier cache reports %d risky package(s)\n", length(risky))) - if (length(risky) == 0L) { + con <- DBI::dbConnect( + RPostgres::Postgres(), + dbname = "build_metadata", + host = "r-binaries.devxy.io", + port = 15432L, + user = "rpkgs", + password = Sys.getenv("PGPASS"), + sslmode = "require" + ) + on.exit(try(DBI::dbDisconnect(con), silent = TRUE), add = TRUE) + + # Every recorded-unsafe row, whichever minor it fails under: the flat object + # is served to clients on all of them, so one failing minor condemns it. + rows <- DBI::dbGetQuery( + con, + paste0( + "SELECT package, version FROM abi_flat_safety ", + "WHERE NOT safe AND codename = $1 AND arch = $2" + ), + params = list(codename, arch) + ) + if (nrow(rows) == 0L) { stop( - "The classifier cache returned nothing. Refusing to proceed: an empty risky set here is indistinguishable from an unreachable cache, and would silently prune nothing while reporting success.", # nolint + "No unsafe objects are recorded for this slot. Refusing to proceed: that is indistinguishable from a slot whose flat-safety backfill never ran, and pruning nothing while reporting success would hide it.", # nolint call. = FALSE ) } -} -doomed <- flat_files[flat_pkg %in% risky] -cat(sprintf( - "%s/%s: %d flat binaries, %d belong to ABI-risky packages\n", - arch, - codename, - length(flat_files), - length(doomed) -)) + # Match the object, not the package. A package can have a safe flat binary at + # one version and an unsafe one at another, and deleting both would throw + # away a working binary. + unsafe_objects <- sprintf("%s_%s", rows$package, rows$version) + doomed <- flat_files[sub("\\.tar\\.gz$", "", flat_files) %in% unsafe_objects] + cat(sprintf( + "%s/%s: %d flat binaries, %d recorded unable to load under some R minor\n", + arch, + codename, + length(flat_files), + length(doomed) + )) +} # Never remove the last artifact of a package. The source fallback republishes # the CRAN source under this key, but only for packages CRAN still ships, so