fix: install system dependencies even on a cache hit - #1
Merged
Conversation
The march binary dynamically links against libblake3, libbrotli, libzstd
and llvm. Those install into system paths (/usr/local/lib, /usr/lib/...),
which are OUTSIDE the cached path (~/.local/march). Gating the
'Install system dependencies' step on cache-hit != 'true' therefore
restored the binary on a cache hit without the shared objects it needs,
and every march invocation failed with exit 127 — the loader cannot
resolve a needed .so, which bash reports as 'command not found'.
The failure is self-inflicted and delayed, which is what made it
confusing: the cold-cache run passes AND populates the cache, so the
NEXT run breaks and stays broken until the cache is evicted. Observed in
march-lean CI: the run that populated the cache passed, then main and
every subsequent PR failed with all 171 corpus files erroring
('--emit-core-ast produced zero bytes; march exit=127').
Ungate the step and document why it must stay ungated. The expensive
work — OCaml setup and the ~2.5 min March build — remains cache-gated,
so cached runs stay fast. Also rm -rf the blake3 clone dir first, since
an unconditional step may now run twice in a job that invokes the action
more than once.
Verified: action.yml parses; only 'Set up OCaml' and 'Clone, build, and
install March' remain cache-gated, both genuinely build-only.
…xed path Review feedback: rm -rf on a fixed /tmp/blake3 is a blunt way to make an unconditional step re-runnable. A unique mktemp -d per run cannot collide in the first place, so there is nothing to delete — and it matches the idiom the 'Clone, build, and install March' step below already uses. Also guard the cd (shellcheck SC2164). bash -e would abort the step anyway, but being explicit costs nothing.
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
1 participant
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.
The bug
marchdynamically links against libblake3, libbrotli, libzstd and llvm.Those install into system paths (
/usr/local/lib,/usr/lib/...) — which areoutside the cached path (
~/.local/march).The
Install system dependenciesstep was gated onsteps.cache.outputs.cache-hit != 'true'. So on a cache hit the actionrestored the
marchbinary but never installed the shared objects it needs.Every invocation then failed with exit 127 — the loader cannot resolve a
needed
.so, which bash surfaces as "command not found".Why it was confusing
The failure is self-inflicted and delayed. The cold-cache run installs the
deps, builds march, passes — and populates the cache. The next run gets a
cache hit, skips the deps, and breaks. It then stays broken until the cache is
evicted.
Observed in
march-leanCI:Every one of the 171 corpus files errored with
--emit-core-ast produced zero bytes; march exit=127, so the conformanceharness reported
MATCH: 0 / ERROR: 171.mainhas been red since.The fix
Ungate the step so system dependencies install on every run, and document why
it must stay ungated — the comment is the point, so this doesn't get
"optimized" back into a gate.
The genuinely expensive work stays cached:
Set up OCamland the ~2.5 minClone, build, and install Marchremain gated on cache miss. Cached runs payonly the apt install plus the blake3 compile.
Clone blake3 into a fresh
mktemp -drather than a fixed/tmp/blake3: nowthat the step is unconditional it can run twice in a job that invokes the action
more than once, and
git cloneinto an existing non-empty directory is a harderror. A unique directory per run cannot collide, so nothing needs deleting —
and it matches the idiom the
Clone, build, and install Marchstep alreadyuses. The
cdis guarded (shellcheck SC2164).Verification
action.ymlparses as valid YAML.Step gating after the change — only build-only steps remain gated:
The real end-to-end proof is a
march-leanCI run that gets a cache hitand still passes — worth confirming after merge, since that's the exact path
that is broken today.
Possible follow-up
A future optimization could split true runtime libraries from build-only
-devpackages, or cache the system libs alongside the binary. I kept this change
minimal and obviously-correct rather than clever, given main is currently red.