From 6281ff6e5c13fe8173e527e9897ff117b3c8660a Mon Sep 17 00:00:00 2001 From: pat-s Date: Tue, 30 Jun 2026 07:58:43 +0200 Subject: [PATCH] docs(spec): design for dynamic per-package patching during builds Adds a brainstormed design for patching packages (env/configure overrides and source diffs) before they are installed by pak, including transitive dependencies like RcppParallel. The mechanism lives in bincraft (pre-built patched binaries served from a prepended local repo); the curated patch registry lives in this repo. --- specs/2026-06-30-package-patching-design.md | 157 ++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 specs/2026-06-30-package-patching-design.md 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..1344173 --- /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.