feat(ci): Add revdep3, pair checks in isolated containers on a shared revdepx core - #2856
Open
krlmlr wants to merge 17 commits into
Open
feat(ci): Add revdep3, pair checks in isolated containers on a shared revdepx core#2856krlmlr wants to merge 17 commits into
krlmlr wants to merge 17 commits into
Conversation
… revdepx core revdep2 checks each reverse dependency's CRAN and dev halves as two simultaneous `R CMD check` processes on one host. Simultaneously checking the same package against two libraries is not a supported mode of operation for the packages being checked: the PSOCK port collision that needed the `R_PARALLEL_PORT` split was one failure class of an open-ended family (shared TMPDIR, caches, locks, every singleton a check assumes it owns). revdep3 keeps the pair -- concurrency still halves a package's wall clock -- but runs each half in its own Docker container, dissolving the interference class instead of patching mechanisms. Checks run under a pinned R version (oldrel by default) on a rocker/r-ver base, and the dependency universe travels as a GHCR image, built once and delta-updated across runs, which replaces the preflight, the per-shard installs and the prebuilt-library artifacts. The engine-agnostic core lands in .github/workflows/revdepx/, shared byte-for-byte with the sibling revdep4 proposal: one artifact family (revdepx-*), one manifest, baseline and timings schema, one comparison code path, one universe-image lineage -- so either workflow reuses the other's baselines, timings, images and reports, and `retry-run` accepts a run of either. revdep2 stays untouched; this is a proposal beside it, not a replacement of it yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
1 task
…nion only The queue engine briefly revived what revdep2 had retired: substituting a stored old-version result for the old check when the plan judged it comparable. The container platform makes that far safer than it was, but a fresh old check is the only result whose provenance the run fully controls -- so the old half now always runs, in both engines, and a comparable stored result rides along purely as a second opinion: `baseline_agrees` records whether the fresh check reproduced it, and a disagreement is printed as drift. Concretely: the queue engine's per-package price is always two halves (no baseline discount in the plan), the queue file loses its skip_old column, and compare_halves() loses its baseline-substitution mode. The baseline artifact, its validity conditions and the cross-workflow lineage are unchanged -- only what a valid row is *for* has narrowed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…licy Two sentences still described the retired skip-old design: the timings bullet halved the queue plan's bill where a baseline covered the old half, and the baseline bullet spoke of reusing rows. Both halves always run fresh now, and a stored row is only ever a second opinion. (This push also re-triggers the checks that failed during the GitHub outage.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
An independent review of both revdepx branches confirmed four bugs and a handful of gaps; this applies everything that lives in the shared core or on this branch. - The check-slice cut no longer dies on shards with fewer runnable packages than slices: `seq(index, n, by = of)` is an R error when index exceeds n, which crashed slices of 1-package retry shards and -- with an empty runnable set -- erased recorded depfail diagnoses into `missing`. The slice is also cut before the source downloads now, so each slice fetches only its own tarballs instead of the whole shard's three times over. The same seq() trap is fixed in plan.R's `part` split. - The base image now installs callr: util.R's run_with_timeout() silently degrades to an unbounded inline call without it, and every "bounded" pak call of the in-container universe build ran with no clock -- the exact hang class revdep2 added callr for on the host. A denied base-image push is now a hard error too: unlike the universe image it has no artifact fallback, and everything downstream pulls it. - timing.json's `script_seconds` accumulates across check slices; before, the final slice's overwrite dropped the earlier slices' driver time and the calibration charged it to per-shard setup, inflating every later plan. - The `compared` commit gate counts only this run's own comparisons: carried-over retry results could pass it and let a run that learnt nothing overwrite the committed report. - ensure_check_sysreqs() runs `apt-get update` before its direct install -- the base image deletes the apt lists, so the install otherwise failed quietly. - The shared concurrency group is built from `github.ref_name`, so a dispatch with the `ref` input filled in serializes against one without it on the same branch. - check-half.sh tells its outer safety-net timeout apart from a real check timeout (both exit 124) by when the axe fell, and reports the outer case as a runner failure, not a package one. - The queue's claimed.log and queue-state.json are copied into the results artifact per slice; in the work directory alone they died with the runner, which is exactly when they are wanted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
The first live run failed in the build container: "vignette builder 'knitr' not found". The pak bootstrap installs the package's hard dependencies, and building vignettes needs the Suggests tree on top -- which revdep2's host build inherited from setup-r-dependencies without anyone deciding it. The binary exists to be installed into the checks' new-half library, and no check ever builds or reads the package-under-test's own vignettes, so `--no-build-vignettes` removes the requirement instead of importing it. Also forward GITHUB_SHA into the container, so meta.json records the real commit when git cannot answer inside the mount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…e-disk runners Mirror of the revdep4 branch's fixes after its first live run (the shared `revdepx/` scripts stay byte-identical by construction): - `docker commit` copies the container's entire rw layer onto the runner's ONE disk (there is no /mnt volume), and the copy filling the disk killed the runner agent mid-commit -- with its logs, artifacts and cache saves. The commit step now measures the delta and the free space first, skips the commit with a loud warning when the copy cannot fit (the shards' local-build fallback carries the run), drops the build container right after a successful commit so the peak subsides before the push, gives the artifact fallback its own space check, and gates the artifact upload and download on a new `saved` output. - build.R moved the dev binary with file.rename() across two bind mounts; rename(2) fails there with EXDEV and file.rename() only says so in an unread return value, so every shard failed installing a binary the artifact never contained. Copy and check instead. - The long steps (universe build, commit, shard image prep) run in-step resource samplers: a sampler backgrounded in its own step keeps writing its file, but its stdout stops reaching the job log when that step ends, and the streamed lines are all that survives a dying runner. - chown the pak cache back to the runner user after root-in-container wrote it, so the cache post-step's tar stops failing on 600 lock files and the cache finally saves. - Rename the "Move docker onto the big disk" steps: there is no big disk to move to, only room to make. - Bump docker/login-action to v4 for the Node.js 24 runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…dence first Mirror of the revdep4 branch's hardening after its second dead runner in the guarded commit (~34 minutes in, both runs, and a dead runner loses even the log lines it already streamed): - measure the delta in bytes AND inodes and upload the numbers as an artifact BEFORE the commit runs, so the next post-mortem starts with data; - the guard reads those numbers, refuses an unmeasured delta, and checks inodes as well as bytes; - the commit runs under a watchdog that stops the docker daemon when disk, inodes or memory approach zero -- killing the client would not abort the daemon-side copy -- then restarts it so the rest of the job still works. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
… before retrying singly Mirror of the revdep4 branch's commit (the shared `revdepx/` scripts stay byte-identical by construction): bind-mount /tmp from the host in the universe build container so killed installs' build residue never enters the committed layer, sweep apt lists and /tmp leftovers at the end of image.R, and re-try a failed chunked install in chunks of 50 before falling back to the one-at-a-time salvage pass, so one broken package no longer drags hundreds of stranded chunk-mates through per-package pak calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…a flat scan Mirror of the revdep4 branch's commit (the shared `revdepx/` scripts stay byte-identical by construction): the salvage pass after a failed chunked install now divides and conquers with fan-out 3 instead of scanning one package at a time -- d culprits hiding in n packages cost about 3 * d * log3(n) pak calls instead of n, and single-package leaves still name each genuine failure with its own log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Mirror of the revdep4 branch's fix (the shared `revdepx/` scripts stay byte-identical by construction): a /tmp bind mount created by plain mkdir is 755, apt-key cannot create its temporary config in it, every repository then fails signature verification, and every apt-get pak runs fails with it -- reproduced and fixed locally against rocker/r-ver:4.5.3. chmod 1777 on every host directory that becomes a container's /tmp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…hardening Mirror of the revdep4 branch's commit (the shared `revdepx/` scripts stay byte-identical by construction): R 4.6.1 hard-coded as the pinned default (at least one reverse dependency needs R-release), Xvfb in the base image and started for installs, load tests and checks (Tk-based packages die headless while lazy-loading), the universe delta measured via `docker ps -as` when the containerd image store leaves GraphDriver empty, the check-sysreqs survey chunked (one call over 3435 packages was OOM-killed at 14 GB) with memory caps on the build containers, load-test roots computed from Depends+Imports so LinkingTo-only packages are load-tested too, and in-step resource samplers on the check slices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Same device as on the revdep4 branch: a workflow_dispatch workflow is only registered once its file exists on the default branch, so the live test fires from a push trigger with the inputs -- and the run title -- hardcoded to the test parameters. Revert this commit after the test run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
…t, depth 2)" This reverts commit 6ae2a4a. The run it started (32148370376) was cancelled within a minute: the slice-truncation bug found in revdep4's completed run would have destroyed two thirds of its results too. The trigger returns after the fixed revdep4 run (most, depth 2, R 4.6.1) completes, so the two workflows do not contend for the runner pool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Mirror of the revdep4 branch's fix (the shared `revdepx/` scripts stay byte-identical by construction): the check phase opened with file.create(manifest_path), which truncates an existing file, so under slicing every slice wiped its predecessors' results and the sweep re-wrote them as `deferred` -- run 32114635495 lost 2293 finished checks to it. Create the manifest only when it does not exist yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Mirror of the revdep4 branch's commit (the shared `revdepx/` scripts stay byte-identical by construction): push and save with `--platform linux/amd64` -- the containerd image store commits an OCI manifest index, which a plain push refuses and a plain save reduces to a 1336-byte manifest shell (run 32148999976) -- refuse to upload a save under 100 MB, let a bad pulled or loaded image fall through to the shard-local build instead of failing the shard, and survive single failing chunks of the sysreqs survey. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Mirror of the revdep4 branch's commit: the containerd image store's commit is an OCI index that can be neither pushed nor usefully saved (run 32158907637); the daemon.json this workflow already writes now disables the containerd snapshotter, restoring the classic store whose committed images push, save and du-measure exactly as everything here assumes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
Same device as on the revdep4 branch: a workflow_dispatch workflow is only registered once its file exists on the default branch, so the live test fires from a push trigger with the inputs -- and the run title -- hardcoded to the test parameters. Revert this commit after the test run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UB7YutLzVWU7xCTYUvF3kF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposes revdep3, the pair engine: the reverse-dependency check rebuilt on Docker containers, as one of two sibling proposals (the other is revdep4, #2857). Both reuse the good bits of revdep2 through a shared, engine-agnostic core, and are fully interoperable with each other.
The problem
revdep2 checks each reverse dependency against the released and the dev igraph simultaneously on one host — two
R CMD checkprocesses racing on the same machine. The PSOCK port collision (fixed by hand withR_PARALLEL_PORT=20000/30000) was only one failure class: the halves share/tmp, caches, locks, the loopback interface — every singleton a check assumes it owns. Simultaneously checking the same package against two libraries is not a supported mode of operation, and per-mechanism patches cannot make it one.What revdep3 does
Keeps the pair (concurrency still halves a package's wall clock), but runs each half in its own container — own network namespace, PID space, mounts,
/tmp. The interference class is dissolved categorically; the port hack is deleted, not replaced.Pipeline:
plan+baseimage (parallel) →build(dev binary, compiled inside the container so it loads under the container's R) →universeimage (the whole dependency universe + sysreqs, committed and pushed to GHCR) →testshards (pull image; per package, two containers side by side) →collect(unchanged reports/baseline/timings).New capabilities that fall out of containerization:
r-version(default 4.6.1 — at least one reverse dependency needs R-release;oldrel/release/any version can be dispatched) onrocker/r-ver, with Xvfb for the Tk-based corner of CRAN — no moving target between R releases; results comparable across runs while the runner image drifts.REVDEPX_IMAGE_MAX_AGE_DAYS(14). Shards pull instead of installing; host shards need no TinyTeX/pandoc/qpdf/apt setup at all.The old half always runs fresh; a stored old result is a second opinion only. Where the plan certifies an earlier run's old result as comparable (same revdep version, our CRAN version, container R, base-image tag, dependency fingerprint, within age), the fresh old check is compared against it:
baseline_agreeson the manifest line, disagreements reported as drift. The stored result never substitutes for the check.Kept from revdep2 (via
.github/workflows/revdepx/)The planner (enumeration,
packages/broken/retry-run/part/dry-run, self-calibrating cost model, cost-balanced dealing, refusal with a ready-madepartsplit), the manifest schema and result vocabulary, baseline lineage and validity conditions, timings artifact and calibration, three-slice check uploads, the resource sampler, chunked deadline-bounded installs, the load test, the comparison/diff/salvage machinery, the report/collect pipeline and the committedrevdep/record.revdep2itself is untouched by this PR.Dropped, deliberately
R_PARALLEL_PORTand all per-mechanism interference patches; the preflight job;revdep2-lib/revdep2-lib-indexartifacts, donor-run walks, tar pack/restore (~350 lines of util.R); host toolchain setup on shards; the preflight-union arithmetic.Compatibility with revdep4 (#2857)
Both PRs ship
.github/workflows/revdepx/byte-for-byte identical (this PR addsrevdep3.yaml+revdep3/; #2857 addsrevdep4.yaml+revdep4/; merging both leaves one copy with no conflicts). The workflows share one artifact family (revdepx-*), one manifest/baseline/timings schema, one comparison code path, one universe-image lineage on GHCR, and one concurrency group per ref. Concretely: either workflow reads the other's baselines as second opinions (keyed on revdep version, our CRAN version, container R, base-image tag, dependency fingerprint, age), consumes the other's timings (one canonical per-half seconds number; each plan prices its own engine from it), retries the other's runs (retry-runaccepts either), and starts from whichever universe image was refreshed last.Notes for review
revdepx-base/revdepx-universe; if org policy forbidsGITHUB_TOKENpackage writes, every run still works via the artifact fallback (slower, no cross-run image reuse).plan.Rdry runs against live CRAN metadata for both engines; container mechanics smoke-tested against a local Docker daemon withrocker/r-ver— including a real containerized pair check exercising the full parse/compare/manifest path;bash -n/parse()clean; actionlint findings identical to what revdep2.yaml already carries.docker commitdisk exhaustion, an EXDEV-silenced binary artifact,/tmppermissions breaking apt, a manifest-truncating slice bug that destroyed two thirds of a run's results, the containerd image store making committed images unpushable, an OOM-killed sysreqs survey, headless Tk installs — and every fix is mirrored here byte-for-byte (this branch's history carries the same commits). The final revdep4 validation run (32158907637,most, depth 2, R 4.6.1) completed green: 3435 packages, 3231 ok, 21 newly broken. revdep3's own validation run with the same parameters exercises the pair engine and, first among the runs, the universe-image publish under the classic docker store.