ci: point the dl cache at the directory buildroot actually uses - #2355
Conversation
build.yml and build-one.yml cache `output/dl` and prune moving-ref tarballs from it. Buildroot never wrote there. DL_DIR defaults to $(TOPDIR)/dl, and TOPDIR is the source tree the Makefile passes to -C, so downloads landed in output/buildroot-$(BR_VER)/dl: $ make -C output/buildroot-2024.02.10 O=$PWD/output -p | grep '^DL_DIR' DL_DIR := /home/dima/git/firmware/output/buildroot-2024.02.10/dl output/dl therefore never existed. actions/cache only archives paths that exist, so the cache saved nothing and restored nothing, and the refresh find matched nothing -- both silently, the find because of its own `2>/dev/null || true`. The two failures cancelled to "every board re-downloads every tarball, every run", which is also why CI never hit the stale rolling tarball that #2352 fixed for local builds. Setting BR2_DL_DIR in the environment moves buildroot to the directory the cache already keys on, rather than teaching three more places to spell output/buildroot-$(BR_VER)/dl. Buildroot reads it ahead of .config by design, and the Makefile's expiry from #2352 already prefers $(BR2_DL_DIR): $ BR2_DL_DIR=$PWD/output/dl make -C ... -p | grep '^DL_DIR' DL_DIR := /home/dima/git/firmware/output/dl Turning the cache on makes the refresh live for the first time, which is the half that keeps it safe -- do not restore one without the other. Its regex covers the moving-ref class this tree actually produces: 24 packages pin _VERSION = HEAD and download as <pkg>-HEAD.tar.gz, majestic-webui as -dist.tar.gz, majestic's S3 tarball as .master.tar.bz2. Everything else in a real 201-file dl is semver- or SHA-named and immutable. The suppression goes with it. A cold cache is legitimate and now says so; anything else is an error that reaches the log.
PR Summary by QodoFix Buildroot download caching in CI workflows
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Matrix cache freezes partial downloads
|
Review on #2355: `dl-<month>` is a single key shared by every board, and actions/cache saves only on a key miss, so the first job to finish freezes that month's snapshot. build-one.yml builds ONE board on manual dispatch. Letting it win that race would pin the shared cache to one board's dependency set for the rest of the month, which is a worse outcome than the partial snapshot the matrix itself produces. Restore-only there. It consumes the cache; it does not get to define it. The matrix keeps the one shared key on purpose. Per-board dl caches would be the obvious alternative and are the wrong trade here: the repo already holds 197 active caches totalling 15.3 GB, because ccache is keyed per board, and those entries buy far more build time than a per-board download set would. A second per-board series would evict them.
Confirmed on this PR's own runFrom Four things this shows:
The cache is restore-only on pull requests by design, so this run does not populate it; the nightly |
Problem
build.ymlandbuild-one.ymlcacheoutput/dland prune moving-ref tarballs from it. Buildroothas never written there.
DL_DIRdefaults to$(TOPDIR)/dl, andTOPDIRis the source tree theMakefile hands to
-C, notO=:So
output/dlnever existed. Two things followed, both silent:actions/cachesaved nothing. It only archives paths that exist, so thedl-<month>key wasnever populated and never restored. Every board has been re-downloading every source tarball,
every run.
find output/dl … 2>/dev/null || trueswallowed the missingdirectory, so the moving-ref protection it exists to provide has never once run.
The two cancel out to "CI is correct but pays a full download for every board on every run" — which
is also why CI escaped the stale rolling tarball that bit local from-source builds in #2352. Correct
by accident, and only while both halves stayed broken.
What this does
Sets
BR2_DL_DIRin the environment so buildroot writes to the directory the cache already keys on,rather than teaching three more call sites to spell
output/buildroot-$(BR_VER)/dland keep it instep with
BR_VER. Buildroot reads it ahead of.configby design — "To make sure that theenvironment variable overrides the .config option, set this before including .config" — and the
Makefile's expiry from #2352 already prefers
$(BR2_DL_DIR)when set:Both workflows are fixed;
build-one.ymlcarries the same two steps and the same bug.Why turning the cache on is safe
Restoring the cache also makes the refresh step live for the first time, and that is the half that
keeps it safe — neither should be restored without the other. Its regex covers the moving-ref
class this tree actually produces:
_VERSION = HEAD(24 packages)ipctool-HEAD.tar.gzmajestic-webui-dist.tar.gzmajestic.hi3516cv500.lite.master.tar.bz2hisilicon-opensdk-<sha>.tar.gzMeasured against a real 201-file
dlfrom local builds: 9 files are moving-ref and get re-fetched,192 are content-addressed and cache correctly. No
_VERSIONin the tree pins a branch outsideHEAD/master/main/dist, so nothing moving escapes the regex.The suppression goes too
2>/dev/null || trueis how this hid for months. A cold cache is a legitimate state and now saysso out loud; anything else is a real error that reaches the log.
Verification
The
DL_DIRvalues above are the real check: buildroot itself, asked before and after, reporting thedirectory it will use.
Scope
CI plumbing.
ci-matrix.py --stdinselects the smoke set forbuild.yml; no image content changes —the same sources are built, they are merely fetched once instead of every run.