From 5690afe51a4290826c462cad9b893e30b6964a85 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Mon, 19 Oct 2026 10:59:30 +0200 Subject: [PATCH 1/4] Gap 9: cargo xtask ci on every PR (Linux x64); ci --quick; one local ci at a time - .github/workflows/ci.yml: the full `cargo xtask ci` on every pull request and push to main (Linux x64, craft-fonts, rust-cache for dependencies), so it becomes the merge gate. - `cargo xtask ci --quick [--base REF]`: fmt, clippy, the changed packages' tests, layers and assets, plus heif / parity / wasm when the changes touch them, measured against the merge base with origin/main (xtask/src/ci_quick.rs). Workspace-wide files test everything. - A machine-wide lock (xtask/src/ci_lock.rs): a second `ci`, from another worktree, waits and names the run it waits for. LIGHTCRAFT_CI_SLOTS=N, LIGHTCRAFT_CI_LOCK=off|. - README, CLAUDE.md quality gate, gaps.md gap 9, target-app-parity.md stability note. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 69 +++++++++++++ CLAUDE.md | 5 +- README.md | 9 ++ docs/gaps.md | 5 +- docs/target-app-parity.md | 5 +- xtask/src/ci_lock.rs | 145 +++++++++++++++++++++++++++ xtask/src/ci_quick.rs | 204 ++++++++++++++++++++++++++++++++++++++ xtask/src/main.rs | 92 +++++++++++++++-- 8 files changed, 519 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 xtask/src/ci_lock.rs create mode 100644 xtask/src/ci_quick.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..f5783ac54 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,69 @@ +# The quality gate (`cargo xtask ci`: fmt, clippy, heif, tests, parity, layers, assets, wasm) on +# every pull request and every push to main, so the full run happens here rather than on the +# machine of everyone with a branch open. Locally, `cargo xtask ci --quick` checks what a branch's +# changes can break in a few minutes; this job is the merge gate. +# +# Linux only: macOS / Windows test jobs are #1148. Standard GitHub-hosted runner (free for public +# repositories), never a larger one. +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + # a fresh build every run: incremental state is only overhead, and it bloats the cache + CARGO_INCREMENTAL: 0 + RUST_BACKTRACE: 1 + # one job per runner: the machine-wide CI lock (xtask/src/ci_lock.rs) has nothing to wait for + LIGHTCRAFT_CI_LOCK: "off" + +jobs: + xtask-ci: + name: cargo xtask ci (Linux x86_64) + runs-on: ubuntu-latest + timeout-minutes: 90 + env: + CRAFT_FONTS_DIR: ${{ github.workspace }}/craft-fonts + CRAFT_FONTS_REQUIRED: "1" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + # the font build input, so the Japanese and Chinese font tests run (they skip without it) + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: storytold/craft-fonts + ref: 3d9ab3a132ed0eff457e424226a435a901da30d3 # as freebsd.yml; bump deliberately + path: craft-fonts + persist-credentials: false + - name: Install system libraries + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libxkbcommon-dev libwayland-dev libx11-dev libxrandr-dev libxi-dev libgl1-mesa-dev + - uses: dtolnay/rust-toolchain@686976e191b89faba57d3206551f0f330d8cb249 # stable + with: + components: rustfmt, clippy + targets: wasm32-unknown-unknown + # dependencies only (rust-cache drops the workspace's own artifacts), saved from main and + # restored by pull requests + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: ci + save-if: ${{ github.ref == 'refs/heads/main' }} + - name: cargo xtask ci + run: cargo xtask ci + - name: Job summary + if: ${{ !cancelled() }} + run: cat target/ci/summary.md >> "$GITHUB_STEP_SUMMARY" || true diff --git a/CLAUDE.md b/CLAUDE.md index 00a2314aa..f432baf96 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,10 @@ of it. Full standard: `../craftrules/standards/never-crash.md` label, cancel)` for its whole run. It adopts the cancel flag the job already checks (`Cancel::Flag`), or uses `Cancel::No` when the job can't stop. No progress window, panel or progress toast of its own; its tests assert the row. - **Resolution independence:** settings use normalized image coordinates and relative radii; previews and exports must match. -- **Quality gates** before every commit: `cargo xtask ci` (fmt, clippy -D warnings, tests, layers, assets, wasm). +- **Quality gates** before every commit: `cargo xtask ci --quick` (fmt, clippy -D warnings, the changed packages' + tests, layers, assets, and heif / parity / wasm when touched). GitHub runs the full `cargo xtask ci` on every PR + (`.github/workflows/ci.yml`); run the full one locally before merging branches (below) or when `--quick` can't see + the risk. One `ci` runs at a time per machine; others wait their turn (`xtask/src/ci_lock.rs`). - **Commits:** one task id per commit (`M2.3: local Laplacian highlights/shadows`). Only green states. End messages with the attribution line required by the environment. ## Assets: icons, images, fonts (ABSOLUTE RULE — never violate) diff --git a/README.md b/README.md index 38ff85898..98eda5710 100644 --- a/README.md +++ b/README.md @@ -396,8 +396,17 @@ cargo run --release -p lightcraft -- --control 7980 # with the automation ch cargo xtask web --serve # the same app in the browser: http://127.0.0.1:8080/ cargo run --release -p lightcraft-cli -- render photo.jpg -o out.jpg --set light.exposure=0.5 cargo xtask ci # fmt, clippy, HEIF, tests, parity, layers, assets, WASM +cargo xtask ci --quick # only what your branch's changes can break (minutes) ``` +`cargo xtask ci --quick` compares with the merge base of `origin/main` (`--base REF` for another) +and runs fmt, clippy, the tests of the changed packages, layers and assets, plus HEIF, parity and +the wasm check when the changes touch them; a change to `Cargo.toml`, `Cargo.lock` or the lint +config tests everything. GitHub runs the full `cargo xtask ci` on every pull request +(`.github/workflows/ci.yml`). Only one `ci` runs at a time per machine: a second one, from another +worktree say, waits for the first and names it, since several at once take longer in total than in +turn. `LIGHTCRAFT_CI_SLOTS=2` allows two; `LIGHTCRAFT_CI_LOCK=off` skips the lock. + CI defaults to line-table debug information and one build job per 1.5 GB of RAM available when it starts (at most one per CPU; 4 if memory can't be read), so an 8 GB machine with 6 GB free builds with 4, and a busy machine with little free diff --git a/docs/gaps.md b/docs/gaps.md index 1b4a385fa..92668ee64 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-15 · **Change:** minor (gap 15: Print Page Options — page numbers, page info, crop marks) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (gap 9 shrinks: `cargo xtask ci` runs on every PR on Linux x64) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -23,7 +23,7 @@ Hours are Opus 5.5 agent wall-clock hours (see [target-app-parity.md](target-app | 6 B | **Compressed ORF** (every Olympus / OM body since ~2008) | measured: 66 of 78 Olympus / OM models preview-only; `orf.rs` refuses; OM bucket 3% of photographers, 3% of them fully work | +2.6 points; best gain per hour of any raw gap (~0.2) | 8–15 | [raw-parity](raw-parity.md#compressed-orf-a-clean-room-starting-point) (the two Olympus patents that describe the method) | | 7 | **Nikon High Efficiency NEF** (HE / HE★ on Z 8, Z 9, Z 6III, Z f, Z50II, Z5II when set to HE) | named and preview only (#193); needs a pure-Rust JPEG XS decoder | +1.1 points (only shots taken in HE; lossless works on the same bodies); low gain per hour (~0.04), so not a beta blocker | 20–40 | [raw-parity](raw-parity.md) | | 8 B | **No lens profile database** (LR-EDIT-OPTICS-PROFILE): only DNG opcodes, RW2 in-file distortion, Sony ILCE-7RM4A and ILCE-7RM6 distortion (`docs/sony-ilce7rm6-lens-corrections.md`) and Nikon Z 6II / Z 8 / Z f distortion (`docs/nikon-lens-corrections.md`) | Adobe ships 3,635 lens profiles; we never use them | distortion / vignetting correction is a default step for many | 40–80 + calibration shots | [raw-parity](raw-parity.md#lens-profiles) | -| 9 B | **Tests don't run in CI on macOS, Windows or Linux x64**: only FreeBSD runs `cargo test`; the others build packages | `.github/workflows/` (freebsd, packaging-lint, release, windows-arm64); the linux-riscv64 release job added in 0.6.0 (#844) runs only `lightcraft-cli --version` under QEMU | regressions reach users on the main platforms; `cargo xtask ci` runs only locally | 6–12 | [target-app-parity](target-app-parity.md#by-dimension) | +| 9 B | **Tests don't run in CI on macOS or Windows**: Linux x64 runs `cargo xtask ci` on every PR and push to main (`.github/workflows/ci.yml`) and FreeBSD runs `cargo test`; the others build packages | `.github/workflows/` (ci, freebsd, packaging-lint, release, windows-arm64); the linux-riscv64 release job added in 0.6.0 (#844) runs only `lightcraft-cli --version` under QEMU; macOS / Windows jobs: #1148 | regressions in `cfg(target_os)` code reach macOS and Windows users | 6–12 | [target-app-parity](target-app-parity.md#by-dimension) | | 10 B | **Per-model verification thin, plus cheap per-body decode fixes**: 545 of 1,446 Adobe models verified (38%), ~85% of photographers on a verified body; Nikon 5 bodies verified, Canon 34, Leica 14 (the Q3 by a recorded check of 38 private DNGs). Cheap fixes: Z 6II packed 14-bit NEF (#769, PR #671), Sony A100 / F828 regression (#702), DSLR-A200…A390 / A850 12-bit ARW (#535) | measured: [raw/coverage.tsv](raw/coverage.tsv) | per-body bugs (levels, crops, WB) surface only from users | 25–45 | [raw-parity](raw-parity.md#by-maker-measured) | | 11 | **Enhance: Raw Details, Super Resolution; AI denoise without shippable weights or X-Trans** | ENH rows 0%; denoise Bayer only, RawNIND weights GPL and opt-in | Fujifilm users get no AI denoise; nobody gets it out of the box | 30–60 + model decision | [raw-parity](raw-parity.md#demosaic-levels-crops-and-lens-metadata) | | 12 | **Content-aware / generative remove, detect distractions, reflections removal** | REM rows 75%; patch synthesis missing | cleanup work goes back to Photoshop | 25–45 (+ generative model) | [target-app-parity](target-app-parity.md#ai) | @@ -77,6 +77,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | Gap 9 shrinks: `.github/workflows/ci.yml` runs `cargo xtask ci` on Linux x64 for every PR and push to main; macOS and Windows remain | | 2026-10-18 | patch | Gap 1: six X-Trans CMOS 4 Fujifilm bodies (X-T30 …) take the X-T3's spectral matrices instead of the neutral matrix | | 2026-10-15 | minor | Gap 15 shrinks: the Page panel's Page Options (Page Numbers, Page Info, Crop Marks) print on Single Image and Contact Sheet pages; identity plate, watermark, photo info, Cut Guides, direct printing and JPEG output remain | | 2026-10-14 | minor | Gap 26: at 100k photos stack and album lookups are indexed, so an "In a Stack" or "Album" smart album counts in ~14 ms (was 0.6–2.7 s); open time unchanged | diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 557f37a6a..8281639a7 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # Target-app parity: LightCraft vs Adobe Lightroom -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-15 · **Change:** minor (RAW: 83% of Adobe's models decode, 38% verified after the camera-coverage sweep #1327) · **Target:** Adobe Lightroom Classic 15.5 (August 2026) and Lightroom 9.6 (desktop) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** patch (stability: `cargo xtask ci` runs on every PR on Linux x64) · **Target:** Adobe Lightroom Classic 15.5 (August 2026) and Lightroom 9.6 (desktop) The authoritative assessment of how close LightCraft is to Lightroom. The row-by-row checklist is [parity-checklist.md](parity-checklist.md); the ranked work list is [gaps.md](gaps.md); deep areas @@ -158,7 +158,7 @@ essentials replaces its guessed 45% raw look with 84% × 0.75 (+1 point). | Discount | Mainstream | Essentials | Evidence | |---|---:|---:|---| | Interaction fidelity / UI clarity | ×0.95 | ×0.95 | brush live preview gaps and feathering (#517, #518); crop artefacts and limits (#731, #742, #770, #771); Compare white-balance selector (#743–#745); low-res slider previews (#330); dialogs taller than the window (#781); mouse-wheel scroll (#714) | -| Stability on real machines | ×0.92 | ×0.92 | launch and quit crashes or silent startup failures (#250 macOS 12 Intel, #315 Windows Intel GPU, #431 Intel Mac, #260, #462, #783 Linux / Nouveau, #790 Windows installer); tests run in CI on FreeBSD only (gap 9) | +| Stability on real machines | ×0.92 | ×0.92 | launch and quit crashes or silent startup failures (#250 macOS 12 Intel, #315 Windows Intel GPU, #431 Intel Mac, #260, #462, #783 Linux / Nouveau, #790 Windows installer); tests run in CI on Linux x64 and FreeBSD only (gap 9) | | File exchange with Lightroom users (mainstream) / opening files people send (essentials) | ×0.92 | ×0.95 | Adobe XMP sidecars change colours (#290), XMP CameraProfile and Upright ignored (#204, #205), catalog import fails (#599), same Light values render differently (#523, #209), no write-back to `.lrcat`; for casual users: CR3 beyond verified bodies, compressed ORF, HE NEF open as previews (#473, #485, #556), HEIC trouble (#693) | | **Product** | **×0.804** | **×0.830** | | @@ -300,6 +300,7 @@ P2 items 12–25 h; look tuning, performance, packaging, hardening 25–45 h. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | patch | Stability note: tests run in CI on Linux x64 (every PR) as well as FreeBSD; macOS and Windows remain (gap 9) | | 2026-10-14 | minor | Localization ~52% → ~55% (mean of the 12 key languages): de, es, fr, ja and uk catalogs filled | | 2026-10-12 | minor | UI/UX remaining 34–58 h → 32–54 h (canvas tools by keyboard); headline numbers unchanged | | 2026-10-14 | minor | Merge and HDR: merge 100% → 88% (LR-MERGE-HDR ✅ → 🟡 after testing on real 67 MP brackets; gap 29); area percentages unchanged | diff --git a/xtask/src/ci_lock.rs b/xtask/src/ci_lock.rs new file mode 100644 index 000000000..321b0f391 --- /dev/null +++ b/xtask/src/ci_lock.rs @@ -0,0 +1,145 @@ +//! One `cargo xtask ci` at a time per machine. +//! +//! Every CI run builds the workspace several times with one job per CPU. Several worktrees running +//! it at once (parallel agents) slow each other down far more than waiting in turn would, and make +//! the machine unusable meanwhile. So `ci` first takes one of `LIGHTCRAFT_CI_SLOTS` (default 1) +//! lock files in a per-user cache directory, shared by every checkout and worktree, and holds it +//! until it exits. The OS drops the lock when the process ends, crashed or not. +//! +//! - `LIGHTCRAFT_CI_SLOTS=N`: N runs at once (1–64). +//! - `LIGHTCRAFT_CI_LOCK=off`: no lock (e.g. a CI runner that only ever runs one job). +//! - `LIGHTCRAFT_CI_LOCK=`: keep the lock files there instead. + +use std::fs::{File, OpenOptions, TryLockError}; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +const POLL: Duration = Duration::from_secs(2); + +/// A held CI slot; dropping it releases the slot. +pub struct Held { + _file: File, + owner: PathBuf, +} + +impl Drop for Held { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.owner); + } +} + +/// Where the lock files live, or `None` when the lock is switched off. +fn lock_dir() -> Option { + match std::env::var_os("LIGHTCRAFT_CI_LOCK") { + Some(v) if matches!(v.to_str(), Some("off" | "0" | "false" | "no")) => return None, + Some(v) if !v.is_empty() => return Some(PathBuf::from(v)), + _ => {} + } + let cache = std::env::var_os("XDG_CACHE_HOME") + .map(PathBuf::from) + .or_else(|| std::env::var_os("LOCALAPPDATA").map(PathBuf::from)) + .or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".cache"))) + .unwrap_or_else(std::env::temp_dir); + Some(cache.join("lightcraft")) +} + +fn slots() -> usize { + std::env::var("LIGHTCRAFT_CI_SLOTS").ok().and_then(|s| s.trim().parse().ok()).unwrap_or(1).clamp(1, 64) +} + +fn slot_path(dir: &Path, slot: usize) -> PathBuf { + if slot == 0 { dir.join("ci.lock") } else { dir.join(format!("ci-{}.lock", slot + 1)) } +} + +fn owner_path(lock: &Path) -> PathBuf { + lock.with_extension("lock.owner") +} + +/// Take the first free slot of `slots` in `dir`, or `None` when all are held. +pub fn try_acquire(dir: &Path, slots: usize, owner: &str) -> Result, String> { + std::fs::create_dir_all(dir).map_err(|e| format!("ci lock: {}: {e}", dir.display()))?; + for slot in 0..slots.max(1) { + let path = slot_path(dir, slot); + let file = + OpenOptions::new().create(true).truncate(false).write(true).open(&path).map_err(|e| format!("ci lock: {}: {e}", path.display()))?; + match file.try_lock() { + Ok(()) => { + // a sidecar file, not the lock file: Windows locks are mandatory, so others couldn't read it + let owner_file = owner_path(&path); + let _ = std::fs::File::create(&owner_file).and_then(|mut f| f.write_all(owner.as_bytes())); + return Ok(Some(Held { _file: file, owner: owner_file })); + } + Err(TryLockError::WouldBlock) => continue, + Err(TryLockError::Error(e)) => return Err(format!("ci lock: {}: {e}", path.display())), + } + } + Ok(None) +} + +/// Who holds each slot, as they wrote it. +fn holders(dir: &Path, slots: usize) -> Vec { + (0..slots.max(1)) + .filter_map(|slot| std::fs::read_to_string(owner_path(&slot_path(dir, slot))).ok()) + .map(|s| s.trim().to_owned()) + .filter(|s| !s.is_empty()) + .collect() +} + +/// Wait for a CI slot (see the module docs). `None` when the lock is off. +pub fn acquire(root: &Path, command: &str) -> Result, String> { + let Some(dir) = lock_dir() else { + return Ok(None); + }; + let slots = slots(); + let owner = format!("pid {} in {}: {command}", std::process::id(), root.display()); + if let Some(held) = try_acquire(&dir, slots, &owner)? { + return Ok(Some(held)); + } + let waiting = Instant::now(); + eprintln!("=== ci: waiting for another CI run to finish ({slots} at a time; LIGHTCRAFT_CI_SLOTS changes it) ==="); + for h in holders(&dir, slots) { + eprintln!(" running: {h}"); + } + loop { + std::thread::sleep(POLL); + if let Some(held) = try_acquire(&dir, slots, &owner)? { + eprintln!("=== ci: waited {:.0} s for a CI slot ===", waiting.elapsed().as_secs_f64()); + return Ok(Some(held)); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("xtask-ci-lock-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + dir + } + + #[test] + fn one_slot_is_exclusive_until_dropped() { + let dir = scratch("one"); + let first = try_acquire(&dir, 1, "first").unwrap().expect("free slot"); + assert!(try_acquire(&dir, 1, "second").unwrap().is_none()); + assert_eq!(holders(&dir, 1), ["first"]); + drop(first); + assert!(holders(&dir, 1).is_empty()); + assert!(try_acquire(&dir, 1, "third").unwrap().is_some()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn more_slots_admit_more_runs() { + let dir = scratch("two"); + let a = try_acquire(&dir, 2, "a").unwrap(); + let b = try_acquire(&dir, 2, "b").unwrap(); + assert!(a.is_some() && b.is_some()); + assert!(try_acquire(&dir, 2, "c").unwrap().is_none()); + assert_eq!(holders(&dir, 2), ["a", "b"]); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/xtask/src/ci_quick.rs b/xtask/src/ci_quick.rs new file mode 100644 index 000000000..a9e98703b --- /dev/null +++ b/xtask/src/ci_quick.rs @@ -0,0 +1,204 @@ +//! `cargo xtask ci --quick`: the checks a branch's own changes can break, for a pre-push check. +//! +//! The full `ci` builds the workspace four ways (clippy, heif, tests, wasm) plus the CLI for the +//! parity check, ~23 minutes cold. `--quick` looks at what changed since the merge base with +//! `origin/main` (or `--base REF`), committed or not, and runs: +//! +//! - fmt, layers, assets: always (seconds); +//! - clippy on the whole workspace (check builds only; catches callers broken by an API change); +//! - the tests of the changed packages only (not their dependents); +//! - heif when `lightcraft-codecs` / `lightcraft-heif` changed; +//! - parity when the tracker docs, a command/control crate, or a file's path changed (renames and +//! deletions: the tracker cites paths); +//! - the wasm check of the changed packages that must build for wasm32. +//! +//! A change to a workspace-wide file (root `Cargo.toml`, `Cargo.lock`, lint or toolchain config) +//! tests everything. The full `ci` (GitHub's CI on every PR) stays the merge gate. + +use std::collections::BTreeSet; +use std::path::Path; +use std::process::Command; + +/// Root-level files whose change can affect every package. +const WORKSPACE_WIDE: &[&str] = + &["Cargo.toml", "Cargo.lock", "clippy.toml", "rustfmt.toml", ".rustfmt.toml", "rust-toolchain", "rust-toolchain.toml"]; +const WORKSPACE_WIDE_DIRS: &[&str] = &[".cargo/"]; +/// Packages whose ids or paths the parity tracker cites (commands, develop controls, UI menus). +const PARITY_PACKAGES: &[&str] = &["lightcraft-engine", "lightcraft-develop", "lightcraft-ui-egui", "lightcraft-cli"]; +const PARITY_DOCS: &[&str] = &["docs/parity-checklist.md", "docs/gaps.md"]; +const HEIF_PACKAGES: &[&str] = &["lightcraft-codecs", "lightcraft-heif"]; + +/// A changed path, relative to the workspace root; `gone` when deleted or renamed away. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Changed { + pub path: String, + pub gone: bool, +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct Plan { + /// Workspace packages with a changed file. + pub packages: BTreeSet, + /// A workspace-wide file changed: test everything. + pub workspace_wide: bool, + pub heif: bool, + pub parity: bool, + /// What the changes were measured against, for the log and the report. + pub base: String, + pub files: usize, +} + +impl Plan { + /// Whether anything compiled changed (otherwise clippy and the tests are skipped). + pub fn rust(&self) -> bool { + self.workspace_wide || !self.packages.is_empty() + } +} + +/// Decide what to run. `packages` are (name, directory relative to the root, `/`-separated). +pub fn plan(changed: &[Changed], packages: &[(String, String)], base: String) -> Plan { + let mut p = Plan { base, files: changed.len(), ..Plan::default() }; + for c in changed { + let path = c.path.as_str(); + if WORKSPACE_WIDE.contains(&path) || WORKSPACE_WIDE_DIRS.iter().any(|d| path.starts_with(d)) { + p.workspace_wide = true; + } + if PARITY_DOCS.contains(&path) || c.gone { + p.parity = true; + } + // the innermost package directory containing the file + let owner = packages + .iter() + .filter(|(_, dir)| !dir.is_empty() && path.strip_prefix(dir.as_str()).is_some_and(|rest| rest.starts_with('/'))) + .max_by_key(|(_, dir)| dir.len()); + if let Some((name, _)) = owner { + p.packages.insert(name.clone()); + } + } + p.heif = p.workspace_wide || HEIF_PACKAGES.iter().any(|h| p.packages.contains(*h)); + p.parity |= p.workspace_wide || PARITY_PACKAGES.iter().any(|h| p.packages.contains(*h)); + p +} + +fn git(root: &Path, args: &[&str]) -> Result { + let out = Command::new("git").args(args).current_dir(root).output().map_err(|e| format!("git {}: {e}", args.join(" ")))?; + if !out.status.success() { + return Err(format!("git {}: {}", args.join(" "), String::from_utf8_lossy(&out.stderr).trim())); + } + Ok(String::from_utf8_lossy(&out.stdout).into_owned()) +} + +/// Parse `git diff --name-status` output. +fn parse_name_status(out: &str) -> Vec { + let mut v = Vec::new(); + for line in out.lines() { + let mut cols = line.split('\t'); + let (Some(status), Some(first)) = (cols.next(), cols.next()) else { continue }; + match (status.chars().next(), cols.next()) { + // renamed / copied: old path, new path + (Some(k @ ('R' | 'C')), Some(second)) => { + v.push(Changed { path: first.to_owned(), gone: k == 'R' }); + v.push(Changed { path: second.to_owned(), gone: false }); + } + (Some('D'), _) => v.push(Changed { path: first.to_owned(), gone: true }), + _ => v.push(Changed { path: first.to_owned(), gone: false }), + } + } + v +} + +/// The files changed since the merge base with `base` (default `origin/main`, else `main`): +/// committed, staged, unstaged and untracked. Returns the base description and the files. +pub fn changed_files(root: &Path, base: Option<&str>) -> Result<(String, Vec), String> { + let base = match base { + Some(b) => b.to_owned(), + None => ["origin/main", "main"] + .into_iter() + .find(|b| git(root, &["rev-parse", "--verify", "--quiet", &format!("{b}^{{commit}}")]).is_ok()) + .ok_or("no origin/main or main to compare with; pass --base REF")? + .to_owned(), + }; + let merge_base = git(root, &["merge-base", "HEAD", &base])?.trim().to_owned(); + let mut files = parse_name_status(&git(root, &["diff", "--name-status", "-M", &merge_base])?); + for path in git(root, &["ls-files", "--others", "--exclude-standard"])?.lines() { + files.push(Changed { path: path.to_owned(), gone: false }); + } + let short: String = merge_base.chars().take(10).collect(); + Ok((format!("{base} (merge base {short})"), files)) +} + +/// (name, root-relative directory) of each workspace package, from `cargo metadata`. +pub fn packages(root: &Path, metadata: &serde_json::Value) -> Vec<(String, String)> { + let roots: Vec = [Some(root.to_path_buf()), root.canonicalize().ok()].into_iter().flatten().collect(); + metadata["packages"] + .as_array() + .into_iter() + .flatten() + .filter_map(|p| { + let name = p["name"].as_str()?; + let dir = Path::new(p["manifest_path"].as_str()?).parent()?; + let rel = roots.iter().find_map(|r| dir.strip_prefix(r).ok())?; + let rel = rel.components().map(|c| c.as_os_str().to_string_lossy()).collect::>().join("/"); + Some((name.to_owned(), rel)) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ch(path: &str) -> Changed { + Changed { path: path.into(), gone: false } + } + + fn pkgs() -> Vec<(String, String)> { + [ + ("lightcraft-raw", "crates/raw"), + ("lightcraft-codecs", "crates/codecs"), + ("lightcraft-engine", "crates/engine"), + ("lightcraft-cli", "apps/lightcraft-cli"), + ("xtask", "xtask"), + ("lightcraft-raw-nested", "crates/raw/nested"), + ] + .into_iter() + .map(|(a, b)| (a.to_owned(), b.to_owned())) + .collect() + } + + #[test] + fn changed_files_pick_their_innermost_package() { + let p = plan(&[ch("crates/raw/src/lib.rs"), ch("crates/raw/nested/src/x.rs"), ch("crates/rawx/y.rs")], &pkgs(), "b".into()); + assert_eq!(p.packages.iter().map(String::as_str).collect::>(), ["lightcraft-raw", "lightcraft-raw-nested"]); + assert!(p.rust() && !p.workspace_wide && !p.heif && !p.parity); + } + + #[test] + fn docs_only_skip_the_builds_but_tracker_docs_run_parity() { + let p = plan(&[ch("README.md"), ch("docs/raw-parity.md")], &pkgs(), "b".into()); + assert!(!p.rust() && !p.parity); + let p = plan(&[ch("docs/gaps.md")], &pkgs(), "b".into()); + assert!(!p.rust() && p.parity); + } + + #[test] + fn workspace_files_heif_and_parity_triggers() { + let p = plan(&[ch("Cargo.lock")], &pkgs(), "b".into()); + assert!(p.workspace_wide && p.rust() && p.heif && p.parity); + let p = plan(&[ch("crates/codecs/src/heif.rs")], &pkgs(), "b".into()); + assert!(p.heif && !p.parity); + let p = plan(&[ch("crates/engine/src/commands.rs")], &pkgs(), "b".into()); + assert!(p.parity && !p.heif); + let p = plan(&[Changed { path: "crates/raw/src/old.rs".into(), gone: true }], &pkgs(), "b".into()); + assert!(p.parity, "a removed path may be cited by the tracker"); + } + + #[test] + fn name_status_parses_renames_and_deletions() { + let v = parse_name_status("M\ta.rs\nR087\told.rs\tnew.rs\nD\tgone.rs\nA\tadded.rs\n\n"); + assert_eq!( + v, + [ch("a.rs"), Changed { path: "old.rs".into(), gone: true }, ch("new.rs"), Changed { path: "gone.rs".into(), gone: true }, ch("added.rs"),] + ); + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 8a7d597b5..0a5e64532 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -5,6 +5,8 @@ mod assets; mod bench; +mod ci_lock; +mod ci_quick; mod ci_report; mod ico; mod layers; @@ -34,11 +36,15 @@ commands: web [--serve [port]] [--dev] build the browser app (apps/lightcraft-web) into /web-site/; --serve serves it on http://127.0.0.1: (default 8080) - ci [--hardware] [--summary-md] + ci [--hardware] [--summary-md] [--quick [--base REF]] fmt --check, clippy -D warnings, heif, test, GPU probe, parity refs, layers, assets, wasm (stops at first failure); writes target/ci/report.json (machine, GPU adapters, data, steps, skipped tests) and target/ci/summary.md; --hardware also runs the GPU tests on - every backend with a hardware adapter; --summary-md prints the Markdown summary + every backend with a hardware adapter; --summary-md prints the Markdown summary. + --quick: only what the changes since the merge base with origin/main (or REF) can + break: clippy, the changed packages' tests, and heif / parity / wasm when they apply. + One ci runs at a time per machine; others wait (LIGHTCRAFT_CI_SLOTS=N for N at once, + LIGHTCRAFT_CI_LOCK=off to skip the lock) corpus [--download] show where test corpora live; --download fetches PngSuite and CC0 raw samples (raw.pixls.us) into corpus/ and checks their sha256 stats [--exact] count tests and lines per crate (--exact: ask the test harness via `-- --list`) @@ -249,7 +255,16 @@ fn wasm_set() -> Result, String> { } fn cmd_wasm() -> Result<(), String> { - let set = wasm_set()?; + cmd_wasm_for(None) +} + +/// The wasm check of the wasm-safe crates, or only those of them in `only` (`ci --quick`). +fn cmd_wasm_for(only: Option<&std::collections::BTreeSet>) -> Result<(), String> { + let set: Vec = wasm_set()?.into_iter().filter(|p| only.is_none_or(|o| o.contains(p))).collect(); + if set.is_empty() { + println!("wasm32-unknown-unknown check: no changed crate builds for wasm32"); + return Ok(()); + } let mut results = Vec::new(); for pkg in &set { let mut c = cargo(); @@ -268,10 +283,36 @@ fn cmd_wasm() -> Result<(), String> { fn cmd_ci(args: &[&str]) -> Result<(), String> { let hardware = args.contains(&"--hardware"); let print_md = args.contains(&"--summary-md"); - if let Some(other) = args.iter().find(|a| !matches!(**a, "--hardware" | "--summary-md")) { + let quick = args.contains(&"--quick"); + let base_at = args.iter().position(|a| *a == "--base"); + let base = base_at.map(|i| args.get(i + 1).copied().ok_or("ci: --base needs a ref")).transpose()?; + let unknown = args + .iter() + .enumerate() + .find(|(i, a)| !matches!(**a, "--hardware" | "--summary-md" | "--quick" | "--base") && base_at.is_none_or(|b| *i != b + 1)); + if let Some((_, other)) = unknown { return Err(format!("ci: unknown option `{other}`\n\n{USAGE}")); } + if base.is_some() && !quick { + return Err("ci: --base only applies to --quick".into()); + } let root = root(); + let command = std::iter::once("cargo xtask ci").chain(args.iter().copied()).collect::>().join(" "); + // held until ci returns: one run at a time per machine (ci_lock.rs) + let _slot = ci_lock::acquire(&root, &command)?; + let plan = if quick { + let (base, files) = ci_quick::changed_files(&root, base)?; + let plan = ci_quick::plan(&files, &ci_quick::packages(&root, &metadata()?), base); + let packages: Vec<&str> = plan.packages.iter().map(String::as_str).collect(); + eprintln!("=== ci --quick: {} changed file(s) since {} ===", plan.files, plan.base); + eprintln!(" packages: {}", if packages.is_empty() { "none".to_owned() } else { packages.join(", ") }); + if plan.workspace_wide { + eprintln!(" a workspace-wide file changed: testing every package"); + } + Some(plan) + } else { + None + }; let report_dir = ci_report::prepare(&root)?; let started = std::time::Instant::now(); let started_unix = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0); @@ -296,6 +337,21 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { }; type Step<'a> = (&'static str, Box Result<(), String> + 'a>); + // `--quick`: which steps apply (all of them in a full run) and the packages to test + let wants = |step: &str| match &plan { + None => true, + Some(p) => match step { + "clippy" | "test" | "wasm" => p.rust(), + "heif" => p.heif, + "parity" => p.parity, + _ => true, + }, + }; + let mut test_args = vec!["test".to_owned()]; + match &plan { + Some(p) if !p.workspace_wide => test_args.extend(p.packages.iter().flat_map(|n| ["-p".to_owned(), n.clone()])), + _ => test_args.push("--workspace".to_owned()), + } let mut steps: Vec = vec![ ( "fmt", @@ -325,8 +381,15 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { test(&["test", "-p", "lightcraft-engine", "--lib", "--features", "lightcraft-codecs/heif", "heic"], "heif") }), ), - ("test", Box::new(|| test(&["test", "--workspace"], "test"))), + ( + "test", + Box::new(|| { + let a: Vec<&str> = test_args.iter().map(String::as_str).collect(); + test(&a, "test") + }), + ), ]; + steps.retain(|(name, _)| wants(name)); if hardware { steps.push(( "gpu-backends", @@ -360,10 +423,17 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { }), )); } - steps.push(("parity", Box::new(|| parity::run(&root, false)))); + if wants("parity") { + steps.push(("parity", Box::new(|| parity::run(&root, false)))); + } steps.push(("layers", Box::new(cmd_layers))); steps.push(("assets", Box::new(|| assets::run(&root)))); - steps.push(("wasm", Box::new(cmd_wasm))); + if wants("wasm") { + match plan.as_ref().filter(|p| !p.workspace_wide) { + Some(p) => steps.push(("wasm", Box::new(|| cmd_wasm_for(Some(&p.packages))))), + None => steps.push(("wasm", Box::new(cmd_wasm))), + } + } let mut results = Vec::new(); let mut failure = None; @@ -373,7 +443,7 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { let r = f(); let status = if r.is_ok() { "ok" } else { "fail" }; results.push(serde_json::json!({"name": name, "status": status, "seconds": t.elapsed().as_secs_f64()})); - if *name == "test" { + if *name == "test" && plan.is_none() { // right after the workspace tests, before parity and wasm build other feature sets probe(); } @@ -383,7 +453,8 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { } } if gpu.borrow().is_null() { - *gpu.borrow_mut() = serde_json::json!({"error": "not probed: the test step did not run"}); + let why = if plan.is_some() { "not probed: --quick" } else { "not probed: the test step did not run" }; + *gpu.borrow_mut() = serde_json::json!({"error": why}); } let done: Vec<&str> = steps.iter().take(results.len()).map(|(n, _)| *n).collect(); match &failure { @@ -407,7 +478,8 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { let ram_mb = available_ram_mb(); let report = serde_json::json!({ "schema": ci_report::SCHEMA, - "command": std::iter::once("cargo xtask ci").chain(args.iter().copied()).collect::>().join(" "), + "command": command, + "quick": plan.as_ref().map(|p| serde_json::json!({"base": p.base, "files": p.files, "packages": p.packages, "workspace_wide": p.workspace_wide})), "result": if failure.is_some() { "fail" } else { "pass" }, "failed_step": failure.as_ref().map(|(n, _)| *n), "started_unix": started_unix, From c3c4cf16aba424f47230ca348433d061d792c887 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Mon, 19 Oct 2026 11:51:28 +0200 Subject: [PATCH 2/4] ci --quick: build the workspace's test targets, run only the changed packages' binaries `cargo test -p ` resolves features for that selection only, so its artifacts differ from the full `ci`'s workspace build (and from a worktree warmed from one): it recompiled ~130 crates, dependencies included. Now `--quick` runs `cargo test --workspace --no-run` (fresh when the workspace was built before) and executes the test binaries of the changed packages, with the package directory as cwd like cargo does. Doc tests stay with the full `ci`. A one-file change in lightcraft-raw in a warmed worktree: 170 s for the whole `ci --quick`. Co-Authored-By: Claude Opus 5.5 --- xtask/src/main.rs | 65 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 0a5e64532..8d4126330 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -190,6 +190,60 @@ fn total_ram_gb() -> Option { Some(bytes.saturating_add(1 << 29) >> 30) } +/// `ci --quick`'s tests: build every workspace test target, then run only the test binaries of +/// `packages`. Building with `-p` instead would resolve features per selection: different +/// artifacts from `ci`'s and a warm worktree's (`cargo xtask warm`) workspace builds, so it would +/// recompile ~130 crates. Doc tests are left to the full `ci`. +fn test_selected(root: &Path, packages: &std::collections::BTreeSet, skip_log: &Path) -> Result<(), String> { + let manifests: std::collections::BTreeMap = metadata()?["packages"] + .as_array() + .into_iter() + .flatten() + .filter_map(|p| Some((p["manifest_path"].as_str()?.to_owned(), p["name"].as_str()?.to_owned()))) + .filter(|(_, name)| packages.contains(name)) + .collect(); + let what = "cargo test --workspace --no-run"; + eprintln!("$ {what} (then the test binaries of {})", packages.iter().map(String::as_str).collect::>().join(", ")); + let out = cargo() + .args(["test", "--workspace", "--no-run", "--message-format=json-render-diagnostics"]) + .stderr(std::process::Stdio::inherit()) + .output() + .map_err(|e| format!("{what}: failed to spawn: {e}"))?; + if !out.status.success() { + return Err(format!("{what}: exited with {}", out.status)); + } + let mut binaries = Vec::new(); + for line in String::from_utf8_lossy(&out.stdout).lines() { + let Ok(msg) = serde_json::from_str::(line) else { continue }; + if msg["reason"] != "compiler-artifact" || msg["profile"]["test"] != true { + continue; + } + let (Some(exe), Some(manifest)) = (msg["executable"].as_str(), msg["manifest_path"].as_str()) else { continue }; + if let Some(name) = manifests.get(manifest) { + binaries.push((name.clone(), manifest.to_owned(), exe.to_owned())); + } + } + if binaries.is_empty() { + println!("no test binaries in {}", packages.iter().map(String::as_str).collect::>().join(", ")); + return Ok(()); + } + let mut failed = Vec::new(); + for (name, manifest, exe) in &binaries { + let dir = Path::new(manifest).parent().unwrap_or(root); + let mut c = Command::new(exe); + c.current_dir(dir).env("CARGO_MANIFEST_DIR", dir).env("CARGO_PKG_NAME", name).env(ci_report::SKIP_LOG_ENV, skip_log); + if std::env::var_os("RUST_TEST_THREADS").is_none() { + // as the cargo runs of ci (`cargo()`): more threads make the frame-budget tests flaky + c.env("RUST_TEST_THREADS", std::thread::available_parallelism().map_or(4, |n| n.get()).min(4).to_string()); + } + let file = Path::new(exe).file_name().map(|f| f.to_string_lossy().into_owned()).unwrap_or_default(); + if run(c, &format!("{name}: {file}")).is_err() { + failed.push(format!("{name} ({file})")); + } + } + if failed.is_empty() { Ok(()) } else { Err(format!("tests failed in {}", failed.join(", "))) } +} + pub fn run(mut cmd: Command, what: &str) -> Result<(), String> { eprintln!("$ {what}"); let status = cmd.status().map_err(|e| format!("{what}: failed to spawn: {e}"))?; @@ -347,11 +401,6 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { _ => true, }, }; - let mut test_args = vec!["test".to_owned()]; - match &plan { - Some(p) if !p.workspace_wide => test_args.extend(p.packages.iter().flat_map(|n| ["-p".to_owned(), n.clone()])), - _ => test_args.push("--workspace".to_owned()), - } let mut steps: Vec = vec![ ( "fmt", @@ -383,9 +432,9 @@ fn cmd_ci(args: &[&str]) -> Result<(), String> { ), ( "test", - Box::new(|| { - let a: Vec<&str> = test_args.iter().map(String::as_str).collect(); - test(&a, "test") + Box::new(|| match &plan { + Some(p) if !p.workspace_wide => test_selected(&root, &p.packages, &ci_report::skip_log(&root, "test")), + _ => test(&["test", "--workspace"], "test"), }), ), ]; From 960e1f14e8e377cdddfcb4bf4d1b5f80308f91df Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Tue, 20 Oct 2026 13:10:53 +0200 Subject: [PATCH 3/4] Gap 9: ci --quick reads git's -z output, tests the crates that embed changed files; CI keeps main's runs - ci.yml: cancel-in-progress only for pull requests, so a push to main never cancels the run of the push before it. - ci --quick: `git diff --name-status -z -M` and `git ls-files -z`, parsed as NUL-separated fields (a rename or copy takes two paths), so a non-ASCII path, which git otherwise quotes and escapes, still maps to its package. - ci --quick: files outside a package that it compiles in (include_str!/include_bytes!, a build script's rerun-if-changed: fonts, camera profiles and the app icon under assets/, docs/whats-new.md, docs/raw/coverage-sweep.tsv, docs/raf-corpus.sha256, contributors/) count as a change to that package. A test scans the workspace and fails when one is not mapped. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 3 +- xtask/src/ci_quick.rs | 175 +++++++++++++++++++++++++++++++++++---- 2 files changed, 162 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5783ac54..181c5f081 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,8 @@ on: concurrency: group: ci-${{ github.ref }} - cancel-in-progress: true + # superseded PR pushes only: every push to main keeps its own full run + cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: contents: read diff --git a/xtask/src/ci_quick.rs b/xtask/src/ci_quick.rs index a9e98703b..3242c95af 100644 --- a/xtask/src/ci_quick.rs +++ b/xtask/src/ci_quick.rs @@ -12,6 +12,8 @@ //! deletions: the tracker cites paths); //! - the wasm check of the changed packages that must build for wasm32. //! +//! A file outside any package that a crate compiles in (`include_str!` of `docs/whats-new.md`, +//! fonts and camera profiles under `assets/`, ...) counts as a change to that crate (`EMBEDDED`). //! A change to a workspace-wide file (root `Cargo.toml`, `Cargo.lock`, lint or toolchain config) //! tests everything. The full `ci` (GitHub's CI on every PR) stays the merge gate. @@ -27,6 +29,26 @@ const WORKSPACE_WIDE_DIRS: &[&str] = &[".cargo/"]; const PARITY_PACKAGES: &[&str] = &["lightcraft-engine", "lightcraft-develop", "lightcraft-ui-egui", "lightcraft-cli"]; const PARITY_DOCS: &[&str] = &["docs/parity-checklist.md", "docs/gaps.md"]; const HEIF_PACKAGES: &[&str] = &["lightcraft-codecs", "lightcraft-heif"]; +/// Files outside a package's directory that it compiles in (`include_str!` / `include_bytes!`, +/// a build script's `rerun-if-changed`): a change to one counts as a change to those packages. +/// An entry ending in `/` covers the directory. `embedded_files_are_mapped` keeps this complete. +const EMBEDDED: &[(&str, &[&str])] = &[ + ("assets/app-icon/", &["lightcraft"]), + ("assets/camera-profiles/", &["lightcraft-engine"]), + ("assets/camera-tone/", &["lightcraft-engine"]), + ("assets/fonts/", &["lightcraft-engine", "lightcraft-ui-egui"]), + ("contributors/", &["lightcraft-ui-egui"]), + ("docs/whats-new.md", &["lightcraft-ui-egui"]), + ("docs/raw/coverage-sweep.tsv", &["lightcraft-raw"]), + ("docs/raf-corpus.sha256", &["lightcraft-raw"]), + ("xtask/src/main.rs", &["lightcraft-raw"]), + ("crates/ui-egui/src/menus.rs", &["xtask"]), +]; + +/// The packages that compile in `path` from outside their own directory. +fn embedders(path: &str) -> impl Iterator + '_ { + EMBEDDED.iter().filter(move |(p, _)| if p.ends_with('/') { path.starts_with(p) } else { path == *p }).flat_map(|(_, pkgs)| pkgs.iter().copied()) +} /// A changed path, relative to the workspace root; `gone` when deleted or renamed away. #[derive(Clone, Debug, PartialEq, Eq)] @@ -74,6 +96,11 @@ pub fn plan(changed: &[Changed], packages: &[(String, String)], base: String) -> if let Some((name, _)) = owner { p.packages.insert(name.clone()); } + for name in embedders(path) { + if packages.iter().any(|(n, _)| n == name) { + p.packages.insert(name.to_owned()); + } + } } p.heif = p.workspace_wide || HEIF_PACKAGES.iter().any(|h| p.packages.contains(*h)); p.parity |= p.workspace_wide || PARITY_PACKAGES.iter().any(|h| p.packages.contains(*h)); @@ -88,25 +115,34 @@ fn git(root: &Path, args: &[&str]) -> Result { Ok(String::from_utf8_lossy(&out.stdout).into_owned()) } -/// Parse `git diff --name-status` output. +/// Parse `git diff --name-status -z` output: NUL-separated fields, a status then its path, or +/// two paths (old, new) for a rename or copy. `-z` leaves non-ASCII paths unquoted. fn parse_name_status(out: &str) -> Vec { let mut v = Vec::new(); - for line in out.lines() { - let mut cols = line.split('\t'); - let (Some(status), Some(first)) = (cols.next(), cols.next()) else { continue }; - match (status.chars().next(), cols.next()) { + let mut fields = out.split('\0'); + while let Some(status) = fields.next() { + let Some(kind) = status.trim().chars().next() else { continue }; + let Some(first) = fields.next() else { break }; + match kind { // renamed / copied: old path, new path - (Some(k @ ('R' | 'C')), Some(second)) => { - v.push(Changed { path: first.to_owned(), gone: k == 'R' }); - v.push(Changed { path: second.to_owned(), gone: false }); + 'R' | 'C' => { + v.push(Changed { path: first.to_owned(), gone: kind == 'R' }); + if let Some(second) = fields.next() { + v.push(Changed { path: second.to_owned(), gone: false }); + } } - (Some('D'), _) => v.push(Changed { path: first.to_owned(), gone: true }), + 'D' => v.push(Changed { path: first.to_owned(), gone: true }), _ => v.push(Changed { path: first.to_owned(), gone: false }), } } v } +/// Parse NUL-separated `git ls-files -z` output. +fn parse_paths_z(out: &str) -> impl Iterator + '_ { + out.split('\0').filter(|p| !p.is_empty()).map(|p| Changed { path: p.to_owned(), gone: false }) +} + /// The files changed since the merge base with `base` (default `origin/main`, else `main`): /// committed, staged, unstaged and untracked. Returns the base description and the files. pub fn changed_files(root: &Path, base: Option<&str>) -> Result<(String, Vec), String> { @@ -119,10 +155,8 @@ pub fn changed_files(root: &Path, base: Option<&str>) -> Result<(String, Vec = parse_paths_z("crates/codecs/ünï code.rs\0").collect(); + assert_eq!(untracked, [ch("crates/codecs/ünï code.rs")]); + let p = plan(&[v, untracked].concat(), &pkgs(), "b".into()); + assert_eq!(p.packages.iter().map(String::as_str).collect::>(), ["lightcraft-codecs", "lightcraft-engine", "lightcraft-raw"]); + } + + #[test] + fn embedded_files_outside_a_crate_test_the_crates_that_embed_them() { + let p = plan(&[ch("docs/raw/coverage-sweep.tsv")], &pkgs(), "b".into()); + assert!(p.rust()); + assert_eq!(p.packages.iter().map(String::as_str).collect::>(), ["lightcraft-raw"]); + let p = plan(&[ch("assets/fonts/Inter-Regular.ttf")], &pkgs(), "b".into()); + assert!(p.rust() && p.packages.contains("lightcraft-engine")); + // a consumer missing from the workspace's package list is not invented + assert!(!p.packages.contains("lightcraft-ui-egui")); + let p = plan(&[ch("assets/ATTRIBUTION.md"), ch("docs/raw-parity.md")], &pkgs(), "b".into()); + assert!(!p.rust(), "only files something compiles in"); + } + + /// Every `include_str!` / `include_bytes!` of a file outside its package, and every `"../…"` + /// path in a build script, is listed in `EMBEDDED` for that package. + #[test] + fn embedded_files_are_mapped() { + use std::path::{Component, PathBuf}; + fn rs_files(dir: &Path, out: &mut Vec) { + for e in std::fs::read_dir(dir).into_iter().flatten().flatten() { + let p = e.path(); + if p.is_dir() && p.file_name().is_some_and(|n| n != "target") { + rs_files(&p, out); + } else if p.extension().is_some_and(|x| x == "rs") { + out.push(p); + } + } + } + let root = Path::new(env!("CARGO_MANIFEST_DIR")).parent().expect("xtask is in the workspace").to_path_buf(); + // `a/b/../c` -> `a/c`, relative to the root; None outside it + let normalize = |p: &Path| -> Option { + let mut parts: Vec = Vec::new(); + for c in p.strip_prefix(&root).ok()?.components() { + match c { + Component::ParentDir => { + parts.pop()?; + } + Component::Normal(s) => parts.push(s.to_string_lossy().into_owned()), + _ => {} + } + } + Some(parts.join("/")) + }; + let mut files = Vec::new(); + for d in ["crates", "apps", "xtask"] { + rs_files(&root.join(d), &mut files); + } + let (mut checked, mut missing) = (0, Vec::new()); + for file in files { + let Ok(src) = std::fs::read_to_string(&file) else { continue }; + // the package: the nearest directory with a Cargo.toml + let Some(pkg_dir) = file.ancestors().skip(1).find(|d| d.join("Cargo.toml").is_file()) else { continue }; + let manifest = std::fs::read_to_string(pkg_dir.join("Cargo.toml")).unwrap_or_default(); + let Some(name) = manifest.lines().find_map(|l| l.strip_prefix("name = \"")?.strip_suffix('"')) else { continue }; + let Some(pkg_rel) = normalize(pkg_dir) else { continue }; + let mut targets = Vec::new(); + for mac in ["include_str!(", "include_bytes!("] { + for (i, _) in src.match_indices(mac) { + let rest = src[i + mac.len()..].trim_start(); + if let Some(lit) = rest.strip_prefix('"').and_then(|r| r.split('"').next()) { + targets.push(file.parent().expect("a file has a directory").join(lit)); + } + } + } + if file.file_name().is_some_and(|n| n == "build.rs") { + for (i, _) in src.match_indices("\"../") { + if let Some(lit) = src[i + 1..].split('"').next() { + targets.push(pkg_dir.join(lit)); + } + } + } + for t in targets { + // outside the workspace (`../craft-fonts`), not a real file, or inside the package + let Some(rel) = normalize(&t) else { continue }; + if !root.join(&rel).exists() || rel.starts_with(&format!("{pkg_rel}/")) { + continue; + } + checked += 1; + // a directory (`rerun-if-changed=../../contributors`) needs a `dir/` entry + let rel = if root.join(&rel).is_dir() { format!("{rel}/") } else { rel }; + if !embedders(&rel).any(|p| p == name) { + missing.push(format!("{rel} (compiled into {name} by {})", file.display())); + } + } + } + assert!(checked > 0, "found no embedded files: the scan is broken"); + assert!(missing.is_empty(), "add these to ci_quick::EMBEDDED:\n{}", missing.join("\n")); } } From 10c4d0c0ab279f51089dc425947b0134109dd95a Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Tue, 20 Oct 2026 15:07:19 +0200 Subject: [PATCH 4/4] Gap 9: only a PR's own pushes share a CI concurrency group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A group holds one active and one pending run, so `ci-${{ github.ref }}` let a later push to main replace a queued run and skip its `cargo xtask ci`. Pull request pushes keep the shared group (that supersession is the point); every other run — a push to main, a manual one — now gets `ci-`, a group of its own, and always runs. Co-Authored-By: Paperclip --- .github/workflows/ci.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 181c5f081..f2afa9537 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,8 +14,11 @@ on: workflow_dispatch: concurrency: - group: ci-${{ github.ref }} - # superseded PR pushes only: every push to main keeps its own full run + # A pull request's pushes share one group, so a new push supersedes the run of the old one. + # Every other run (a push to main, a manual one) gets a group of its own: a group keeps only one + # active and one pending run, so sharing one would let a later push replace a queued run and skip + # its `cargo xtask ci` — and every push to main is meant to get a full run. + group: ${{ github.event_name == 'pull_request' && format('ci-{0}', github.ref) || format('ci-{0}', github.run_id) }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: