Skip to content

Commit 621c612

Browse files
localai-botmudler
andauthored
ci(bump-deps): register ds4 + move version pin into the Makefile (#9761)
* ci(bump-deps): register ds4 + move version pin into the Makefile The initial ds4 PR (#9758) put the upstream commit pin in backend/cpp/ds4/prepare.sh as a shell variable. The auto-bump bot at .github/bump_deps.sh greps for ^$VAR?= in a Makefile, so DS4_VERSION was invisible to it - other backends (llama-cpp, ik-llama-cpp, turboquant, voxtral, etc.) all pin in their Makefile. This change: - Moves DS4_VERSION?= and DS4_REPO?= to the top of backend/cpp/ds4/Makefile. - Inlines the git init/fetch/checkout recipe into the 'ds4:' target (matches llama-cpp's 'llama.cpp:' target pattern). Directory acts as the target so make only re-clones when missing. - Deletes the now-redundant prepare.sh. - Adds antirez/ds4 + DS4_VERSION + main + backend/cpp/ds4/Makefile to the .github/workflows/bump_deps.yaml matrix so the daily bot opens PRs against this pin. - Updates .agents/ds4-backend.md to point at the Makefile. Verified: $ grep -m1 '^DS4_VERSION?=' backend/cpp/ds4/Makefile DS4_VERSION?=ae302c2fa18cc6d9aefc021d0f27ae03c9ad2fc0 $ make -C backend/cpp/ds4 ds4 # clones into ds4/ at the pin $ make -C backend/cpp/ds4 ds4 # no-op on second invocation make: 'ds4' is up to date. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * ci: route backend/cpp/ds4/ changes through changed-backends.js scripts/changed-backends.js:inferBackendPath has an explicit branch per cpp dockerfile suffix (ik-llama-cpp, turboquant, llama-cpp). Without a matching branch the function returns null, the backend never lands in the path map, and PR change-detection cannot map "backend/cpp/ds4/X changed" -> "rebuild ds4 image". This is why PR #9761 produced zero ds4 jobs even though it directly edits backend/cpp/ds4/Makefile. Adds the missing branch (Dockerfile.ds4 -> backend/cpp/ds4/), placed before the llama-cpp branch (since both share the .cpp ancestry but ds4 is more specific - same ordering rule documented in .agents/adding-backends.md). Verified with a local Node simulation of the script against this PR's diff: the path map now contains 'ds4 -> backend/cpp/ds4/' and a 'backend/cpp/ds4/Makefile' change correctly triggers the ds4 backend in the rebuild set. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * docs(adding-backends): harden the two gotchas that bit ds4 Both omissions are silent at the time you ADD a backend - the failure mode only appears later (the bump bot stays silent forever, or the path filter shows up on the next PR that touches your backend with zero CI jobs and looks broken for unrelated reasons). Expanding the `scripts/changed-backends.js` paragraph from a one-liner to a fully worked example, and adding a new sibling paragraph for the `bump_deps.yaml` + Makefile-pin contract. Both call out the specific mistakes from the ds4 timeline (#9758#9761) so future contributors can pattern-match on the cause. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> --------- Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
1 parent e3f9de1 commit 621c612

6 files changed

Lines changed: 94 additions & 44 deletions

File tree

.agents/adding-backends.md

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,55 @@ The build matrix is data-only YAML at `.github/backend-matrix.yml` (not inside `
3434

3535
**Without an entry here no image is ever built or pushed, and the gallery entry in `backend/index.yaml` will point at a tag that does not exist.** The `dockerfile:` field must point at `./backend/Dockerfile.<lang>` matching the language bucket from step 1 (e.g. `Dockerfile.python`, `Dockerfile.golang`, `Dockerfile.rust`). The `tag-suffix` must match the `uri:` in the corresponding `backend/index.yaml` image entry exactly.
3636

37-
If you add a new language bucket, `scripts/changed-backends.js` also needs a branch in `inferBackendPath` so PR change-detection routes file edits correctly.
37+
**`scripts/changed-backends.js` registration — REQUIRED for any new dockerfile suffix.** This is the single most common omission, because it has no effect on the PR that adds the backend (when no prior path filter could catch it anyway) — it only breaks the *next* PR that touches your backend's directory, which then gets zero CI jobs and looks broken for unrelated reasons. Edit `scripts/changed-backends.js:inferBackendPath` and add a branch BEFORE the more-generic suffixes:
38+
39+
```js
40+
if (item.dockerfile.endsWith("<your-dockerfile-suffix>")) {
41+
return `backend/cpp/<your-backend>/`; // or backend/python|go|rust/...
42+
}
43+
```
44+
45+
The `endsWith()` test is against the matrix entry's `dockerfile:` value (e.g. `./backend/Dockerfile.ds4``endsWith("ds4")`). Specificity order matters here just like it does for importers: more-specific suffixes go BEFORE more-generic ones (e.g. `ds4` before `llama-cpp` even though both end with letters, because some upstream might one day call itself `super-ds4-llama-cpp`). Verify locally before pushing:
46+
47+
```bash
48+
# Confirm your dockerfile suffix is unique enough
49+
node -e "
50+
const yaml = require('js-yaml'); const fs = require('fs');
51+
const m = yaml.load(fs.readFileSync('.github/backend-matrix.yml','utf8'));
52+
for (const e of m.include.filter(e => e.backend === '<your-backend>')) {
53+
console.log(e.dockerfile, '->', e.dockerfile.endsWith('<suffix>'));
54+
}"
55+
```
56+
57+
A quick way to find the right insertion point: `grep -n 'item.dockerfile.endsWith' scripts/changed-backends.js`.
58+
59+
**`bump_deps.yaml` registration — REQUIRED for any backend pinning an upstream commit.** If your backend's Makefile has a `*_VERSION?=<sha>` pin to a third-party repo, the daily auto-bump bot at `.github/workflows/bump_deps.yaml` won't notice it unless you register the backend in its matrix. The bot runs `.github/bump_deps.sh` which `grep`s for `^$VAR?=` in the Makefile you list — so the pin MUST live in the Makefile (not in a separate shell script). The bump for ds4 (#9761) had to walk this back because the original landed the pin in `prepare.sh`, which the bot can't see. Pattern (for `antirez/ds4`):
60+
61+
```yaml
62+
# .github/workflows/bump_deps.yaml
63+
matrix:
64+
include:
65+
- repository: "antirez/ds4"
66+
variable: "DS4_VERSION"
67+
branch: "main"
68+
file: "backend/cpp/ds4/Makefile"
69+
```
70+
71+
And the corresponding Makefile shape (mirror `backend/cpp/llama-cpp/Makefile`):
72+
73+
```makefile
74+
DS4_VERSION?=ae302c2fa18cc6d9aefc021d0f27ae03c9ad2fc0
75+
DS4_REPO?=https://github.com/antirez/ds4
76+
...
77+
ds4:
78+
mkdir -p ds4
79+
cd ds4 && git init -q && \
80+
git remote add origin $(DS4_REPO) && \
81+
git fetch --depth 1 origin $(DS4_VERSION) && \
82+
git checkout FETCH_HEAD
83+
```
84+
85+
If you have a `prepare.sh` doing the clone, delete it — the recipe belongs in the Makefile target so `make purge && make` works as a clean-and-rebuild and so the bump bot finds the pin.
3886

3987
**Placement in file:**
4088
- CPU builds: Add after other CPU builds (e.g., after `cpu-chatterbox`)

.agents/ds4-backend.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@ LocalAI wraps the engine's C API (`ds4/ds4.h`) with a fresh C++ gRPC server at
66

77
## Pin
88

9-
`backend/cpp/ds4/prepare.sh` clones `antirez/ds4` at `DS4_VERSION`. Bump that
10-
commit to follow upstream.
9+
`backend/cpp/ds4/Makefile` pins `DS4_VERSION?=<sha>` at the top. The `ds4`
10+
target in the Makefile clones `antirez/ds4` at that commit (mirroring the
11+
llama-cpp / ik-llama-cpp / turboquant pattern). The bump-deps bot
12+
(`.github/workflows/bump_deps.yaml`) finds this pin via grep and opens a
13+
daily PR to update it. To bump manually: edit the `DS4_VERSION?=` line,
14+
then `make purge && make` (or rely on CI's clean build).
1115

1216
## Wire shape
1317

.github/workflows/bump_deps.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ jobs:
2222
variable: "TURBOQUANT_VERSION"
2323
branch: "feature/turboquant-kv-cache"
2424
file: "backend/cpp/turboquant/Makefile"
25+
- repository: "antirez/ds4"
26+
variable: "DS4_VERSION"
27+
branch: "main"
28+
file: "backend/cpp/ds4/Makefile"
2529
- repository: "ggml-org/whisper.cpp"
2630
variable: "WHISPER_CPP_VERSION"
2731
branch: "master"

backend/cpp/ds4/Makefile

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
# ds4 backend Makefile.
2-
CURDIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
3-
DS4_DIR := $(CURDIR)ds4
4-
BUILD_DIR := $(CURDIR)build
2+
#
3+
# Upstream pin lives below as DS4_VERSION?= so the bump-deps bot
4+
# (.github/bump_deps.sh) can find and update it - matches the
5+
# llama-cpp / ik-llama-cpp / turboquant convention.
6+
7+
DS4_VERSION?=ae302c2fa18cc6d9aefc021d0f27ae03c9ad2fc0
8+
DS4_REPO?=https://github.com/antirez/ds4
9+
10+
CURRENT_MAKEFILE_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
11+
BUILD_DIR := build
512

613
BUILD_TYPE ?=
714
NATIVE ?= false
@@ -27,37 +34,45 @@ ifneq ($(NATIVE),true)
2734
CMAKE_ARGS += -DDS4_NATIVE=OFF
2835
endif
2936

30-
.PHONY: prepare grpc-server package clean purge test all
37+
.PHONY: grpc-server package clean purge test all
3138
all: grpc-server
3239

33-
prepare:
34-
bash $(CURDIR)prepare.sh
40+
# Clone the upstream ds4 source at the pinned commit. Directory acts as the
41+
# target so make only re-clones when missing. After a DS4_VERSION bump,
42+
# run 'make purge && make' to refetch (or rely on CI's clean build).
43+
ds4:
44+
mkdir -p ds4
45+
cd ds4 && \
46+
git init -q && \
47+
git remote add origin $(DS4_REPO) && \
48+
git fetch --depth 1 origin $(DS4_VERSION) && \
49+
git checkout FETCH_HEAD
3550

3651
# Build ds4's engine object files via its own Makefile, which already encodes
3752
# the right per-platform compile flags (Objective-C/Metal on Darwin, nvcc on Linux+CUDA).
38-
$(DS4_DIR)/ds4.o: prepare
53+
ds4/ds4.o: ds4
3954
ifeq ($(BUILD_TYPE),cublas)
40-
+$(MAKE) -C $(DS4_DIR) ds4.o ds4_cuda.o
55+
+$(MAKE) -C ds4 ds4.o ds4_cuda.o
4156
else ifeq ($(UNAME_S),Darwin)
42-
+$(MAKE) -C $(DS4_DIR) ds4.o ds4_metal.o
57+
+$(MAKE) -C ds4 ds4.o ds4_metal.o
4358
else
44-
+$(MAKE) -C $(DS4_DIR) ds4_cpu.o
59+
+$(MAKE) -C ds4 ds4_cpu.o
4560
endif
4661

47-
grpc-server: $(DS4_DIR)/ds4.o
62+
grpc-server: ds4/ds4.o
4863
mkdir -p $(BUILD_DIR)
49-
cd $(BUILD_DIR) && cmake $(CMAKE_ARGS) -DDS4_DIR=$(DS4_DIR) $(CURDIR) && cmake --build . --config Release -j $(JOBS)
50-
cp $(BUILD_DIR)/grpc-server $(CURDIR)grpc-server
64+
cd $(BUILD_DIR) && cmake $(CMAKE_ARGS) $(CURRENT_MAKEFILE_DIR) && cmake --build . --config Release -j $(JOBS)
65+
cp $(BUILD_DIR)/grpc-server grpc-server
5166

5267
package: grpc-server
53-
bash $(CURDIR)package.sh
68+
bash package.sh
5469

5570
test:
5671
@echo "ds4 backend: e2e coverage at tests/e2e-backends/ (BACKEND_BINARY mode)"
5772

5873
clean:
59-
rm -rf $(BUILD_DIR) $(CURDIR)grpc-server $(CURDIR)package
60-
if [ -d $(DS4_DIR) ]; then $(MAKE) -C $(DS4_DIR) clean; fi
74+
rm -rf $(BUILD_DIR) grpc-server package
75+
if [ -d ds4 ]; then $(MAKE) -C ds4 clean; fi
6176

6277
purge: clean
63-
rm -rf $(DS4_DIR)
78+
rm -rf ds4

backend/cpp/ds4/prepare.sh

Lines changed: 0 additions & 24 deletions
This file was deleted.

scripts/changed-backends.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ function inferBackendPath(item) {
3232
// via a thin wrapper Makefile. Changes to either dir should retrigger it.
3333
return `backend/cpp/turboquant/`;
3434
}
35+
if (item.dockerfile.endsWith("ds4")) {
36+
return `backend/cpp/ds4/`;
37+
}
3538
if (item.dockerfile.endsWith("llama-cpp")) {
3639
return `backend/cpp/llama-cpp/`;
3740
}

0 commit comments

Comments
 (0)