Skip to content

ci: run the screenshot job in the toolchain container - #35

Open
spacedevin wants to merge 1 commit into
mainfrom
ci/screenshot-container
Open

spacedevin wants to merge 1 commit into
mainfrom
ci/screenshot-container

Conversation

@spacedevin

Copy link
Copy Markdown
Member

Motivation

screenshot.yml fires on every PR that touches crates/** (most of them) and, in parallel with the verify matrix, rebuilt its whole toolchain by hand: apt-get install of imagemagick/freetype/cmake/…, a libmgba 0.10.5 source build (cached, but only that one piece), dtolnay/rust-toolchain for the nightly, cargo install --locked agb-gbafix compiled from source every run, and setup-node. ci.yml's rom and verify jobs had already moved every one of those into ghcr.io/schlopai/chuggie-toolchain — the image's Dockerfile lists them as exactly the per-job cost it exists to remove — leaving this as the one job still paying the old way. It also builds the same overworld-demo ROM that verify overworld-demo builds in the same PR run.

Change

Of the two options (fold into ci.yml, or reuse the container), this is the smaller one: the job keeps its own workflow, trigger paths, name and artifact, and only the setup changes.

  • container: is the same image and the same tag ci.yml pins (8f52ecdbceed), with the same credentials: block; a comment says to move the two together.
  • permissions: { contents: read, packages: read } — packages: read is what the ghcr pull needs, as in ci.yml.
  • Removed: the apt step, the libmgba cache + source build, the rust toolchain step, cargo install agb-gbafix, setup-node, and the explicit MGBA_PREFIX: /home/runner/mgba-prefix env. The image sets MGBA_PREFIX=/usr/local/mgba, which scripts/shot_common.sh's find_mgba_prefix reads first; ensure_gba_shot then compiles tools/gba-shot.c against it with the image's cc, and screenshot.sh finds the image's ImageMagick convert for the PPM→PNG step.
  • Kept as-is: checkout, npm ci (with its comment about .cargo/config.toml's patch), the screenshot.sh invocation with TISH, the upload step, timeout-minutes: 40.

About the libmgba cache. The brief was to keep it; I removed it instead and want to flag that explicitly. In the container the library is prebuilt at /usr/local/mgba and nothing would ever populate or read ~/mgba-prefix, so the cache step would be dead weight alongside a Build libmgba step that never runs. The property the cache was providing — not rebuilding libmgba per run — is what the image provides now. If you would rather keep the fallback path for a runner without the image, say so and I will put it back as a conditional.

Verification

actionlint and a YAML parse are clean. Runtime check is the PR's own run of this workflow (it triggers on .github/workflows/screenshot.yml).

Not done here

verify overworld-demo in ci.yml already builds, boots and checks that this ROM draws something (scripts/verify_default.sh); the only thing this workflow adds is the uploaded PNG. Whether that is worth a separate ~2-minute job per PR is a product call, not made in this PR.

screenshot.yml still assembled its toolchain by hand on every PR touching crates/**: apt
(imagemagick, freetype, cmake...), a libmgba 0.10.5 source build behind an actions/cache,
a rustup nightly with rust-src, `cargo install agb-gbafix` compiled from source every run,
and setup-node. ci.yml's rom and verify jobs had already moved all of that into
ghcr.io/schlopai/chuggie-toolchain; this was the one job left paying the old way, and it
ran in parallel with a verify matrix that was building the same overworld-demo ROM.

The job now runs in the same image, pinned to the same tag as ci.yml. The libmgba cache
step goes with the rest of the setup: the library is prebuilt in the image at MGBA_PREFIX,
which scripts/shot_common.sh already reads, so there is nothing left for the cache to hold.
What the job proves is unchanged — build overworld-demo with the pinned tish, boot it under
libmgba headless, upload the frame.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant