fix(build): strip the s3 scheme so per-minor objects reach their pass #192

Merged
pat-s merged 2 commits from fix/strip-s3-scheme-from-cache-paths into main 2026-09-02 07:11:11 +00:00
2 changed files with 48 additions and 7 deletions
Showing only changes of commit f0b76ddf94 - Show all commits

fix(build): strip the s3 scheme so per-minor objects reach their pass

`s3_dir_ls()` returns keys with the `s3://` scheme attached, but the contrib
prefix was removed as a fixed substring. That took the prefix out of the middle
of the key and left `s3://4.4/curl_1.0.tar.gz`, so the leading scheme defeated
the `^<minor>/` test in `select_cache_for_pass()`.

Every one of the 21212 per-minor objects on amd64/resolute was therefore read as
a flat-slot object: the primary pass reported "119053 of 119053 objects apply to
this pass" and each sensitive pass got an empty cache and recompiled all 10248
sensitive packages it already had.

This change will:

- anchor the contrib prefix and swallow an optional `s3://` with it
- abort when the strip leaves fewer per-minor paths than the listing held, since
  the failure is otherwise silent and only shows up as a full rebuild
- mark the cache format with a `slot_relative` attribute and read that in
  `build-all.R`, rather than sniffing for a `/`, which misreads a slot-relative
  cache holding no per-minor or Archive object as a legacy one
Patrick Schratz 2026-09-01 22:47:24 +00:00
No known key found for this signature in database
GPG key ID: 62050D5BC68AB6DC

View file

@ -201,10 +201,15 @@ s3_cache_paths <- readRDS("/mnt/cache/packages/s3_cache.rds")
# nothing; hand it nothing and it rebuilds what it already has. amd64/resolute
# recompiled 8683 packages that way.
select_cache_for_pass <- function(paths, sensitive_only, r_minor) {
# A cache written before this change holds bare basenames. Filtering those by
# path would select nothing and trigger a full rebuild, so use them as they
# are; #190's staleness check replaces it on the next pipeline anyway.
if (!any(grepl("/", paths, fixed = TRUE))) {
# A cache written before #191 holds bare basenames. Filtering those by path
# would select nothing and trigger a full rebuild, so use them as they are;
# #190's staleness check replaces it on the next pipeline anyway.
#
# The format is read from the marker `packages-to-build.R` sets, not guessed
# from the content: a slot-relative cache with no per-minor or Archive object
# holds bare names too, and treating that as legacy would hand a per-minor
# pass the flat slot's binaries and build nothing.
if (!isTRUE(attr(paths, "slot_relative"))) {
message("S3 cache is in the legacy basename format; using it unfiltered.")
return(paths)
}

View file

@ -190,14 +190,44 @@ binary_cache <- setdiff(file_names, source_served)
# So the cache keeps the path relative to the slot, and `build-all.R` selects
# the part that matches the pass it is running: the flat slot for the primary,
# `<minor>/` for a per-minor pass.
#
# `s3_dir_ls()` returns keys with the `s3://` scheme attached, so stripping the
# prefix as a fixed substring takes it out of the middle and leaves
# `s3://4.4/curl_1.0.tar.gz`. That leading scheme defeats the `^<minor>/` test
# downstream, so every per-minor object is read as a flat-slot object: the
# sensitive passes see an empty cache and recompile everything they already
# have. Anchor the pattern and swallow the scheme with it.
contrib_prefix <- sprintf(
"devxy-rpkgs-binaries/%s/%s/latest/src/contrib/",
"^(s3://)?devxy-rpkgs-binaries/%s/%s/latest/src/contrib/",
arch,
codename
)
relative_paths <- sub(contrib_prefix, "", s3_pkgs, fixed = TRUE)
relative_paths <- sub(contrib_prefix, "", s3_pkgs)
# The strip is load-bearing and fails silently, so assert it. Both counts are
# derived from the same listing and use the same shape of pattern, so they must
# agree exactly; a mismatch means the prefix no longer describes the keys.
stripped_per_minor <- grepl("^[0-9]+\\.[0-9]+/[^/]+$", relative_paths)
if (sum(stripped_per_minor) != sum(per_minor_object)) {
stop(sprintf(
paste0(
"S3 prefix strip failed: %d per-minor objects in the listing, %d after ",
"stripping /%s/. Example key: %s"
),
sum(per_minor_object),
sum(stripped_per_minor),
contrib_prefix,
if (any(per_minor_object)) {
s3_pkgs[which(per_minor_object)[1L]]
} else {
"<none>"
}
))
}
source_basenames <- source_served
existence_cache <- relative_paths[!basename(relative_paths) %in% source_basenames]
existence_cache <- relative_paths[
!basename(relative_paths) %in% source_basenames
]
cat(sprintf(
"S3 cache: %d objects, %d served as CRAN source, %d usable binaries\n",
length(file_names),
@ -208,6 +238,12 @@ cat(sprintf(
# 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
# Mark the format explicitly. `build-all.R` has to tell a slot-relative cache
# from a pre-#191 basename one, and sniffing for a "/" cannot: a new-format
# cache for a slot with no per-minor or Archive objects holds bare names too,
# and would be read as legacy and used unfiltered, which makes a per-minor pass
# believe the flat slot's binaries are its own and build nothing.
attr(existence_cache, "slot_relative") <- TRUE
saveRDS(existence_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