fix(turboquant,bonsai): do not apply vendored llama.cpp patches to fork trees#10866
Merged
Merged
Conversation
…rk trees The turboquant and bonsai backends copy backend/cpp/llama-cpp/ wholesale into their build directories and reuse its Makefile/prepare.sh against their own llama.cpp forks. When PR #10837 added backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch, the copied patches/ directory was mis-applied to the fork checkouts: the fork trees diverge from upstream, hunks rejected, and because the patch-apply loop in prepare.sh ran before set -e took effect the build kept going and died much later with a confusing compile error ("'LLM_ARCH_MINIMAX_M3' was not declared in this scope"). This broke tests-turboquant-grpc on that PR. Two hardening changes: - turboquant/bonsai Makefiles: delete the copied patches/ directory right after the cp -rf of backend/cpp/llama-cpp/. Patches vendored for upstream llama.cpp must never be applied to the forks; each fork carries its own patch series under backend/cpp/<backend>/patches/, applied by its apply-patches.sh. - llama-cpp prepare.sh: run the patch-apply loop under set -e so a rejecting patch fails fast and loudly at apply time instead of surfacing as a downstream compile error. A missing or empty patches/ directory remains a no-op success, so all existing callers (the llama-cpp Makefile targets and the turboquant/bonsai copies) are unaffected when no patches ship. Exposed by PR #10837. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
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
2 participants
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.
What
Hardening exposed by #10837. That PR adds
backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch, and the turboquant and bonsai backends copybackend/cpp/llama-cpp/wholesale (cp -rf) into their build directories before running the copied Makefile/prepare.sh against their own llama.cpp forks (TheTom/llama-cpp-turboquant, PrismML). The copiedpatches/directory got mis-applied to the fork trees: hunks rejected, and because the patch-apply loop inprepare.shran beforeset -etook effect, the build continued and died much later with a confusing compile error ('LLM_ARCH_MINIMAX_M3' was not declared in this scope). This broketests-turboquant-grpcon #10837.Changes
patches/directory right after thecp -rfofbackend/cpp/llama-cpp/(all four copy sites: both build defines and both cpu-all targets). Patches vendored for upstream llama.cpp must never be applied to the forks; each fork carries its own patch series underbackend/cpp/<backend>/patches/, applied by its ownapply-patches.sh.backend/cpp/llama-cpp/prepare.sh: movedset -eabove the patch-apply loop so a rejecting patch fails fast and loudly at apply time instead of surfacing as a downstream compile error. A missing or emptypatches/directory remains a no-op success, so no existing caller changes behavior when no patches ship (ik-llama-cpp is unaffected as it ships its own prepare.sh).Verification
Temp-dir simulation: a rejecting patch now exits non-zero at apply time; missing and empty
patches/dirs still succeed; the copiedpatches/dir is confirmed removed by the new Makefile step;make -non the affected targets confirms recipe expansion is intact. Notebackend/cpp/llama-cpp/patches/does not exist on master yet, so on this PR alone the newrm -rfis a no-op; the real proof is #10837 rebased on top of this going green ontests-turboquant-grpc.Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Ping @mudler