build-cran-binaries/specs/2026-06-30-package-patching-design.md
pat-s 55a18fd87d
Some checks failed
ci/crow/cron/process-updates/15 Pipeline failed
ci/crow/cron/process-updates/14 Pipeline failed
ci/crow/cron/process-updates/16 Pipeline failed
ci/crow/cron/process-updates/18 Pipeline failed
ci/crow/cron/process-updates/11 Pipeline failed
ci/crow/cron/process-updates/12 Pipeline failed
ci/crow/cron/process-updates/17 Pipeline failed
ci/crow/cron/process-updates/6 Pipeline failed
ci/crow/cron/process-updates/5 Pipeline failed
feat: patch registry + wiring for per-package patching (#103)
## Summary

Adds the curated **patch registry** and wiring that drives bincraft's new package-patching mechanism (see bincraft PR `feat/package-patching`).
Lets specific packages be patched (env/configure/Makevars overrides or source diffs) before pak installs them — including as transitive dependencies — so compiler-/OS-specific failures like RcppParallel's bundled TBB stop cascading.

## What's included

- `local/patches/registry.json` — initial entry: RcppParallel with `RCPP_PARALLEL_USE_TBB=0` for alpine / ubuntu-2604, plus `local/patches/README.md` schema docs.
- `local/validate-patches.R` — validates schema, referenced patch files, and ambiguous overlaps; clean failure + exit 1 (no stacktrace).
- `.pre-commit-config.yaml` — a `validate-patches` hook (re-runs when the registry or the validator changes).
- `local/build-one.R` / `local/build-all.R` — pass `patches = "local/patches"` to `bincraft::build_binary_package()`.
- `specs/2026-06-30-package-patching-design.md` and `plans/2026-06-30-package-patching-implementation.md`.

## ⚠️ Merge ordering (blocker)

This PR adds a `patches = ...` argument to `build_binary_package()` calls.
The `.crow/*.yaml` workflows currently pin bincraft **v4.2.3**, which does not accept that argument — CI will error with `unused argument (patches=...)` until:

1. bincraft **v4.3.0** is released (PR `feat/package-patching`), and
2. the pin is bumped in `.crow/build-all-versions-install-deps.yaml`, `.crow/build-all-versions.yaml`, and `.crow/process-updates.yaml`.

The `.crow` pin bump will be added to this PR once bincraft v4.3.0 is tagged. Do not merge before then.

Reviewed-on: #103
2026-06-30 08:50:19 +00:00

157 lines
9.4 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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:
```
<package>_<version>_<platform>_<arch>_<rminor>_<patchhash>
```
`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.