fix(abi): prune the objects recorded broken, not every risky package (#211)
## Why A dry run of the prune on `ubuntu-2404-amd64` (pipeline 12494): ``` Dry run: 3013 object(s) left in place. Re-run with --apply to delete. ``` 3013 objects on one slot. The recorded verdicts say roughly **230** of that slot's objects actually fail `dyn.load()` anywhere. The gap is the selector: the prune used `abi_cache_risky_packages()`, the source classifier, which calls **93%** of compiled packages risky where the binaries' own symbols say **4.3%**. Applying it would have deleted ~2800 working binaries and turned each into a source fallback for every client that cannot be routed by R minor. This is the same mistake #129 fixed in the index writer. The prune was written before the per-object verdicts existed (#200, before #128/#129) and was never brought forward. ## What changed - Selection comes from `abi_flat_safety`, every `NOT safe` row for the slot regardless of which minor it fails under, since the flat object is served to clients on all of them. - Matching is by **object**, `<package>_<version>`, not by package name. A package can have a safe flat binary at one version and an unsafe one at another; the old code deleted both. - The explicit `[pkg ...]` override is unchanged in behaviour and still matches by package name, which is what you want when naming a package by hand. - The refusal guard moves with the selector. An empty result now means the slot's flat-safety backfill never ran — the exact state rhel8 was found in today — and pruning nothing while reporting success would have hidden it. ## Not changed Still a dry run unless `PRUNE_APPLY=yes`, and still refuses to delete an object whose package CRAN no longer ships, since there would be no source to fall back to. ## Ordering note A prune converts each deleted binary into a source fallback. bincraft #131 fixes source fallbacks of *archived* versions being advertised as binaries, which breaks `uvr` outright. That should land before any large prune, or this will manufacture new instances of it. Reviewed-on: #211
This commit is contained in:
parent
af4cc6f414
commit
a84cca1949
2 changed files with 75 additions and 29 deletions
|
|
@ -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
|
# The list comes from `abi_flat_safety`, so it names the objects the binaries
|
||||||
# slot, so a risky package should have no flat binary at all. Packages built
|
# themselves report as broken, not the ones a source heuristic suspects. That
|
||||||
# before they were classified risky still do, and the flat slot is what every
|
# distinction is the whole cost of the operation: the heuristic calls 93% of
|
||||||
# client that cannot be routed by R minor receives. That binary was built under
|
# compiled packages risky and would delete roughly 3000 objects per slot to fix
|
||||||
# whichever R was primary at the time, so it dies at load under any minor that
|
# about 230.
|
||||||
# dropped a symbol it uses:
|
#
|
||||||
|
# `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
|
# base64enc.so: undefined symbol: SETLENGTH
|
||||||
#
|
#
|
||||||
|
|
|
||||||
|
|
@ -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:
|
# Usage:
|
||||||
# Rscript local/prune-stale-flat-binaries.R <arch> <codename> [--apply] [pkg ...]
|
# Rscript local/prune-stale-flat-binaries.R <arch> <codename> [--apply] [pkg ...]
|
||||||
#
|
#
|
||||||
# `upload_single_binary()` writes an ABI-risky package only to the per-minor
|
# The list comes from `abi_flat_safety`, written by the flat-safety backfill
|
||||||
# slot, never to the flat one, so a risky package should have no flat binary at
|
# from the binaries' own symbols, so it names exactly the objects that fail at
|
||||||
# all. Packages built before they were classified risky still do, and the flat
|
# dyn.load() somewhere. It deliberately does *not* use
|
||||||
# slot is what every client that cannot be routed by R minor receives. That
|
# `abi_cache_risky_packages()`: that source heuristic calls 93% of compiled
|
||||||
# binary was built under whichever R was primary at the time, so it fails at
|
# packages risky where the binaries say 4.3%, and pruning on it would delete
|
||||||
# load under any minor that dropped a symbol it uses:
|
# 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
|
# base64enc.so: undefined symbol: SETLENGTH
|
||||||
#
|
#
|
||||||
|
|
@ -60,27 +69,57 @@ flat_files <- relative[is_flat]
|
||||||
flat_pkg <- sub("_.*$", "", flat_files)
|
flat_pkg <- sub("_.*$", "", flat_files)
|
||||||
|
|
||||||
if (length(explicit) > 0L) {
|
if (length(explicit) > 0L) {
|
||||||
risky <- explicit
|
doomed <- flat_files[flat_pkg %in% explicit]
|
||||||
cat(sprintf("Restricted to %d named package(s)\n", length(risky)))
|
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 {
|
} else {
|
||||||
risky <- bincraft:::abi_cache_risky_packages()
|
con <- DBI::dbConnect(
|
||||||
cat(sprintf("Classifier cache reports %d risky package(s)\n", length(risky)))
|
RPostgres::Postgres(),
|
||||||
if (length(risky) == 0L) {
|
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(
|
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
|
call. = FALSE
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
doomed <- flat_files[flat_pkg %in% risky]
|
# Match the object, not the package. A package can have a safe flat binary at
|
||||||
cat(sprintf(
|
# one version and an unsafe one at another, and deleting both would throw
|
||||||
"%s/%s: %d flat binaries, %d belong to ABI-risky packages\n",
|
# 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,
|
arch,
|
||||||
codename,
|
codename,
|
||||||
length(flat_files),
|
length(flat_files),
|
||||||
length(doomed)
|
length(doomed)
|
||||||
))
|
))
|
||||||
|
}
|
||||||
|
|
||||||
# Never remove the last artifact of a package. The source fallback republishes
|
# 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
|
# the CRAN source under this key, but only for packages CRAN still ships, so
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue