Repository navigation
third_party: bump llama.cpp to ab79894d (the pin cannot build: arch_name() is a std::string) - #1267
bong-water-water-bong wants to merge 1 commit into
Conversation
…ame() is a std::string) Root cause: the pin main carries, ced49c61, does not compile. Its src/llama-context.cpp:160 passes `model.arch_name()` - a `std::string` - to a printf-style variadic (`LLAMA_LOG_WARN`, our own engine #681 comment sits above it), which clang rejects outright: src/llama-context.cpp:160:42: error: cannot pass non-trivial object of type 'std::string' (aka 'basic_string<char>') to variadic function; expected type from format string was 'char *' [-Wnon-pod-varargs] so no payload can be rolled from main, and cycle 31 (the take stick) is blocked on it as well as on the Lemonade bump. This is the only call of its class in the tree: enumerating every `arch_name()` use at the pin leaves this one (the rest are `std::runtime_error(str + ...)` or typed parameters). Fix: the fork already has it as ab79894d (fork PR #220) - one token, `model.arch_name().c_str()`. This commit only moves the pin to it. Measured, both with /usr/lib/llvm-19/bin/clang++ -std=c++17 -fsyntax-only on the tree extracted from each commit (include/, src/, ggml/include/): * at ced49c61 -> 1 error, the -Wnon-pod-varargs above * at ab79894d -> clean, no errors, and the line reads `__func__, model.arch_name().c_str());` What proves it end to end is the payload roll after this merges (the engine build itself is gcc and only warns here; the payload's build is the one that errors).
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at e99395e8. The pin move is right and the fix is where you say it is; the bump is missing the
registry line that CI checks for exactly this, so as it stands the PR lands registry_pins red.
What is right. ab79894d is on the pin branch (#220's squash) and carries the one-token fix:
src/llama-context.cpp:160 __func__, model.arch_name().c_str());
so the claim "the fork already has it" is true, and the gitlink moves ced49c61 -> ab79894d with nothing else in the diff.
What blocks it. registry/architectures.json still records the old pin, and the repo has a test for that:
CMakeLists.txt:545 registers registry_pins -> tools/registry_build.py --check-pins, which ci.yml:96 runs in ctest.
At this head:
$ python3 tools/registry_build.py --check-pins
/…/registry/architectures.json: llama.cpp (hrx) recorded at ced49c61b, pinned at ab79894d2
a pin moved without regenerating the registry: check out the submodules at their pins
(git submodule update --init) and run tools/registry_build.py
rc 1
and with that one line changed (registry/architectures.json:4, "llama.cpp (hrx)": "ced49c61ba92…" ->
"ab79894d2f2431566ab14894b655d53c0118e2b7"):
$ python3 tools/registry_build.py --check-pins
registry/architectures.json records the committed pins rc 0
The counts do not move (this commit touches one .cpp line and no architecture), which is why the regenerated file differs
from yours by exactly that string — so the fix is that line, or the real regeneration if you prefer the tool to write it.
Nothing else in the tree references the old sha (grep over tracked .md/.json: no hits), so there is no second place to
update.
Why I am not waving it through: this is the PR that unblocks the c31 payload roll, and the failure it would land is a
ctest step whose whole purpose is "a pin moved without regenerating the registry" — the one message a reviewer six weeks
from now will read as "the bump was done carelessly". It is one line and it makes the gate green in the same commit.
Everything else in the body checks out: the clang-19 -fsyntax-only A/B is the same failure I reproduced on this box with
clang 24 at ced49c61 (-Wnon-pod-varargs at llama-context.cpp:160:42), and arch_name() being the only call of its
class in the tree matches my own git grep. And you are right that engine CI will not show the build — the engine compiles
it with gcc, where this is a warning.
|
Duplicate of #1265, and equivalent — measured: Both bumps are exactly 1 commit on top of I opened this one because it pins the branch the engine has been tracking ( |
Blocker: engine main cannot roll a payload right now. The pin it carries,
ced49c61, does not compile.src/llama-context.cpp:160passesmodel.arch_name()— astd::string— into a printf-style variadic (LLAMA_LOG_WARN; our ownengine #681comment sits directly above it), and clang refuses:It is the only call of its class in the tree: enumerating every
arch_name()use at the pin leaves this one (the rest arestd::runtime_error(str + …)or typed parameters).Fix: the fork already has it —
ab79894d(fork PR #220), one token,model.arch_name().c_str(). This PR only moves the pin.Measured both ways,
/usr/lib/llvm-19/bin/clang++ -std=c++17 -fsyntax-onlyon the tree extracted from each commit (include/,src/,ggml/include/):ced49c61(main now)-Wnon-pod-varargsaboveab79894d(this PR)__func__, model.arch_name().c_str());Why it blocks cycle 31: c31 is the take stick and needs a fresh payload (the Lemonade bump
64eb505 -> 52a2a6bis a submodule pin, so an sfs patch cannot carry it). No payload can be built from main until this lands. ci on this PR will not show the build (the engine build is gcc and only warns here); the payload roll after the merge is what proves it end to end — pi-os3 is rebuilding the failed targets with the same one-line change in the meantime and has the failing attempt on real source.Merging is the owner's (overnight rule: no merges by lanes).