diff --git a/.gitignore b/.gitignore index c072661..f4124a2 100644 --- a/.gitignore +++ b/.gitignore @@ -102,7 +102,3 @@ docs/ # Superpowers SDD scratch (briefs, reports, ledger) .superpowers/ - -# Internal design docs (plans and specs) are kept out of the repo. -plans/ -specs/ diff --git a/.markdownlint-cli2.yaml b/.markdownlint-cli2.yaml index 66e7cc8..032c0ea 100644 --- a/.markdownlint-cli2.yaml +++ b/.markdownlint-cli2.yaml @@ -2,3 +2,6 @@ ignores: - LICENSE.md - docs/superpowers/** + # Internal design docs (specs/plans) are not user-facing reference material. + - specs/** + - plans/** diff --git a/README.md b/README.md index df69048..a737b00 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,8 @@ Patching uses a two-tier approach: The system is implemented in `bincraft`: when a package needs patching, `bincraft` pre-builds it with the patch and serves the patched binary to `pak`, ensuring transitive dependents receive the fixed package. This way, the fix cascades to all packages that depend on it. +For the design rationale and architecture, see `specs/2026-06-30-package-patching-design.md`. + ## Build Environment Binaries are built on a mixed-architecture Kubernetes cluster using CI. diff --git a/local/archive-missed-pkgs.R b/local/archive-missed-pkgs.R index f817c3e..4a9aa35 100644 --- a/local/archive-missed-pkgs.R +++ b/local/archive-missed-pkgs.R @@ -8,8 +8,8 @@ s3fs::s3_file_system( refresh = T ) -arch = "arm64" -os = "alpine321" +arch <- "arm64" +os <- "alpine321" files <- s3fs::s3_dir_ls(sprintf( "devxy-r-package-binaries-hel1/%s/%s/latest/src/contrib", @@ -17,11 +17,11 @@ files <- s3fs::s3_dir_ls(sprintf( os )) length(unique(sapply(strsplit(basename(files), "_"), function(x) x[1]))) -non_archived = unique(sapply(strsplit(basename(files), "_"), function(x) { +non_archived <- unique(sapply(strsplit(basename(files), "_"), function(x) { x[1] })[duplicated(sapply(strsplit(basename(files), "_"), function(x) x[1]))]) # RInno is not built because it only exists for Windows -non_archived = setdiff(non_archived, "RInno") +non_archived <- setdiff(non_archived, "RInno") # future::plan("sequential", workers = 8) future::plan("sequential", workers = 1) # future.apply::future_lapply(non_archived, function(x) archive_package(x, codename = os, arch = arch)) diff --git a/local/check-NA.R b/local/check-NA.R index 95c158e..ce109bd 100644 --- a/local/check-NA.R +++ b/local/check-NA.R @@ -1,22 +1,30 @@ ### Scope: Check whether there are any incomplete entries in PACKAGES.rds. These will result in NA when calling available.packages() -foo = available.packages("https://cran.devxy.io/amd64/jammy/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/amd64/jammy/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/amd64/noble/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/amd64/noble/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/amd64/rhel8/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/amd64/rhel8/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/amd64/rhel9/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/amd64/rhel9/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages( +foo <- available.packages( "https://cran.devxy.io/amd64/alpine320/latest/src/contrib" ) sum((is.na(foo[, "Version"]))) @@ -24,24 +32,32 @@ which((is.na(foo[, "Version"]))) # arm64 -foo = available.packages("https://cran.devxy.io/arm64/jammy/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/arm64/jammy/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/arm64/noble/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/arm64/noble/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/arm64/rhel8/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/arm64/rhel8/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages("https://cran.devxy.io/arm64/rhel9/latest/src/contrib") +foo <- available.packages( + "https://cran.devxy.io/arm64/rhel9/latest/src/contrib" +) sum((is.na(foo[, "Version"]))) which((is.na(foo[, "Version"]))) -foo = available.packages( +foo <- available.packages( "https://cran.devxy.io/arm64/alpine320/latest/src/contrib" ) sum((is.na(foo[, "Version"]))) diff --git a/local/debug-packages-writing.R b/local/debug-packages-writing.R index 533ae2b..195c024 100644 --- a/local/debug-packages-writing.R +++ b/local/debug-packages-writing.R @@ -1,7 +1,7 @@ library(future) future::plan(multisession) future::plan(sequential) -time = Sys.time() +time <- Sys.time() files <- s3fs::s3_dir_ls( sprintf( "devxy-r-package-binaries-hel1/arm64/rhel9/latest/src/contrib/Archive" @@ -54,8 +54,8 @@ curl::curl_download( "https://cran.devxy.io/amd64/rhel9/latest/src/contrib/PACKAGES.db", "PACKAGES.db" ) -con = DBI::dbConnect(RSQLite::SQLite(), "PACKAGES.db") -df = DBI::dbReadTable(con, "packages") +con <- DBI::dbConnect(RSQLite::SQLite(), "PACKAGES.db") +df <- DBI::dbReadTable(con, "packages") cranlike:::write_packages_files("/tmp", "PACKAGES.db") @@ -69,11 +69,11 @@ system("cat /tmp/PACKAGES | grep '^Package: digest' | wc -l") curl::curl_fetch_memory( "https://cloud.r-project.org/src/contrib/Meta/archive.rds" ) -remotes = readRDS(url( +remotes <- readRDS(url( "https://cloud.r-project.org/src/contrib/Meta/archive.rds", "rb" )) -packages = readRDS(url( +packages <- readRDS(url( "https://cran.devxy.io/arm64/noble/latest/src/contrib/PACKAGES.rds", "rb" )) diff --git a/local/missing-cran-packages.R b/local/missing-cran-packages.R index ca5cad8..3021faa 100644 --- a/local/missing-cran-packages.R +++ b/local/missing-cran-packages.R @@ -42,14 +42,14 @@ files <- s3fs::s3_dir_ls(sprintf( "devxy-r-package-binaries-hel1/arm64/alpine320/latest/src/contrib" )) -files_b = basename(files) -files_b_split = sapply(strsplit(basename(files_b), "_"), function(x) x[1]) +files_b <- basename(files) +files_b_split <- sapply(strsplit(basename(files_b), "_"), function(x) x[1]) cran_pkgs <- tools::CRAN_package_db() |> filter(`Date/Publication` <= "2025-01-25") |> pull(Package) -pkgs = setdiff(cran_pkgs, files_b_split) +pkgs <- setdiff(cran_pkgs, files_b_split) formatted_pkgs <- gsub( '"', "'", diff --git a/local/missing-packages-in-index.R b/local/missing-packages-in-index.R index 6453888..0872b57 100644 --- a/local/missing-packages-in-index.R +++ b/local/missing-packages-in-index.R @@ -39,14 +39,14 @@ files <- s3fs::s3_dir_ls(sprintf( "devxy-r-package-binaries-hel1/arm64/alpine320/latest/src/contrib" )) -files_b = basename(files) -files_b_split = sapply(strsplit(basename(files_b), "_"), function(x) x[1]) -files_b_split = setdiff( +files_b <- basename(files) +files_b_split <- sapply(strsplit(basename(files_b), "_"), function(x) x[1]) +files_b_split <- setdiff( files_b_split, c("PACKAGES", "PACKAGES.gz", "PACKAGES.rds", "PACKAGES.db", "Archive") ) # windows-only -files_b_split = setdiff( +files_b_split <- setdiff( files_b_split, c( "PACKAGES", @@ -67,12 +67,12 @@ files_b_split = setdiff( "R2wd" ) ) -pkgs_index = available.packages( +pkgs_index <- available.packages( "https://cran.devxy.io/amd64/jammy/latest/src/contrib" )[, "Package"] -pkgs_index_b = basename(pkgs_index) +pkgs_index_b <- basename(pkgs_index) if (length(files_b_split) != length(pkgs_index_b)) { - pkgs_missing = setdiff(files_b_split, pkgs_index_b) + pkgs_missing <- setdiff(files_b_split, pkgs_index_b) message("Packages missing in index but present in S3:") pkgs_missing formatted_pkgs <- gsub( diff --git a/plans/2026-06-30-package-patching-implementation.md b/plans/2026-06-30-package-patching-implementation.md new file mode 100644 index 0000000..b01ddc1 --- /dev/null +++ b/plans/2026-06-30-package-patching-implementation.md @@ -0,0 +1,1506 @@ +# Package Patching Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a curated registry of packages be "patched" (env/configure/Makevars overrides and/or source diffs) before `pak` installs them, including when they are transitive dependencies, so compiler-/OS-specific failures like `RcppParallel` stop cascading. + +**Architecture:** The mechanism lives in `bincraft`. Before `pak::local_install_deps()` runs, bincraft pre-builds each registry-matched package as a binary (reusing `pkgbuild::build(binary=TRUE)` + `cranlike::add_PACKAGES`), caches it, and serves it from a local `file://` repo prepended to `options("repos")` so `pak` installs the patched binary — direct or transitive — without recompiling. The curated registry (`registry.json` + diff files) lives in this repo and is passed in via a new `patches` argument. + +**Tech Stack:** R, `pak`, `pkgbuild`, `cranlike` (fork `pat-s/cranlike@s3`), `pkgsearch`, `withr`, `jsonlite`, `testthat` (3e), `mockery`. Two repos: `bincraft` at `/Users/pjs/git/codefloe.com/rpkgs/bincraftr`, and this repo `build-cran-binaries`. + +## Global Constraints + +- bincraft package name is `bincraft`; repo dir is `…/rpkgs/bincraftr`. Current version `4.2.2.9999`. +- Logging uses bincraft's wrappers only: `log_info`, `log_warn`, `log_success`, `log_debug`, `log_error`, `log_header` (never bare `message`/`cat` in package code). +- `cli`-style inline markup is allowed in log messages (e.g. `{.pkg %s}`, `{.path %s}`); curly braces in dynamic/error text must be escaped as already done in `install_helpers.R`. +- Tests: `testthat` 3rd edition, files at `tests/testthat/test-.R`, network/build tests guarded with `skip_on_cran()` / `skip_if_offline()`; end-to-end build tests guarded behind `skip_if_not(nzchar(Sys.getenv("BINCRAFT_PATCH_E2E")))`. +- New exported functions need roxygen with `@keywords internal` for non-user helpers; run `devtools::document()` after adding roxygen. +- Patched-binary cache dir default: `/mnt/cache/patched-binaries`. +- Platform tokens for matching = `c(, , )`, e.g. `ubuntu-2604` → `c("ubuntu-2604","ubuntu","amd64")`. +- Registry entry required fields: `package`, `versions`, `platforms`, `reason`. Optional: `env`, `configure_args`, `makevars`, `patch`. +- One sentence per line in prose/commit messages; do not hard-wrap at 80 columns. +- Use `fj -H codefloe.com` for any PR operations (Forgejo), not `gh`. + +--- + +## Phase A — bincraft mechanism + +All Phase A paths are relative to `/Users/pjs/git/codefloe.com/rpkgs/bincraftr`. + +### Task A0: Proof of mechanism — pak installs a patched binary from a prepended `file://` repo + +This de-risks the core assumption before building anything on top: that `pak` installs a binary from a local `file://` repo in preference to CRAN for an equal version, and does so without recompiling. If this fails, the contingency (documented in Step 4) is to serve patched _source_ and rely on `pkgcache` build-caching — the rest of the plan changes only inside `build_patched_binary()`. + +**Files:** + +- Create: `tools/verify-patch-mechanism.R` + +**Interfaces:** + +- Produces: a runnable script proving `pak::pkg_install()` resolves a local patched binary over CRAN. No package API. + +- [ ] **Step 1: Write the verification script** + +```r +# tools/verify-patch-mechanism.R +# Proves pak installs a patched binary from a prepended file:// repo instead of +# CRAN's, without recompiling. Run inside a Linux build-env container: +# Rscript tools/verify-patch-mechanism.R +# Exits 0 on success, 1 on failure. + +pkg <- "glue" # small, pure-R CRAN package +sentinel <- "PatchMechanismProof" + +work <- tempfile("verify_") +repo <- file.path(work, "repo", "src", "contrib") +lib <- file.path(work, "lib") +dir.create(repo, recursive = TRUE) +dir.create(lib, recursive = TRUE) + +# 1. Download CRAN source for the current version. +ap <- available.packages(repos = "https://cloud.r-project.org") +ver <- ap[pkg, "Version"] +src <- file.path(work, sprintf("%s_%s.tar.gz", pkg, ver)) +download.file( + sprintf("https://cloud.r-project.org/src/contrib/%s_%s.tar.gz", pkg, ver), + src, mode = "wb" +) + +# 2. Unpack, inject a sentinel field into DESCRIPTION, build a binary. +untar(src, exdir = work) +desc <- file.path(work, pkg, "DESCRIPTION") +writeLines(c(readLines(desc), sprintf("%s: yes", sentinel)), desc) +pkgbuild::build( + file.path(work, pkg), binary = TRUE, vignettes = FALSE, + dest_path = repo, quiet = TRUE +) +built <- list.files(repo, pattern = sprintf("^%s_.*\\.tar\\.gz$", pkg), full.names = TRUE) +file.rename(built[1L], file.path(repo, sprintf("%s_%s.tar.gz", pkg, ver))) +cranlike::add_PACKAGES(sprintf("%s_%s.tar.gz", pkg, ver), repo) + +# 3. Install with the local repo prepended; assert our patched build won. +withr::with_options( + list(repos = c(patched = sprintf("file://%s", dirname(dirname(repo))), + CRAN = "https://cloud.r-project.org")), + pak::pkg_install(pkg, lib = lib, ask = FALSE, upgrade = FALSE) +) + +installed_desc <- file.path(lib, pkg, "DESCRIPTION") +ok <- file.exists(installed_desc) && + any(grepl(sentinel, readLines(installed_desc))) + +if (ok) { + cat("PROOF PASSED: pak installed the patched local binary.\n") + quit(status = 0L) +} else { + cat("PROOF FAILED: pak did not install the patched local binary.\n") + quit(status = 1L) +} +``` + +- [ ] **Step 2: Run the proof in a build-env container** + +Run (amd64 example; use any supported build-env image): + +```bash +just build-single ubuntu 2604 amd64 4.5.0 glue 1.0.0 1 || true # warms the env +docker run --rm -v "$PWD":/work -w /work reg.devxy.io/rpkgs/build-env-ubuntu:2604 \ + Rscript tools/verify-patch-mechanism.R +``` + +Expected: final line `PROOF PASSED: pak installed the patched local binary.` and exit status 0. + +- [ ] **Step 3: Commit** + +```bash +git add tools/verify-patch-mechanism.R +git commit -m "test(patches): prove pak installs a patched binary from a local file:// repo" +``` + +- [ ] **Step 4: Record the outcome / contingency** + +If the proof PASSED, proceed to Task A1 unchanged. +If it FAILED (pak recompiled or picked CRAN's), the mechanism switches to serving patched _source_: in Task A4 `build_patched_binary()` skips `pkgbuild::build()` and instead repackages the patched source tree with `pkgbuild::build(binary = FALSE)`; everything else (registry, matching, cache, repo prepend) is unchanged because `pak` build-caches the compiled result via `pkgcache`. Note the chosen path in the commit message and continue. + +--- + +### Task A1: Registry loading and normalization + +**Files:** + +- Create: `R/patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Produces: `load_patch_registry(patches_dir)` → `list()` of normalized entries; each entry is a named list with `package`, `versions`, `platforms` (character vector), `env` (named list), `configure_args` (character), `makevars` (named list), `reason`, and `patch_path` (absolute path or `NULL`). `normalize_patch_entry(entry, patches_dir)` → one normalized entry; errors on missing required field or missing patch file. + +- [ ] **Step 1: Write the failing test** + +```r +# tests/testthat/test-patches.R +test_that("load_patch_registry parses and normalizes entries", { + dir <- withr::local_tempdir() + writeLines("--- a patch ---", file.path(dir, "fix.patch")) + jsonlite::write_json( + list(list( + package = "RcppParallel", versions = "*", + platforms = list("alpine", "ubuntu-2604"), + env = list(RCPP_PARALLEL_USE_TBB = "0"), + patch = "fix.patch", reason = "bundled TBB fails" + )), + file.path(dir, "registry.json"), auto_unbox = TRUE + ) + + reg <- load_patch_registry(dir) + + expect_length(reg, 1L) + expect_identical(reg[[1L]]$package, "RcppParallel") + expect_identical(reg[[1L]]$platforms, c("alpine", "ubuntu-2604")) + expect_identical(reg[[1L]]$env$RCPP_PARALLEL_USE_TBB, "0") + expect_identical(reg[[1L]]$configure_args, character(0L)) + expect_true(file.exists(reg[[1L]]$patch_path)) +}) + +test_that("load_patch_registry returns empty list when no registry", { + expect_identical(load_patch_registry(NULL), list()) + expect_identical(load_patch_registry(withr::local_tempdir()), list()) +}) + +test_that("normalize_patch_entry errors on missing required field", { + expect_error( + normalize_patch_entry(list(package = "x"), tempdir()), + "missing required field" + ) +}) + +test_that("normalize_patch_entry errors on missing patch file", { + expect_error( + normalize_patch_entry( + list(package = "x", versions = "*", platforms = "alpine", + reason = "r", patch = "nope.patch"), + tempdir() + ), + "does not exist" + ) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL with "could not find function load_patch_registry". + +- [ ] **Step 3: Write minimal implementation** + +```r +# R/patches.R + +#' Load and validate the patch registry +#' +#' Reads `registry.json` from `patches_dir` and returns normalized entries. +#' +#' @param patches_dir Directory containing `registry.json` and any patch files, +#' or `NULL` to disable patching. +#' @return A list of normalized patch entries (possibly empty). +#' @keywords internal +load_patch_registry <- function(patches_dir) { + if (is.null(patches_dir)) { + return(list()) + } + registry_file <- file.path(patches_dir, "registry.json") + if (!file.exists(registry_file)) { + log_warn(sprintf( + "Patch directory {.path %s} has no registry.json; patching disabled.", + patches_dir + )) + return(list()) + } + raw <- jsonlite::fromJSON(registry_file, simplifyVector = FALSE) + lapply(raw, normalize_patch_entry, patches_dir = patches_dir) +} + +#' Normalize and validate a single patch registry entry +#' +#' @param entry A list parsed from `registry.json`. +#' @param patches_dir Directory used to resolve a relative `patch` path. +#' @return The entry with defaults filled and `patch_path` resolved. +#' @keywords internal +normalize_patch_entry <- function(entry, patches_dir) { + required <- c("package", "versions", "platforms", "reason") + missing <- setdiff(required, names(entry)) + if (length(missing) > 0L) { + stop( + sprintf("Patch entry is missing required field(s): %s", toString(missing)), + call. = FALSE + ) + } + entry$platforms <- as.character(unlist(entry$platforms)) + entry$env <- if (is.null(entry$env)) list() else entry$env + entry$configure_args <- if (is.null(entry$configure_args)) { + character(0L) + } else { + as.character(unlist(entry$configure_args)) + } + entry$makevars <- if (is.null(entry$makevars)) list() else entry$makevars + if (!is.null(entry$patch)) { + patch_path <- file.path(patches_dir, entry$patch) + if (!file.exists(patch_path)) { + stop( + sprintf( + "Patch file '%s' for package '%s' does not exist.", + patch_path, entry$package + ), + call. = FALSE + ) + } + entry$patch_path <- patch_path + } else { + entry$patch_path <- NULL + } + entry +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS (4 tests). + +- [ ] **Step 5: Commit** + +```bash +git add R/patches.R tests/testthat/test-patches.R +git commit -m "feat(patches): load and validate the patch registry" +``` + +--- + +### Task A2: Platform matching and version-constraint satisfaction + +**Files:** + +- Modify: `R/patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Consumes: normalized entries from Task A1. +- Produces: `build_platform_tokens(platform, arch)` → character vector; `entry_matches_platform(entry, tokens)` → logical; `match_patch_entries(registry, platform, arch)` → filtered list; `version_satisfies(version, constraint)` → logical (constraint forms: `"*"` handled by caller, `"x.y.z"` exact, `">=x"`, `"<=x"`, `">x"`, `"=5.1.0")) + expect_false(version_satisfies("5.0.0", ">=5.1.0")) + expect_true(version_satisfies("5.1.11-2", "<=5.1.11-2")) + expect_false(version_satisfies("5.1.12", "<=5.1.11-2")) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL with "could not find function build_platform_tokens". + +- [ ] **Step 3: Write minimal implementation (append to `R/patches.R`)** + +```r +#' Build platform tokens for patch matching +#' @keywords internal +build_platform_tokens <- function(platform, arch) { + family <- sub("-.*$", "", platform) + unique(c(platform, family, arch)) +} + +#' Does a patch entry apply to the current platform tokens? +#' @keywords internal +entry_matches_platform <- function(entry, tokens) { + any(entry$platforms == "*") || + length(intersect(entry$platforms, tokens)) > 0L +} + +#' Filter registry entries applicable to the current build +#' @keywords internal +match_patch_entries <- function(registry, platform, arch) { + if (length(registry) == 0L) { + return(list()) + } + tokens <- build_platform_tokens(platform, arch) + Filter(function(e) entry_matches_platform(e, tokens), registry) +} + +#' Test whether a version satisfies a single constraint +#' +#' @param version A version string (CRAN style, may contain `-`). +#' @param constraint One of `"x.y.z"`, `"==x"`, `">=x"`, `"<=x"`, `">x"`, `"=|<=|==|>|<)?\\s*(.+)$", constraint) + )[[1L]] + op <- parts[2L] + target <- parts[3L] + v <- package_version(version) + t <- package_version(target) + if (op == "" || op == "==") { + return(v == t) + } + switch( + op, + ">=" = v >= t, + "<=" = v <= t, + ">" = v > t, + "<" = v < t, + FALSE + ) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add R/patches.R tests/testthat/test-patches.R +git commit -m "feat(patches): platform matching and version-constraint checks" +``` + +--- + +### Task A3: Cache key and version resolution + +**Files:** + +- Modify: `R/patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Consumes: normalized entries. +- Produces: `patch_cache_key(entry, version, platform, arch, r_minor)` → string `"_____"`, where `hash12` covers `env`/`configure_args`/`makevars`/patch bytes; `resolve_patch_version(entry)` → latest CRAN version satisfying `entry$versions`, or `NA_character_`; `describe_patch(entry)` → short human label. + +- [ ] **Step 1: Write the failing test** + +```r +test_that("patch_cache_key is stable and sensitive to env/patch changes", { + e1 <- list(package = "P", env = list(A = "1"), + configure_args = character(0L), makevars = list(), + patch_path = NULL) + e2 <- e1; e2$env <- list(A = "2") + + k1 <- patch_cache_key(e1, "1.0", "alpine-324", "amd64", "4.5") + expect_identical(k1, patch_cache_key(e1, "1.0", "alpine-324", "amd64", "4.5")) + expect_false(identical( + k1, patch_cache_key(e2, "1.0", "alpine-324", "amd64", "4.5") + )) + expect_match(k1, "^P_1.0_alpine-324_amd64_4.5_[0-9a-f]{12}$") +}) + +test_that("resolve_patch_version returns latest for wildcard, NA when unmet", { + local_mocked_bindings( + cran_package = function(pkg) list(Version = "5.1.12"), + .package = "pkgsearch" + ) + expect_identical( + resolve_patch_version(list(package = "RcppParallel", versions = "*")), + "5.1.12" + ) + expect_identical( + resolve_patch_version(list(package = "RcppParallel", versions = ">=9.0")), + NA_character_ + ) +}) + +test_that("describe_patch summarizes the active overrides", { + expect_match( + describe_patch(list(env = list(RCPP_PARALLEL_USE_TBB = "0"), + configure_args = character(0L), makevars = list(), + patch_path = NULL)), + "env: RCPP_PARALLEL_USE_TBB=0" + ) + expect_match( + describe_patch(list(env = list(), configure_args = character(0L), + makevars = list(), patch_path = "/x/fix.patch")), + "source patch" + ) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL with "could not find function patch_cache_key". + +- [ ] **Step 3: Write minimal implementation (append to `R/patches.R`)** + +```r +#' Compute the cache key for a patched binary +#' @keywords internal +patch_cache_key <- function(entry, version, platform, arch, r_minor) { + payload <- list( + env = entry$env, + configure_args = entry$configure_args, + makevars = entry$makevars, + patch = if (!is.null(entry$patch_path)) { + readBin(entry$patch_path, "raw", file.size(entry$patch_path)) + } else { + raw(0L) + } + ) + tmp <- tempfile() + on.exit(unlink(tmp), add = TRUE) + saveRDS(payload, tmp) + hash <- substr(unname(tools::md5sum(tmp)), 1L, 12L) + sprintf( + "%s_%s_%s_%s_%s_%s", + entry$package, version, platform, arch, r_minor, hash + ) +} + +#' Resolve the CRAN version to build for a patch entry +#' +#' Returns the latest CRAN version satisfying the entry's `versions` constraint, +#' or `NA_character_` when CRAN's latest does not satisfy it or lookup fails. +#' @keywords internal +resolve_patch_version <- function(entry) { + latest <- tryCatch( + pkgsearch::cran_package(entry$package)$Version, + error = function(e) NA_character_ + ) + if (is.na(latest)) { + return(NA_character_) + } + if (identical(entry$versions, "*") || version_satisfies(latest, entry$versions)) { + return(latest) + } + NA_character_ +} + +#' Short human label describing a patch entry's overrides +#' @keywords internal +describe_patch <- function(entry) { + bits <- character(0L) + if (length(entry$env) > 0L) { + bits <- c(bits, sprintf( + "env: %s", + paste( + names(entry$env), + unlist(entry$env), + sep = "=", collapse = "," + ) + )) + } + if (length(entry$configure_args) > 0L) { + bits <- c(bits, sprintf("configure: %s", toString(entry$configure_args))) + } + if (length(entry$makevars) > 0L) { + bits <- c(bits, "makevars") + } + if (!is.null(entry$patch_path)) { + bits <- c(bits, "source patch") + } + if (length(bits) == 0L) "no-op" else paste(bits, collapse = "; ") +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS. (Note: `local_mocked_bindings` requires testthat >= 3.1.7; bincraft uses 3e.) + +- [ ] **Step 5: Commit** + +```bash +git add R/patches.R tests/testthat/test-patches.R +git commit -m "feat(patches): cache key, version resolution, and patch description" +``` + +--- + +### Task A4: Build a patched binary in isolation + +**Files:** + +- Modify: `R/patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Consumes: a normalized entry, a resolved `version`, a `dest_dir`. +- Produces: `download_cran_source(package, version, dest_dir, cran)` → path or `NULL`; `apply_source_patch(patch_path, pkg_src)` → logical; `configure_args_to_build_args(configure_args)` → character; `build_patched_binary(entry, version, dest_dir)` → path to built binary tarball or `NULL`. + +- [ ] **Step 1: Write the failing test** + +```r +test_that("configure_args_to_build_args formats configure args", { + expect_identical(configure_args_to_build_args(character(0L)), character(0L)) + expect_identical( + configure_args_to_build_args(c("--with-foo", "--no-bar")), + "--configure-args=--with-foo --no-bar" + ) +}) + +test_that("apply_source_patch returns FALSE when patch does not apply", { + src <- withr::local_tempdir() + writeLines("unrelated content", file.path(src, "file.txt")) + bad_patch <- tempfile(fileext = ".patch") + writeLines(c( + "--- a/missing.txt", "+++ b/missing.txt", + "@@ -1 +1 @@", "-nope", "+nope2" + ), bad_patch) + expect_false(apply_source_patch(bad_patch, src)) +}) + +test_that("build_patched_binary returns NULL when download fails", { + local_mocked_bindings(download_cran_source = function(...) NULL) + expect_null( + build_patched_binary( + list(package = "P", env = list(), configure_args = character(0L), + makevars = list(), patch_path = NULL), + "1.0", withr::local_tempdir() + ) + ) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL with "could not find function configure_args_to_build_args". + +- [ ] **Step 3: Write minimal implementation (append to `R/patches.R`)** + +```r +#' Download a CRAN source tarball for an exact version +#' @keywords internal +download_cran_source <- function( + package, + version, + dest_dir, + cran = "https://cloud.r-project.org" +) { + fname <- sprintf("%s_%s.tar.gz", package, version) + urls <- c( + sprintf("%s/src/contrib/%s", cran, fname), + sprintf("%s/src/contrib/Archive/%s/%s", cran, package, fname) + ) + dest <- file.path(dest_dir, fname) + for (u in urls) { + ok <- tryCatch( + { + utils::download.file(u, dest, mode = "wb", quiet = TRUE) + file.exists(dest) && file.size(dest) > 0L + }, + error = function(e) FALSE + ) + if (isTRUE(ok)) { + return(dest) + } + } + log_warn(sprintf( + "Could not download CRAN source for {.pkg %s} %s.", + package, version + )) + NULL +} + +#' Apply a unified diff to an unpacked source tree +#' +#' Uses `patch -p1 --forward` so an already-applied or non-applying patch fails +#' cleanly (returns FALSE) instead of corrupting the tree. +#' @keywords internal +apply_source_patch <- function(patch_path, pkg_src) { + status <- system2( + "patch", + args = c( + "-p1", "--forward", "--batch", + "-d", shQuote(pkg_src), + "-i", shQuote(patch_path) + ), + stdout = FALSE, stderr = FALSE + ) + identical(status, 0L) +} + +#' Format configure args for `pkgbuild::build(args = ...)` +#' @keywords internal +configure_args_to_build_args <- function(configure_args) { + if (length(configure_args) == 0L) { + return(character(0L)) + } + sprintf("--configure-args=%s", paste(configure_args, collapse = " ")) +} + +#' Build a patched binary for one registry entry, in isolation +#' +#' Downloads CRAN source for `version`, applies the source patch (if any), and +#' builds a binary with the entry's env / configure / Makevars overrides scoped +#' to this build only. Returns the built tarball path, or `NULL` on any failure. +#' @keywords internal +build_patched_binary <- function(entry, version, dest_dir) { + workdir <- tempfile("patch_build_") + dir.create(workdir, recursive = TRUE, showWarnings = FALSE) + on.exit(unlink(workdir, recursive = TRUE, force = TRUE), add = TRUE) + + src_tarball <- download_cran_source(entry$package, version, workdir) + if (is.null(src_tarball)) { + return(NULL) + } + + utils::untar(src_tarball, exdir = workdir) + pkg_src <- file.path(workdir, entry$package) + + if (!is.null(entry$patch_path)) { + if (!apply_source_patch(entry$patch_path, pkg_src)) { + log_warn(sprintf( + "Patch for {.pkg %s} %s did not apply cleanly; skipping patched build.", + entry$package, version + )) + return(NULL) + } + } + + build_env <- entry$env + if (length(entry$makevars) > 0L) { + mk <- tempfile(fileext = ".mk") + writeLines( + vapply( + names(entry$makevars), + function(k) sprintf("%s=%s", k, entry$makevars[[k]]), + character(1L) + ), + mk + ) + build_env$R_MAKEVARS_USER <- mk + } + + tryCatch( + withr::with_envvar(build_env, { + pkgbuild::build( + path = pkg_src, + binary = TRUE, + vignettes = FALSE, + dest_path = dest_dir, + args = configure_args_to_build_args(entry$configure_args), + quiet = TRUE + ) + }), + error = function(e) { + log_warn(sprintf( + "Isolated patched build of {.pkg %s} %s failed: %s", + entry$package, version, conditionMessage(e) + )) + NULL + } + ) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add R/patches.R tests/testthat/test-patches.R +git commit -m "feat(patches): build a patched binary in isolation from CRAN source" +``` + +--- + +### Task A5: Orchestrate the local patched repo (cache + index) + +**Files:** + +- Modify: `R/patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Consumes: all helpers above. +- Produces: `prepare_patched_repo(patches_dir, platform, arch, r_minor, cache_dir, repo_dir)` → path to a `src/contrib`-style dir containing patched binaries + a `PACKAGES` index, or `NULL` when nothing matched/built. On a cache hit it copies the cached tarball into `repo_dir`; on a miss it builds, then writes the result into `cache_dir`. + +- [ ] **Step 1: Write the failing test** + +```r +test_that("prepare_patched_repo returns NULL when no entries match", { + dir <- withr::local_tempdir() + jsonlite::write_json( + list(list(package = "A", versions = "*", platforms = list("redhat"), + reason = "r")), + file.path(dir, "registry.json"), auto_unbox = TRUE + ) + expect_null( + prepare_patched_repo(dir, "ubuntu-2604", "amd64", "4.5", + cache_dir = withr::local_tempdir(), + repo_dir = withr::local_tempdir()) + ) +}) + +test_that("prepare_patched_repo serves a cached binary and writes an index", { + dir <- withr::local_tempdir() + jsonlite::write_json( + list(list(package = "glue", versions = "*", platforms = list("*"), + env = list(A = "1"), reason = "r")), + file.path(dir, "registry.json"), auto_unbox = TRUE + ) + cache <- withr::local_tempdir() + repo <- withr::local_tempdir() + + local_mocked_bindings( + resolve_patch_version = function(entry) "1.0.0", + build_patched_binary = function(entry, version, dest_dir) { + f <- file.path(dest_dir, sprintf("%s_%s.tar.gz", entry$package, version)) + writeLines("fake binary", f) + f + } + ) + + out <- prepare_patched_repo(dir, "ubuntu-2604", "amd64", "4.5", + cache_dir = cache, repo_dir = repo) + + expect_identical(out, repo) + expect_true(file.exists(file.path(repo, "glue_1.0.0.tar.gz"))) + expect_true(file.exists(file.path(repo, "PACKAGES"))) + # The build result was cached under the key. + expect_length(list.files(cache, pattern = "^glue_1.0.0_.*\\.tar\\.gz$"), 1L) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL with "could not find function prepare_patched_repo". + +- [ ] **Step 3: Write minimal implementation (append to `R/patches.R`)** + +```r +#' Prepare a local repo of patched binaries for the current build +#' +#' For each registry entry matching the current platform, ensures a patched +#' binary is present in `repo_dir` (from `cache_dir` if available, else built +#' and then cached) and writes a `PACKAGES` index over them. +#' +#' @param patches_dir Directory with `registry.json`, or `NULL`. +#' @param platform Build platform, e.g. `"ubuntu-2604"`. +#' @param arch Build arch, e.g. `"amd64"`. +#' @param r_minor R `"major.minor"` string, e.g. `"4.5"`. +#' @param cache_dir Persistent cache for patched binaries. +#' @param repo_dir Directory to assemble the local repo in. +#' @return `repo_dir` if at least one patched binary was produced, else `NULL`. +#' @keywords internal +prepare_patched_repo <- function( + patches_dir, + platform, + arch, + r_minor, + cache_dir = file.path("/mnt", "cache", "patched-binaries"), + repo_dir = tempfile("patched_repo_") +) { + entries <- match_patch_entries( + load_patch_registry(patches_dir), platform, arch + ) + if (length(entries) == 0L) { + return(NULL) + } + + dir.create(repo_dir, recursive = TRUE, showWarnings = FALSE) + dir.create(cache_dir, recursive = TRUE, showWarnings = FALSE) + + produced <- 0L + for (entry in entries) { + version <- resolve_patch_version(entry) + if (is.na(version)) { + log_warn(sprintf( + "No CRAN version of {.pkg %s} satisfies '%s'; patch skipped.", + entry$package, entry$versions + )) + next + } + + key <- patch_cache_key(entry, version, platform, arch, r_minor) + cached <- file.path(cache_dir, sprintf("%s.tar.gz", key)) + target <- file.path( + repo_dir, sprintf("%s_%s.tar.gz", entry$package, version) + ) + + if (file.exists(cached)) { + log_info(sprintf( + "Using cached patched binary for {.pkg %s} %s.", + entry$package, version + )) + file.copy(cached, target, overwrite = TRUE) + } else { + log_info(sprintf( + "Applying patch to {.pkg %s} %s [%s]: %s", + entry$package, version, describe_patch(entry), entry$reason + )) + built <- build_patched_binary(entry, version, repo_dir) + if (is.null(built)) { + next + } + if (!identical(normalizePath(built), normalizePath(target))) { + file.copy(built, target, overwrite = TRUE) + } + file.copy(target, cached, overwrite = TRUE) + } + produced <- produced + 1L + } + + if (produced == 0L) { + return(NULL) + } + cranlike::add_PACKAGES( + list.files(repo_dir, pattern = "\\.tar\\.gz$"), + repo_dir + ) + repo_dir +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS. + +- [ ] **Step 5: Update imports and document** + +Add to `DESCRIPTION` `Imports:` (alphabetical) `jsonlite` if not present, and ensure `pkgsearch`, `pkgbuild`, `cranlike`, `withr` are listed (they are). Then: + +```bash +Rscript -e 'devtools::document()' +git add R/patches.R tests/testthat/test-patches.R DESCRIPTION NAMESPACE +git commit -m "feat(patches): orchestrate local patched-binary repo with caching" +``` + +--- + +### Task A6: Wire patched repo into the pak install path + +**Files:** + +- Modify: `R/install_helpers.R:333-389` (`run_pak_install_with_mutex`) +- Modify: `R/install-deps.R:23-75` (`install_pkg_sys_deps`) +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Consumes: `prepare_patched_repo()`. +- Produces: `run_pak_install_with_mutex(local_clone_dir_single, env_vars, patched_repo = NULL)` — prepends `file://` to `options("repos")` for the install; `install_pkg_sys_deps(package_name, tag, local_clone_dir, platform, aggressive_cleanup = FALSE, patches = NULL, arch = NULL)` — builds the patched repo before installing. + +- [ ] **Step 1: Write the failing test** + +```r +test_that("run_pak_install_with_mutex prepends the patched repo to repos", { + seen <- NULL + local_mocked_bindings( + acquire_pak_mutex = function(...) tempfile(), + release_pak_mutex = function(...) invisible(NULL), + retry_with_backoff = function(func, ...) func() + ) + local_mocked_bindings( + local_install_deps = function(...) { + seen <<- getOption("repos") + invisible(TRUE) + }, + .package = "pak" + ) + + run_pak_install_with_mutex( + tempfile(), list(), patched_repo = "/tmp/patched" + ) + + expect_true(any(grepl("file:///tmp/patched", seen))) +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL — `patched_repo` argument not yet accepted / repos not prepended. + +- [ ] **Step 3: Edit `run_pak_install_with_mutex` in `R/install_helpers.R`** + +Change the signature line: + +```r +run_pak_install_with_mutex <- function(local_clone_dir_single, env_vars) { +``` + +to: + +```r +run_pak_install_with_mutex <- function( + local_clone_dir_single, + env_vars, + patched_repo = NULL +) { +``` + +Replace the inner `retry_with_backoff(...)` block (the one wrapping `pak::local_install_deps`) with: + +```r + retry_with_backoff(function() { + withr::with_envvar(env_vars, { + repos <- getOption("repos") + if (!is.null(patched_repo)) { + repos <- c( + patched = sprintf("file://%s", patched_repo), + repos + ) + } + withr::with_options(list(repos = repos), { + # Default to non-verbose (suppressed messages) + suppressMessages(pak::local_install_deps(sprintf( + "%s", + local_clone_dir_single + ))) + }) + }) + }) +``` + +- [ ] **Step 4: Edit `install_pkg_sys_deps` in `R/install-deps.R`** + +Change the signature to add `patches` and `arch`: + +```r +install_pkg_sys_deps <- function( + package_name, + tag, + local_clone_dir, + platform = platform, + aggressive_cleanup = FALSE, + patches = NULL, + arch = NULL +) { +``` + +Immediately before the `run_pak_install_with_mutex(...)` call, insert: + +```r + # Build a local repo of patched binaries (if any apply) and serve it to pak. + r_minor <- paste( + R.version$major, + strsplit(R.version$minor, ".", fixed = TRUE)[[1L]][1L], + sep = "." + ) + patched_repo <- tryCatch( + prepare_patched_repo(patches, platform, arch, r_minor), + error = function(e) { + log_warn(sprintf("Patch preparation failed: %s", conditionMessage(e))) + NULL + } + ) +``` + +and change the call from: + +```r + run_pak_install_with_mutex( + local_clone_dir_single, + env_vars + ) +``` + +to: + +```r + run_pak_install_with_mutex( + local_clone_dir_single, + env_vars, + patched_repo = patched_repo + ) +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add R/install_helpers.R R/install-deps.R tests/testthat/test-patches.R +git commit -m "feat(patches): serve patched binaries to pak during dep install" +``` + +--- + +### Task A7: Thread `patches` through the public build API + +**Files:** + +- Modify: `R/build_binaries.R` (`build_binary_package`, `execute_package_builds`, `build_single_tag`, `handle_system_dependencies`) +- Create: `man-roxygen/param-patches.R` +- Test: `tests/testthat/test-patches.R` + +**Interfaces:** + +- Produces: `build_binary_package(..., patches = NULL)` and the internal chain each carry `patches` down to `install_pkg_sys_deps()`. `handle_system_dependencies(..., patches = NULL)` passes `patches` and `arch` through. + +- [ ] **Step 1: Write the failing test** + +```r +test_that("handle_system_dependencies forwards patches and arch", { + captured <- list() + local_mocked_bindings( + install_pkg_sys_deps = function(package_name, tag, local_clone_dir_single, + platform, patches = NULL, arch = NULL) { + captured <<- list(patches = patches, arch = arch) + invisible(TRUE) + } + ) + handle_system_dependencies( + "RcppParallel", "5.1.11-2", "ubuntu-2604", tempfile(), "amd64", + NULL, NULL, NULL, NULL, NULL, NULL, NULL, + patches = "local/patches" + ) + expect_identical(captured$patches, "local/patches") + expect_identical(captured$arch, "amd64") +}) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")'` +Expected: FAIL — `handle_system_dependencies` has no `patches` argument. + +- [ ] **Step 3: Create the roxygen template** + +```r +# man-roxygen/param-patches.R +#' @param patches Optional path to a patch registry directory containing a +#' `registry.json` (and any referenced diff files). When set, matching +#' packages are pre-built as patched binaries and served to `pak` during +#' dependency installation. Defaults to `NULL` (no patching). +``` + +- [ ] **Step 4: Edit the four functions in `R/build_binaries.R`** + +In `build_single_tag()`'s call to `handle_system_dependencies(...)`, add `patches = patches` as the final argument, and add `patches = NULL` to `build_single_tag`'s own signature plus `#' @template param-patches` to its roxygen block. + +Change `handle_system_dependencies` signature to end with `metadata_db_sslmode,` then add `patches = NULL`, and change its inner `install_pkg_sys_deps(...)` call from: + +```r + install_pkg_sys_deps( + package_name, + tag, + local_clone_dir_single, + platform + ) +``` + +to: + +```r + install_pkg_sys_deps( + package_name, + tag, + local_clone_dir_single, + platform, + patches = patches, + arch = arch + ) +``` + +In `execute_package_builds()`, add `patches = NULL` to the signature and pass `patches = patches` into its `build_single_tag(...)` call inside `worker_function`. + +In `build_binary_package()`, add `patches = NULL` to the signature (after `s3_package_cache`), add `#' @template param-patches` to its roxygen, and pass `patches = patches` into the `execute_package_builds(...)` call. + +- [ ] **Step 5: Document, test, and run package check** + +Run: + +```bash +Rscript -e 'devtools::document()' +Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")' +``` + +Expected: PASS for the new test; `man/build_binary_package.Rd` etc. regenerated. + +- [ ] **Step 6: Commit** + +```bash +git add R/build_binaries.R man-roxygen/param-patches.R man NAMESPACE tests/testthat/test-patches.R +git commit -m "feat(patches): thread patches argument through build_binary_package" +``` + +--- + +### Task A8: End-to-end patch test (guarded) and version bump + +**Files:** + +- Modify: `tests/testthat/test-patches.R` +- Modify: `DESCRIPTION` (version), `NEWS.md` + +**Interfaces:** + +- Produces: a guarded e2e test proving a dependent package builds when its failing dependency is patched. + +- [ ] **Step 1: Add the guarded e2e test** + +```r +test_that("a patched dependency unblocks a dependent build (e2e)", { + skip_if_not(nzchar(Sys.getenv("BINCRAFT_PATCH_E2E"))) + skip_if_offline() + + patches_dir <- withr::local_tempdir() + jsonlite::write_json( + list(list( + package = "RcppParallel", versions = "*", platforms = list("*"), + env = list(RCPP_PARALLEL_USE_TBB = "0"), + reason = "bundled TBB fails on this toolchain" + )), + file.path(patches_dir, "registry.json"), auto_unbox = TRUE + ) + + out <- withr::local_tempdir() + result <- build_binary_package( + "rts2", tag = "latest", local_output_dir_root = out, + upload = FALSE, archive = FALSE, patches = patches_dir + ) + expect_true(isTRUE(result) || identical(result, "skipped")) +}) +``` + +- [ ] **Step 2: Run the e2e test in a container** + +Run: + +```bash +docker run --rm -e BINCRAFT_PATCH_E2E=1 -v "$PWD":/work -w /work \ + reg.devxy.io/rpkgs/build-env-ubuntu:2604 \ + Rscript -e 'devtools::load_all("."); testthat::test_file("tests/testthat/test-patches.R")' +``` + +Expected: the e2e test runs (not skipped) and PASSES; build log shows `Applying patch to RcppParallel`. + +- [ ] **Step 3: Bump version and changelog** + +In `DESCRIPTION`, bump `Version:` to `4.3.0.9999`. Prepend to `NEWS.md`: + +```markdown +# bincraft 4.3.0 + +- `build_binary_package()` gains a `patches` argument: a registry of + per-package env / configure / Makevars overrides and source diffs that are + pre-built into patched binaries and served to pak, fixing compiler- and + OS-specific failures (e.g. RcppParallel) including for transitive deps. +``` + +- [ ] **Step 4: Commit** + +```bash +git add tests/testthat/test-patches.R DESCRIPTION NEWS.md +git commit -m "test(patches): guarded end-to-end test; bump to 4.3.0.9999" +``` + +--- + +## Phase B — build-cran-binaries registry and wiring + +All Phase B paths are relative to this repo (`build-cran-binaries`). Phase B depends on a bincraft build that includes Phase A (install the updated bincraft in the build containers, or bump the pinned version in `.crow/build-all-versions-install-deps.yaml`). + +### Task B1: Create the patch registry with the RcppParallel entry + +**Files:** + +- Create: `local/patches/registry.json` +- Create: `local/patches/README.md` + +**Interfaces:** + +- Produces: the curated registry consumed by bincraft's `patches` argument. + +- [ ] **Step 1: Write the registry** + +```json +[ + { + "package": "RcppParallel", + "versions": "*", + "platforms": ["alpine", "ubuntu-2604"], + "env": { "RCPP_PARALLEL_USE_TBB": "0" }, + "configure_args": [], + "makevars": {}, + "patch": null, + "reason": "bundled Intel TBB fails to build on musl and on newer toolchains (e.g. g++ 15 on ubuntu-2604); disabling TBB falls back to TinyThread" + } +] +``` + +- [ ] **Step 2: Write the README** + +`local/patches/README.md` documents the schema (copy the field table from `specs/2026-06-30-package-patching-design.md`), how to add an entry, and that source diffs go in `local/patches//.patch` referenced by the `patch` field. + +- [ ] **Step 3: Verify it parses** + +Run: `Rscript -e 'x <- jsonlite::fromJSON("local/patches/registry.json", simplifyVector = FALSE); stopifnot(length(x) == 1L, x[[1]]$package == "RcppParallel"); cat("ok\n")'` +Expected: `ok`. + +- [ ] **Step 4: Commit** + +```bash +git add local/patches/registry.json local/patches/README.md +git commit -m "feat(patches): add patch registry with RcppParallel TBB workaround" +``` + +--- + +### Task B2: Registry validator script + +**Files:** + +- Create: `local/validate-patches.R` + +**Interfaces:** + +- Consumes: `local/patches/registry.json`. +- Produces: a script that exits non-zero on schema violations, missing patch files, or ambiguous overlapping entries. + +- [ ] **Step 1: Write the validator** + +```r +#!/usr/bin/env Rscript +# Validate local/patches/registry.json: schema, referenced patch files, and +# ambiguous overlaps. Exits 1 on any problem. Used by pre-commit and CI. + +dir <- "local/patches" +registry_file <- file.path(dir, "registry.json") +if (!file.exists(registry_file)) { + cat("No registry.json found; nothing to validate.\n") + quit(status = 0L) +} + +reg <- jsonlite::fromJSON(registry_file, simplifyVector = FALSE) +required <- c("package", "versions", "platforms", "reason") +errs <- character(0L) + +for (i in seq_along(reg)) { + e <- reg[[i]] + missing <- setdiff(required, names(e)) + if (length(missing) > 0L) { + errs <- c(errs, sprintf( + "entry %d (%s): missing %s", i, + if (is.null(e$package)) "?" else e$package, toString(missing) + )) + } + if (!is.null(e$patch)) { + p <- file.path(dir, e$patch) + if (!file.exists(p)) { + errs <- c(errs, sprintf("entry %d (%s): patch file '%s' missing", + i, e$package, p)) + } + } +} + +# Ambiguous overlap: two entries for the same package with identical platforms +# and versions. +keys <- vapply(reg, function(e) { + sprintf("%s|%s|%s", e$package, + paste(sort(as.character(unlist(e$platforms))), collapse = ","), + e$versions) +}, character(1L)) +dups <- keys[duplicated(keys)] +if (length(dups) > 0L) { + errs <- c(errs, sprintf("ambiguous duplicate entries: %s", toString(unique(dups)))) +} + +if (length(errs) > 0L) { + cat("Patch registry validation FAILED:\n") + cat(paste0(" - ", errs, "\n")) + quit(status = 1L) +} +cat(sprintf("Patch registry OK (%d entrie(s)).\n", length(reg))) +``` + +- [ ] **Step 2: Run it (expect success on the B1 registry)** + +Run: `Rscript local/validate-patches.R` +Expected: `Patch registry OK (1 entrie(s)).` and exit 0. + +- [ ] **Step 3: Run it against a broken registry (expect failure)** + +Run: + +```bash +cp local/patches/registry.json /tmp/reg.bak +Rscript -e 'writeLines("[{\"package\":\"X\"}]", "local/patches/registry.json")' +Rscript local/validate-patches.R; echo "exit=$?" +cp /tmp/reg.bak local/patches/registry.json +``` + +Expected: prints `validation FAILED` with a missing-field message and `exit=1`. + +- [ ] **Step 4: Commit** + +```bash +git add local/validate-patches.R +git commit -m "feat(patches): add registry validator script" +``` + +--- + +### Task B3: Hook the validator into pre-commit + +**Files:** + +- Modify: `.pre-commit-config.yaml` + +**Interfaces:** + +- Produces: a local hook that runs `local/validate-patches.R` when the registry or patch files change. + +- [ ] **Step 1: Add the hook** + +Add a `repo: local` hook entry to `.pre-commit-config.yaml`: + +```yaml +- repo: local + hooks: + - id: validate-patches + name: validate patch registry + entry: Rscript local/validate-patches.R + language: system + files: ^local/patches/ + pass_filenames: false +``` + +- [ ] **Step 2: Verify the hook runs** + +Run: `pre-commit run validate-patches --all-files` +Expected: hook passes (`Patch registry OK`). + +- [ ] **Step 3: Commit** + +```bash +git add .pre-commit-config.yaml +git commit -m "ci(patches): validate patch registry in pre-commit" +``` + +--- + +### Task B4: Pass `patches` through the build entry points + +**Files:** + +- Modify: `local/build-one.R:99-119` (the `build_binary_package` call) +- Modify: `local/build-all.R:125-145` (the `build_binary_package` call) + +**Interfaces:** + +- Consumes: the new bincraft `patches` argument (Phase A) and `local/patches/`. +- Produces: both entry points pass `patches = "local/patches"`. + +- [ ] **Step 1: Edit `local/build-one.R`** + +In the `bincraft::build_binary_package(` call, add as a new argument (e.g. after `archive = TRUE,`): + +```r + patches = "local/patches", +``` + +- [ ] **Step 2: Edit `local/build-all.R`** + +In the `bincraft::build_binary_package(` call, add: + +```r + patches = "local/patches", +``` + +- [ ] **Step 3: Verify the scripts still parse** + +Run: `Rscript -e 'invisible(parse("local/build-one.R")); invisible(parse("local/build-all.R")); cat("parse ok\n")'` +Expected: `parse ok`. + +- [ ] **Step 4: Commit** + +```bash +git add local/build-one.R local/build-all.R +git commit -m "feat(patches): pass patch registry to bincraft build calls" +``` + +--- + +### Task B5: Document the feature in the README + +**Files:** + +- Modify: `README.md` + +**Interfaces:** + +- Produces: a short "Patching packages" section explaining the registry and linking the design spec. + +- [ ] **Step 1: Add a README section** + +Add a `## Patching packages` section after the "Build Process" section describing: why patching exists (compiler/OS-specific failures cascading via shared deps like RcppParallel), where the registry lives (`local/patches/registry.json`), the two tiers (env/configure/Makevars overrides vs source diffs), and that bincraft pre-builds patched binaries served to pak. Link `specs/2026-06-30-package-patching-design.md`. + +- [ ] **Step 2: Commit** + +```bash +git add README.md +git commit -m "docs(patches): document the package patching workflow" +``` + +--- + +## Self-Review + +**Spec coverage:** + +- Registry in this repo, passed to bincraft → Task A7 (`patches` arg), B1 (registry), B4 (wiring). ✓ +- Mechanism in bincraft (resolve → pre-build → prepend repo → install) → A4–A6. ✓ +- Both tiers, env-first → A4 (`env`, `configure_args`, `makevars`, `patch` all applied; env is the cheap default and the RcppParallel entry uses only env). ✓ +- Pre-built patched binary served from a prepended local repo → A0 proof + A5/A6. ✓ +- Cache keyed by pkg/version/platform/arch/rminor/patchhash → A3 (`patch_cache_key`) + A5 (cache use). ✓ +- S3 upload of patched binaries → **partial**: A5 caches locally only. Cross-machine S3 reuse is deferred (see note below) to keep the first cut shippable; local `/mnt/cache` reuse already removes per-dependent rebuilds within a container. Recorded as out-of-first-cut, not dropped. +- Error handling (diff fails → skip+warn; build fails → skip+warn; no match → skip; ambiguous overlap → validation error) → A4 (`apply_source_patch` FALSE path, build tryCatch), A5 (`resolve_patch_version` NA path), B2 (overlap validation). ✓ +- Observability (log line per applied patch) → A5 (`log_info` with `describe_patch`). DB metadata recording of applied patches is deferred with S3 (same note). +- Testing (registry parse/match, cache key, integration, failure path) → A1–A5 unit tests, A8 e2e, A4 patch-fail test. ✓ + +**Deferred from spec (call out to user):** S3 upload/reuse of patched binaries and recording applied patches in the Postgres build-metadata row. The local `/mnt/cache/patched-binaries` cache already prevents repeated rebuilds within a container; S3 reuse across CI jobs is a follow-up. If you want it in the first cut, add a Task A5b (upload `cached` tarball to a `…/patched/` S3 slot and check there before building) and a metadata column — say so and I'll insert them. + +**Placeholder scan:** No TBD/TODO; every code step shows complete code. README/registry-README prose steps describe exact content to write (acceptable for docs). + +**Type consistency:** `prepare_patched_repo` signature is identical across A5 (definition) and A6 (call site, via defaults). `patch_cache_key(entry, version, platform, arch, r_minor)` argument order matches between A3 and A5. `run_pak_install_with_mutex(..., patched_repo)` matches between A6 definition and the A6 test. `handle_system_dependencies(..., patches)` matches between A7 edit and A7 test. `build_patched_binary(entry, version, dest_dir)` matches A4 and A5. + +**Open risk:** Task A0 gates the whole approach; if it fails, the documented source-serving contingency keeps every later task valid with a localized change in A4. diff --git a/plans/2026-08-07-per-minor-edge-routing.md b/plans/2026-08-07-per-minor-edge-routing.md new file mode 100644 index 0000000..3c3e320 --- /dev/null +++ b/plans/2026-08-07-per-minor-edge-routing.md @@ -0,0 +1,166 @@ +# Per-R-minor edge routing implementation plan + +Spec: `specs/2026-08-07-per-minor-edge-routing-design.md` + +**Goal:** let a stock `install.packages()` see the per-minor packages by routing `PACKAGES*` requests to `…/src/contrib//`, where `bincraft` publishes a union index. + +**Architecture:** the union is built in `bincraft`; the edge script only redirects index requests, gated on a `UNION_SLOTS` script variable; the script lives in this repo and is applied by OpenTofu. + +**Tech stack:** Deno / TypeScript (Bunny Edge Scripting, SDK 0.12), OpenTofu with `BunnyWay/bunnynet` 0.17, R (bincraft). + +## Global constraints + +- Redirect only `PACKAGES`, `PACKAGES.gz` and `PACKAGES.rds`; never a tarball, because the union index already carries the correct tarball URL for both classes of package. +- Every redirect carries `Cache-Control: no-store`; redirect targets stay UA-independent. +- `UNION_SLOTS` is empty by default, so deploying the script is a no-op until a slot is backfilled. +- A slot is `/`, e.g. `amd64/alpine324`. +- Verified prerequisites: `PACKAGES*` is served `cdn-cache: BYPASS`, so the script sees every index request; `Deno.env.get()` reads script variables; the SDK local server listens on `127.0.0.1:8080`. + +--- + +## Task 1: Edge script and its test matrix + +**Files:** + +- Create: `edge/rpkgs-router.ts` +- Create: `edge/rpkgs-router.test.ts` +- Modify: `justfile` (add `edge-test`) + +**Produces:** a single-file script deployable as `bunnynet_compute_script.content`, reading `UNION_SLOTS` from the environment. + +- [ ] **Step 1: write the test matrix first** + +`edge/rpkgs-router.test.ts` spawns `deno run -A edge/rpkgs-router.ts` with `UNION_SLOTS=amd64/alpine324`, waits for `127.0.0.1:8080`, and issues requests with `redirect: "manual"`. + +Cases, asserted on the `location` header (or its absence): + +| # | path | User-Agent | expectation | +| --- | ------------------------------------------------------ | --------------------------------------------- | ------------------------------------------------------------ | +| 1 | `/amd64/alpine324/latest/src/contrib/PACKAGES.gz` | `R (4.5.3 x86_64-pc-linux-musl …)` | 302 → `…/src/contrib/4.5/PACKAGES.gz` | +| 2 | same | `R (4.6.0 …)` | 302 → `…/src/contrib/4.6/PACKAGES.gz` | +| 3 | same, but slot `amd64/noble` | `R (4.5.3 …)` | no redirect (slot not in `UNION_SLOTS`) | +| 4 | `…/src/contrib/curl_7.1.0.tar.gz` | `R (4.5.3 …)` | no redirect | +| 5 | `…/src/contrib/4.5/PACKAGES.gz` | `R (4.5.3 …)` | no redirect (loop guard) | +| 6 | `…/src/contrib/PACKAGES.gz` | `curl/8.0` | no redirect (no R minor) | +| 7 | `/src/contrib/PACKAGES.gz` | alpine UA with `Alpine Linux … 3.24` | 302 → `/amd64/alpine324/latest/src/contrib/4.5/PACKAGES.gz` | +| 8 | `/src/contrib/PACKAGES.gz` | `R (4.5.3 x86_64-pc-linux-musl …)`, no distro | 302 → `cran.r-project.org`, **not** a `linux-musl` slot | +| 9 | `/src/contrib/foo_1.0.tar.gz` | `R (4.5.1 aarch64-apple-darwin20 …)` | 302 → `/bin/macosx/big-sur-arm64/contrib/4.5/foo_1.0.tar.gz` | +| 10 | `/bin/macosx/big-sur-arm64/contrib/4.5/foo_1.0.tar.gz` | any | 302 → `cran.r-project.org` | +| 11 | any redirect above | — | `cache-control: no-store` | + +- [ ] **Step 2: run the tests and watch them fail** + +`just edge-test` → every case fails, because `edge/rpkgs-router.ts` does not exist. + +- [ ] **Step 3: write `edge/rpkgs-router.ts`** + +Order of evaluation in `onOriginRequest`: + +1. normalise `//` runs in the path +2. darwin `/src/contrib/*` → `/bin/macosx//contrib//` +3. `/bin/macosx/**` → CRAN +4. `/{arch}/{os}/latest/src/contrib/`: pass through if `rest` already starts with `/`, or is not an index file, or the slot is not in `UNION_SLOTS`, or the UA has no R minor; otherwise redirect into `/` +5. `/`, `/src/contrib`, `/src/contrib/**`: resolve arch+os from the UA, redirect to CRAN when the distro is unidentifiable, otherwise redirect to the qualified path, adding `/` under the same index-file rule +6. anything else: pass through + +The R minor comes from either `R/4.5.3` or `R (4.5.3 …)`, so a stock UA is enough. The `linux-gnu` / `linux-musl` fallback in `parseUserAgent` is deleted: those are not slot names. + +- [ ] **Step 4: run the tests until they pass** + +`just edge-test` + +- [ ] **Step 5: commit** + +```bash +git add edge/rpkgs-router.ts edge/rpkgs-router.test.ts justfile +git commit -m "feat(edge): route PACKAGES requests to the per-R-minor slot" +``` + +--- + +## Task 2: Manage the script from OpenTofu + +**Files:** + +- Modify: `cdn.tf` + +**Consumes:** `edge/rpkgs-router.ts` from Task 1. + +- [ ] **Step 1: add the resources** + +```terraform +resource "bunnynet_compute_script" "rpkgs_router" { + type = "middleware" + name = "rpkgs-router" + content = file("${path.module}/edge/rpkgs-router.ts") +} + +resource "bunnynet_compute_script_variable" "rpkgs_router_union_slots" { + script = bunnynet_compute_script.rpkgs_router.id + name = "UNION_SLOTS" + default_value = "" + required = false +} +``` + +and replace `middleware_script = 29277` with `middleware_script = bunnynet_compute_script.rpkgs_router.id`. + +- [ ] **Step 2: validate** + +`tofu init -backend=false && tofu validate` + +- [ ] **Step 3: import the existing script (needs `BUNNYNET_API_KEY`)** + +```bash +tofu import bunnynet_compute_script.rpkgs_router 29277 +tofu plan +``` + +The plan must show an in-place `content` update and **no** replacement of the pull zone. A replacement means the import did not take. + +- [ ] **Step 4: commit** + +```bash +git add cdn.tf +git commit -m "feat(cdn): manage the edge middleware script from this repo" +``` + +--- + +## Task 3: Union index writer in bincraft + +**Files (repo `codefloe.com/rpkgs/bincraft`):** + +- Modify: `R/package_index.R` +- Test: `tests/testthat/test-package_index.R` + +**Produces:** `write_union_index(flat_records, minor_records)` returning the merged records, called from `upload_package_index()` when `r_minor` is set. + +- [ ] **Step 1: write the failing tests** + +- a package present in both slots keeps the per-minor record, with `Path = "4.5"` +- a package only in the flat slot survives with no `Path` +- a package only in the per-minor slot survives with `Path = "4.5"` +- a union smaller than the flat input raises an error rather than returning + +- [ ] **Step 2: run them and watch them fail** + +`Rscript -e 'testthat::test_file("tests/testthat/test-package_index.R")'` + +- [ ] **Step 3: implement `write_union_index()` and call it from `upload_package_index()`** + +After `update_PACKAGES()` has written the per-minor index, read the flat slot's `PACKAGES.rds`, set `Path = ` on the per-minor records, drop the flat records for packages the per-minor slot already has, and rewrite `PACKAGES`, `PACKAGES.gz` and `PACKAGES.rds` in the per-minor slot. + +- [ ] **Step 4: run the tests until they pass** + +- [ ] **Step 5: commit and open the PR against bincraft** + +--- + +## Task 4: Roll out slot by slot + +- [ ] Re-index one slot (`amd64/alpine324`, R 4.5) and confirm the union index lists both `curl` (per-minor, `Path: 4.5`) and `jsonlite` (flat, no `Path`). +- [ ] Set `UNION_SLOTS = "amd64/alpine324"` and confirm in `reg.devxy.io/r/r-alpine:4.5-3.24` that `available.packages()` returns the union count and `"curl" %in% rownames(...)`. +- [ ] Add `arm64/alpine324`, then the remaining slots. + +`install.packages("curl")` will still fail to build on `alpine324` until that slot's source tarballs are replaced with real binaries. That is tracked separately. diff --git a/specs/2026-06-30-package-patching-design.md b/specs/2026-06-30-package-patching-design.md new file mode 100644 index 0000000..feeae3a --- /dev/null +++ b/specs/2026-06-30-package-patching-design.md @@ -0,0 +1,157 @@ +# Design: Dynamic per-package patching during binary builds + +Date: 2026-06-30 +Status: Approved (pending spec review) + +## Problem + +Some CRAN packages fail to compile on specific build platforms due to compiler- or OS-specific issues that have nothing to do with the package being built. +The canonical example is `RcppParallel`: its bundled Intel TBB sources fail to build on musl (Alpine) and on newer OS/compiler combinations. +Observed failure on `ubuntu-2604` ("resolute") with `g++ 15.2.0`: + +``` +../build/common.inc:74: *** "" is not supported. Add build/.inc file with os-specific settings . Stop. +make: *** [Makevars:163: tbb] Error 2 +ERROR: compilation failed for package 'RcppParallel' +``` + +Because `RcppParallel` is a dependency of many packages, a single such failure cascades: every dependent package (e.g. `rts2`) also fails, even though nothing is wrong with the dependent itself. + +Today there is no way to intervene. +A package can only be **excluded** (`local/excluded-packages.json`), which is all-or-nothing and does not help dependents. + +## Goal + +Allow a curated set of packages to be "patched" — via lightweight build-time overrides or, when necessary, real source diffs — **before** they are installed, whether the package is a direct build target or a transitive dependency pulled in by `pak`. + +## Key constraint that drives the design + +When `RcppParallel` fails here, it is being installed as a **transitive dependency** by `pak`, inside `bincraft::build_binary_package()`. +`pak` downloads, configures, and compiles it in one subprocess; this repo never touches that source. +For a fix to reach a dependency-of-a-dependency, the fixed package must be visible to `pak` itself, where `pak`'s repositories/sources are configured — which is inside `bincraft`. + +Decisions taken during brainstorming: + +- **Mechanism lives in `bincraft`** (the engine), because only there can transitive deps be influenced. +- **Patch tiers: both, env-overrides first.** Support cheap per-package env vars / configure args / Makevars (version-independent) _and_ true source diffs (version-pinned), preferring the lightweight override. +- **Registry data lives in this repo** (`build-cran-binaries`) and is passed into `bincraft`, keeping `bincraft` as pure mechanism and the frequently-changing policy data with operational config. + +## Approaches considered + +| Approach | How pak sees the fix | Verdict | +| ---------------------------------------------------------------------------------------- | -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| A. Patched **source** repo — drop patched `.tar.gz` source into a local repo, prepend it | pak recompiles from your source | Simple, but env-tier overrides leak globally (one subprocess builds everything) and the dep recompiles on every dependent build | +| **B. Pre-built patched binary repo (chosen)** | pak installs a ready binary by repo priority | Per-package scoping is free; no recompile; the binary is a cacheable/uploadable artifact that fits the existing system | +| C. pkgdepends per-build hook | intercept each build | No clean per-package pre-compile hook exists; fragile | + +Chosen: **B**. + +## Architecture + +### Registry (this repo) + +``` +local/patches/ + registry.json # the manifest + RcppParallel/ + fix.patch # optional source diff, referenced by an entry +``` + +`registry.json` is an array of entries: + +```json +[ + { + "package": "RcppParallel", + "versions": "*", + "platforms": ["alpine", "ubuntu-2604"], + "env": { "RCPP_PARALLEL_USE_TBB": "0" }, + "configure_args": [], + "makevars": {}, + "patch": null, + "reason": "bundled TBB fails to build on musl / newer compilers" + } +] +``` + +Field semantics: + +- `package` (string, required): CRAN package name. +- `versions` (string, required): `"*"` for any, a constraint such as `">=5.1.0"`, or an exact version `"5.1.11-2"`. + Env-tier fixes are typically `"*"`; source diffs are normally exact or lower-bounded because a diff is pinned to the source it was generated against. +- `platforms` (array of strings, required): matched against the running build's platform tokens — distro family (`alpine`, `ubuntu`, `redhat`), codename (`ubuntu-2604`, `alpine-324`), and arch (`amd64`, `arm64`). + An entry matches if any listed token matches any build token. + `["*"]` matches all platforms. +- `env` (object, optional): environment variables exported only for this package's isolated build. +- `configure_args` (array, optional): passed as `--configure-args` to the isolated build. +- `makevars` (object, optional): key/value pairs written into a package-local Makevars for the isolated build. +- `patch` (string or null, optional): path (relative to `local/patches/`) to a unified diff applied to the unpacked CRAN source before building. +- `reason` (string, required): human explanation, surfaced in logs and metadata. + +A fix is any combination of `env`, `configure_args`, `makevars`, and `patch`. +"Env-first" is an authoring guideline (prefer the lightweight override) and an ordering of effort, not a runtime branch — all present fields are applied together for the isolated build. + +### Flow (inside bincraft, around existing pak resolution) + +1. **Resolve** the dependency set (dry-run) to learn the concrete versions `pak` will install. + Reuse bincraft's existing resolution where possible (e.g. a `pkgdepends` proposal: `$resolve()` → inspect resolution → ... → `$solve()` / `$install()` after the local repo is prepended). +2. For each resolved package that matches a registry entry (name + `versions` + `platforms`): obtain a **patched binary** for the exact `version × platform × arch × R-minor`: + - **Cache hit** (local `/mnt/cache/patched-binaries/` or S3): fetch it into the local repo. + - **Cache miss**: download the CRAN **source** for that version, apply the source `patch` (if any) to the unpacked tree, build the binary in isolation with `env` / `configure_args` / `makevars` applied, then place the binary in the local repo and write it to the cache (and S3 if uploading is enabled). +3. **Prepend** the local binary repo (`file://…`) to `pak`'s repo list, and regenerate its `PACKAGES` index. +4. Run the **normal install**. + `pak` resolves the patched binary for the matched package — direct or transitive — because it wins on repo priority for an equal version, and installs it without recompiling. + +### Caching (essential) + +`RcppParallel` is a dependency of dozens of packages; without caching the fix would be rebuilt on every dependent build. +Patched binaries are keyed by: + +``` +_____ +``` + +`patchhash` is a hash of the normalized registry entry plus the referenced diff file contents. +Editing a patch therefore changes the hash and auto-invalidates stale cached binaries. + +- Local cache: `/mnt/cache/patched-binaries/`. +- Optional S3 cache for cross-build reuse: a dedicated `…/patched/` slot under the existing arch/codename structure, mirroring how normal binaries are stored. + +### S3 upload + +Patched binaries **are** uploaded to S3 (in addition to the local cache) so they are reused across CI jobs and machines, not just within one container. +They live in a separate `patched/` slot and are not published into the user-facing `src/contrib` index — they are an internal build accelerator, not a distributed artifact. + +## Error handling + +- **Source diff fails to apply** (CRAN moved past the pinned version): log a clear warning, skip that entry, and proceed. + The package builds unpatched (status quo) and may fail. + The skipped/failed-to-apply patch is surfaced in build metadata. +- **Pre-build of the patched binary fails**: log a warning, skip, proceed. +- **No version or platform match**: skip silently (the entry simply does not apply to this build). +- **Overlapping entries for one package**: the most specific entry wins (a concrete `platforms`/`versions` beats `"*"`). + Genuine ambiguity (two equally specific, conflicting entries) is a validation error reported before the build. + +## Observability + +- One log line per applied patch, e.g.: `Applying patch to RcppParallel 5.1.11-2 [env: RCPP_PARALLEL_USE_TBB=0]: bundled TBB fails on musl / newer compilers` +- The set of applied patches (package, version, `patchhash`) is recorded in the Postgres build-metadata row for the build, so it is queryable later. + +## Testing + +- **Unit (registry):** parsing and matching — version constraints, platform token matching, precedence/specificity, and detection of ambiguous overlaps. +- **Unit (cache key):** `patchhash` changes when the entry or diff changes; is stable otherwise. +- **Integration:** `RcppParallel` on `resolute` (and/or Alpine) fails to build without a registry entry and succeeds with one; a dependent package such as `rts2` succeeds once the dependency is patched. +- **Failure path:** an entry pinned to an old version against a newer CRAN release → graceful skip with a warning, build continues. + +## Out of scope + +- Shipping a default registry inside `bincraft` (registry is repo-local for now; a baseline-in-engine + repo-override model can come later if needed). +- Publishing patched binaries into the public `src/contrib` index. +- Automatic detection of which packages need patches — entries are curated by hand. + +## Split of work + +- **bincraft:** the mechanism — registry ingestion, resolution hook, isolated patched-binary build, caching/upload, local-repo prepend, logging, metadata recording. + A new `patches` argument on `build_binary_package()`. +- **build-cran-binaries (this repo):** the `local/patches/` registry and diffs, passing `patches = "local/patches"` through `build-one.R` / `build-all.R`, and documentation. diff --git a/specs/2026-08-07-per-minor-edge-routing-design.md b/specs/2026-08-07-per-minor-edge-routing-design.md new file mode 100644 index 0000000..b55812c --- /dev/null +++ b/specs/2026-08-07-per-minor-edge-routing-design.md @@ -0,0 +1,174 @@ +# Design: Routing clients to per-R-minor binary slots + +Date: 2026-08-07 +Status: Approved (pending spec review) + +## Problem + +`bincraft` routes ABI-"risky" packages to a per-minor slot `…/latest/src/contrib//` and indexes every directory independently (`upload_package_index()` calls `cranlike::update_PACKAGES()` on one prefix at a time). +Nothing unions those indices, and `contrib.url()` only ever yields `/src/contrib`, so no value of `options(repos)` can address a per-minor slot. +Only `uvr` resolves per-minor URLs, which means the per-minor slots are invisible to `install.packages()` by construction. + +Measured on 2026-08-07: + +| slot | flat `src/contrib` | `src/contrib/4.5` | unique packages only in the per-minor slot | +| ----------------- | ------------------ | ----------------- | ------------------------------------------ | +| `amd64/alpine324` | 21 640 | 3 310 | 2 886 | +| `amd64/noble` | 24 495 | 398 | 23 | + +This is what issue #63 records as "missing binaries" on `alpine324`. +The packages are not missing; they are in a directory base R cannot reach. +The user-visible symptom in `reg.devxy.io/r/r-alpine:4.5-3.24` is: + +``` +> install.packages("curl") +Warning message: +package 'curl' is not available for this version of R +``` + +A second, unrelated defect exists on the same slot and is **out of scope here**: many `alpine324` tarballs are byte-identical CRAN _source_ tarballs that the index nevertheless stamps `Built: R 4.5.3; …-linux-musl`. +Routing exposes `curl`; only a rebuild of that slot makes it install. + +## Goal + +Let a stock `install.packages()` see one complete package list for its own R minor, without duplicating tarballs and without an R-version-varying cache key anywhere in the CDN. + +## Key constraint that drives the design + +R resolves a package's download URL from the index, not from the request path, and it keeps the `contriburl` it _asked for_ rather than the one it was redirected to. +Measured with `options(repos = …/latest)` against a middleware that redirects the index into `4.5/`: + +``` +curl available: TRUE +curl repo: …/latest/src/contrib # the flat URL, not the 4.5 one it was served from +``` + +So the union index is always addressed relative to the **flat** directory, whatever path it was fetched from. +`available.packages()` honours a `Path:` field and folds it into the `Repository` column, which gives the whole routing for free: + +- a per-minor record carries `Path: `, so its tarball is fetched from `…/src/contrib//` +- a flat record carries no `Path`, so its tarball is fetched from `…/src/contrib/` + +Verified end to end against the live CDN with a locally built union index for `amd64/alpine324` (31 507 records): + +``` +curl: 7.1.0 -> …/latest/src/contrib/4.5 -> curl_7.1.0.tar.gz 717 725 B +jsonlite: 2.0.0 -> …/latest/src/contrib -> jsonlite_2.0.0.tar.gz 1 055 849 B +``` + +The corollary is that the edge script must **not** rewrite tarball requests: every tarball URL is already correct when it leaves the client, and redirecting one into `/` would break exactly the flat packages the union is meant to preserve. + +The complementary trick does not work: R's `gzcon()` reads only the first member of a concatenated gzip stream (10 291 of an expected 31 931 records), so an edge-side merge would have to fully decompress and recompress both indices and additionally 404 `PACKAGES.rds` to stop R preferring it. +That is why the union is produced in `bincraft`, not at the edge. + +## Approaches considered + +| Approach | Where the union lives | Verdict | +| ----------------------------------------------------------- | ------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------- | +| **A. Union index written by `bincraft` (chosen)** | per-minor `PACKAGES*`, per-minor entries carry `Path: ` | Edge does one redirect; `PACKAGES.rds` stays correct; no duplication | +| B. Merge at the edge | middleware fetches both indices, recompresses | ~2 MB decompress/recompress per cache fill, cache key must include the R minor, breaks R's `.rds` fast path | +| C. Move the minor up the path (`latest//src/contrib/`) | addressable by `options(repos)` directly | No edge logic at all, but a full layout migration and breaks the published URL contract | + +Chosen: **A**. + +## Architecture + +### bincraft: union index (separate PR) + +After writing a per-minor index, republish it as a union of that slot and the flat slot: + +1. Read the flat slot's `PACKAGES.rds` and the per-minor slot's own records. +2. Set `Path: ` on every per-minor record, so its tarball resolves into the per-minor directory. +3. Drop every flat record whose package is already present in the per-minor slot, so the per-minor build always wins, and leave the survivors without a `Path`. +4. Write the merged `PACKAGES`, `PACKAGES.gz` and `PACKAGES.rds` into `…/src/contrib//`. + +Guard: refuse to publish a union with fewer records than the flat index it was built from. +A truncated union is worse than no union, because it silently removes packages from every client on that minor. + +### Edge script (this repo) + +The script routes `PACKAGES`, `PACKAGES.gz` and `PACKAGES.rds` requests, and nothing else. + +``` +normalize path +parseClient(UA) -> { rMinor, arch, os } # rMinor from "R (4.5.3 …)" or "R/4.5.3" +darwin branches # unchanged +if path is /{arch}/{os}/latest/src/contrib/PACKAGES* + already under // ? pass through # loop guard + rMinor known && slot in UNION_SLOTS ? 302 -> …/src/contrib//PACKAGES* + else pass through # flat slot, today's behaviour +if path is /src/contrib/… # bare root + resolve arch + os; unknown -> 302 to CRAN + then apply the same PACKAGES* rule +else pass through +``` + +Redirects carry `Cache-Control: no-store`. +Every cacheable URL is therefore UA-independent, and no cache key has to vary by R version. + +### Repaired bare-root detection + +The bare `https://cran.rpkgs.com` form is currently broken for every Linux client that uses a stock R user agent. +`ALPINE_REGEX`, `UBUNTU_REGEX` and `RHEL_REGEX` only match a Posit-style user agent the user has to set by hand; stock R never carries the distro, so the script falls through to `extractOs()` and redirects to a slot that does not exist: + +``` +UA: R (4.5.3 x86_64-pc-linux-musl …) -> 302 /amd64/linux-musl/latest/… (404) +UA: R (4.5.3 x86_64-pc-linux-gnu …) -> 302 /amd64/linux-gnu/latest/… (404) +``` + +The fallback to a phantom `linux-musl` / `linux-gnu` slot is removed. +An unidentifiable distro redirects to CRAN, which is the existing behaviour for an unparseable user agent. +The R _minor_ is always present in a stock user agent, so per-minor routing itself does not depend on distro detection. + +### Rollout gate + +`UNION_SLOTS` is a `bunnynet_compute_script_variable` listing the slots whose per-minor index is already a union. +It is empty by default, so deploying the script changes nothing until `bincraft` has backfilled a slot, and a rollback is a variable edit rather than a code deploy. +All slots currently carry `4.4`, `4.5` and `4.6`; a client on any other minor falls through to the flat slot. + +### Deployment from this repo + +The script is a file in the repo, applied by the existing OpenTofu configuration: + +``` +edge/rpkgs-router.ts # the script +edge/rpkgs-router.test.ts # UA x path -> expected Location matrix +cdn.tf # bunnynet_compute_script + _variable +``` + +Provider `BunnyWay/bunnynet` v0.17.0 (already pinned) ships `bunnynet_compute_script` with `content` loadable via `file()`, plus `bunnynet_compute_script_variable`. +`middleware_script = bunnynet_compute_script.rpkgs_router.id` replaces the hard-coded `29277`, after a one-time `tofu import` of the existing script. + +## Error handling + +- Unknown R minor, or a slot not listed in `UNION_SLOTS`: pass through to the flat slot. + The client sees exactly today's behaviour. +- Unparseable distro on the bare-root form: redirect to CRAN. +- A request already under `…/src/contrib//`: pass through, so a redirect can never loop. +- A per-minor slot that does not exist for a listed minor: the client gets the origin's 404. + `UNION_SLOTS` is the operator's assertion that the slot is ready, so this is a configuration error, not a runtime condition to paper over. + +## Testing + +Local, before any apply: `deno run -A edge/rpkgs-router.ts` serves the middleware against the real origin, so `edge/rpkgs-router.test.ts` drives the whole matrix against that local server. + +- User agent matrix: R 4.4 / 4.5 / 4.6 on musl and gnu, both arches, Posit-style and stock forms, plus a darwin UA and a non-R UA. +- Path matrix: `PACKAGES`, `PACKAGES.gz`, `PACKAGES.rds`, a tarball, a path already under `4.5/`, and `/src/contrib/…` on the bare root. +- Assertion is the `Location` header (or its absence), not the body. + +After apply, a smoke test against `cran.rpkgs.com`: + +- `available.packages()` inside `reg.devxy.io/r/r-alpine:4.5-3.24` returns the union count, and `"curl" %in% rownames(...)` is `TRUE`. +- A flat-slot package still downloads from `…/src/contrib/`, and a per-minor package downloads from `…/src/contrib//`. + +## Out of scope + +- `Meta/archive.rds` stays flat-only, so `remotes::install_version()` does not see per-minor archives. +- The `alpine324` source-tarball defect: that slot serves CRAN sources stamped as binaries, and needs a rebuild independent of this work. +- Any change to how `uvr` resolves per-minor URLs; it already addresses the slots directly. + +## Split of work + +1. `bincraft`: union index writer plus its guard, and a re-index of one slot to validate. +2. This repo: `edge/rpkgs-router.ts`, its test matrix, and the `cdn.tf` resources with `UNION_SLOTS` empty. +3. Enable `UNION_SLOTS` slot by slot as `bincraft` backfills them. diff --git a/specs/2026-08-12-shard-weekly-rebuild-design.md b/specs/2026-08-12-shard-weekly-rebuild-design.md new file mode 100644 index 0000000..f4ccbaa --- /dev/null +++ b/specs/2026-08-12-shard-weekly-rebuild-design.md @@ -0,0 +1,167 @@ +# Design: Sharding and resuming the weekly rebuild + +Date: 2026-08-12 +Status: Approved (pending spec review) + +## Problem + +`weekly-rebuild-missing` runs one job per `-` and walks that slot's rebuild list serially in a single `R -q -e` invocation (`.crow/weekly-rebuild-missing.yaml:165`). +Until 2026-08-09 that was cheap, because every source fallback was skipped as "already built" and the list was effectively empty. +Since bincraft #105/#106/#107 and build-cran-binaries #159 the gate works, and the lists are now large. + +Share of records whose object is byte-identical to CRAN's source, measured against `cran.r-project.org` MD5s on 2026-08-12: + +| slot | records | source-served | share | +| ------------------ | ------: | ------------: | -----------: | +| `amd64/resolute` | 24 212 | 15 023 | 62.1% | +| `arm64/resolute` | 24 291 | 13 670 | 56.3% | +| `arm64/alpine324` | 24 328 | 9 514 | 39.2% | +| `amd64/alpine324` | 24 343 | 8 917 | 36.7% | +| `arm64/rhel10` | 24 695 | 5 384 | 21.9% | +| `amd64/rhel10` | 24 881 | 4 712 | 19.2% | +| 12 remaining slots | ~24 700 | 850 to 2 130 | 3.5% to 8.7% | + +A single serial job cannot absorb that. +Pipeline 10910 (`weekly_rebuild_missing:alpine-324-amd64`) started on 2026-08-09, ran for roughly two days, reached `[8692/23885] cholera`, and was killed there. + +Two distinct failures follow from that shape. + +**No parallelism.** The work is embarrassingly parallel across packages, but one job does all of it. + +**No resumability, and no clean stopping point.** The loop has no terminating condition other than exhausting the list, so the only way to stop it is a kill. +A restarted run re-reads the same list and walks it from the first entry. +It skips completed packages via `check_s3_root_package()`, but that costs a CRAN version resolution and an S3 `HEAD` per package, thousands of times, before it reaches new work. +Worse, a kill is not a pipeline failure: the `Purge CDN cache` step is guarded by `when: status: [success, failure]` (`.crow/weekly-rebuild-missing.yaml:206-207`), and on 10910 it produced no output at all. +So the ~4 600 binaries that run did publish stayed hidden behind stale edge copies. + +## Goal + +Turn each slot's rebuild into bounded, parallel, restartable units, without introducing state that can disagree with the bucket. + +## Design + +### 1. Shard the matrix three ways + +Each of the 18 `OS`/`ARCH` rows in `.crow/weekly-rebuild-missing.yaml` gains `SPLIT_INTO: 3` and `SPLIT_INDEX: 1|2|3`, giving 54 rows. +This mirrors `.crow/build-all-versions.yaml:57-98`, which already shards its matrix four ways per arch. + +Routing needs no change. +The cron filter `cron: weekly-rebuild-missing-${OS}-${ARCH}` and the manual `evaluate: weekly_rebuild_missing == "${OS}-${ARCH}"` both match all three shards of a slot. +Placement stays on the `rpkgs-${ARCH}` group label, so shards queue against available capacity rather than oversubscribing it. + +### 2. Extract the loop into `local/rebuild-missing.R` + +The build is currently a single ~1 500-character `R -q -e` argument. +Shard arithmetic and resume logic do not belong in a YAML string, and none of it is testable there. +The loop moves to `local/rebuild-missing.R`, invoked as `Rscript local/rebuild-missing.R $SPLIT_INTO $SPLIT_INDEX`, mirroring `local/build-all.R`. +Its body is unchanged in substance: read `/tmp/rebuild_pkgs.txt`, subtract `local/excluded-packages.json`, loop with `tryCatch` around `bincraft::build_binary_package()`. + +The slice is **interleaved**, not contiguous: + +```r +# the list is alphabetical and build cost clusters by name (Rcpp*, Bioc*, +# rstan*), so contiguous thirds would be badly unbalanced +mine <- pkgs[seq(split_index, length(pkgs), by = split_into)] +``` + +`local/build-all.R:64` uses contiguous chunks via `cut()`. +That is fine there because its list is every CRAN package and version, so the chunks average out. +Here the list is a filtered backlog in which expensive families sit adjacent, so interleaving is the better default. +Interleaving also makes each shard's `[i/n]` progress representative of the slot as a whole. + +### 3. Resume by re-deriving state from the bucket + +Before the loop, the shard performs one `s3fs::s3_dir_info()` on `devxy-rpkgs-binaries///latest/src/contrib` and reads the `etag` column. +It fetches CRAN's `PACKAGES` once for the latest version and published `MD5sum` of every package. +A package is still outstanding if and only if the object at `_.tar.gz` has an ETag equal to CRAN's `MD5sum` for that version, which is the definition `check_s3_root_package()` already applies one package at a time. + +```r +# one paginated listing instead of ~2900 sequential HEAD requests per shard +info <- s3fs::s3_dir_info(slot_dir) +etag <- setNames(gsub('^"|"$', "", info$etag), basename(info$uri)) + +key <- sprintf("%s_%s.tar.gz", mine, cran_version[mine]) +# keep a package when no object exists yet, or when the object is still +# byte-identical to CRAN's source; drop it once a real binary is published +mine <- mine[is.na(etag[key]) | etag[key] == cran_md5[key]] +``` + +This is the whole resume mechanism. +There is no progress file, no volume, and no database cursor. +A restarted shard recomputes ground truth and continues where it stopped, and it is correct even when a sibling shard, a `process-updates` cron, or a manual `just rebuild` completed something in the meantime. + +Three properties make this the right source of truth: + +- **It is what the build itself checks.** Any other store can disagree with the bucket; this one cannot. +- **It is agent-independent.** `.crow/weekly-rebuild-missing.yaml` mounts no `volumes:`, unlike `.crow/build-all-versions.yaml:132-133`, so `/mnt/cache` is per-job and cannot carry progress anyway. +- **It costs one listing.** `cranlike`'s `s3` fork already does exactly this call against this bucket at ~24 000 objects, so the approach is proven at the required scale. + +It must read ETags rather than the slot index's `Built` field, which is how `local/packages-to-build.R:104-130` answers the same question. +Under this design the index is not rewritten until the dependent re-index pipeline runs (section 5), so mid-run it cannot reflect the current run's progress. + +Packages that genuinely fail to build re-publish their CRAN source, so they stay outstanding and would be retried on every restart. +That is already handled upstream: `bincraft::filter_packages_with_errors()` (`R/build_binaries.R:1018`, `:1143`) drops anything with `error_occurred = TRUE`, and `store_build_metadata = TRUE` is passed on every call. +No additional poison-pill filter is needed here. + +Only the flat `src/contrib` path is considered. +The rebuild call passes no `is_r_minor_sensitive`, so it defaults to `FALSE` and only ever targets the flat path; the resume filter matches that scope deliberately. + +### 4. Give each shard a wall-clock budget + +`local/rebuild-missing.R` takes a budget, defaulting to 20 hours, and breaks out of the loop once it is exceeded: + +```r +# exit cleanly rather than being killed, so the dependent re-index still runs +if (difftime(Sys.time(), started, units = "hours") > budget_hours) { + cat(sprintf("Budget of %sh reached after %d/%d packages; stopping cleanly\n", budget_hours, i, n)) + break +} +``` + +It exits 0 and reports how much of the slice it covered. +Every run then has a terminating condition, the re-index and purge always fire, and the remainder is picked up by the next run with no bookkeeping, because section 3 recomputes the outstanding set from scratch. + +### 5. Move the re-index and purge into `.crow/weekly-rebuild-reindex.yaml` + +Three shards per slot means three concurrent `upload_package_index()` calls on the same S3 prefix. +`cranlike::update_PACKAGES()` lists the live bucket, so an early lister that uploads last publishes an index missing its siblings' work. +The re-index steps (`.crow/weekly-rebuild-missing.yaml:171-176`) and the purge step (`:187-207`) therefore leave that file entirely. + +The new file carries: + +```yaml +depends_on: + - weekly-rebuild-missing +runs_on: [success, failure] +``` + +`runs_on: [success, failure]` validates as a workflow-level key under `crow lint`, so a failing shard no longer withholds the re-index. +The file uses the same 18-row matrix and the same `when:` gating as `weekly-rebuild-missing`, so it only re-indexes slots that actually ran. +Each row re-indexes the flat slot and every per-minor slot. +`scripts/purge_cdn_zone.sh` runs once on a single row, because all hostnames share pull zone `3857050` and 18 identical zone purges would be waste. + +## Failure behaviour + +| case | today | after | +| -------------------------- | ----------------------------------- | ----------------------------------------------------- | +| one package errors | `tryCatch` logs, loop continues | unchanged | +| a shard fails outright | purge runs, re-index does not | re-index and purge run via `runs_on` | +| a shard exceeds its budget | cannot happen, runs until killed | exits 0, re-index and purge run | +| a shard is killed | nothing runs | still nothing; trigger the re-index pipeline alone | +| a shard restarts | re-walks the list, HEAD per package | one listing, resumes at the first outstanding package | + +The known cost of `depends_on` being file-level rather than row-level: on the weekly cron no slot is re-indexed until the slowest of all 54 jobs finishes. +The 20-hour budget bounds that at roughly one day. + +## Out of scope + +- `build-all-versions` still cannot rebuild source fallbacks, because `local/build-all.R:113-122` drops every version with any `single_builds` row for the platform and arch, which is precisely the source-fallback set. That is a separate change. +- Bunny Perma-Cache eviction. `scripts/purge_cdn_zone.sh` purges the regular edge cache only; see the note in `CLAUDE.md` and issue history. +- The audit that produces the rebuild list is unchanged. + +## Verification + +- `crow lint .crow/` passes for both pipeline files. +- `local/rebuild-missing.R` gets unit coverage in `local/tests/` for the two pure pieces: the interleaved slice (disjoint, covering, deterministic) and the outstanding-set filter (source-served ETag kept, binary ETag dropped, absent object kept). +- A single-slot manual run of `alpine-324-amd64` shard 1 confirms the listing shortcut against the live bucket, and that the reported outstanding count is close to the 8 917 measured above divided by three. +- Restarting that shard mid-run confirms it resumes rather than replaying, by comparing the outstanding count it reports on the second start.