From f430648614b3218cd52bb4b25d4c445a966652b4 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 20 Jul 2026 18:16:43 +0000 Subject: [PATCH] fix(ci): rebuild Go backends on linked pkg/ changes and on matrix entry edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #10975 taught the backend matrix filter about shared build inputs, but left two paths that still rebuild nothing. Go backends link code from the main tree. `go list -deps ./backend/go/...` resolves to exactly six pkg subtrees (audio, grpc incl. base/grpcerrors/proto, httpclient, sound, store, utils), identical for GOOS/GOARCH in {linux,darwin} x {amd64,arm64}. Editing any of them changes the shipped binary, but they sit outside every backend directory so the prefix match never saw them. Enumerating those six rather than taking all of pkg/ is the point: all of pkg/ changes in ~8.6% of commits, these six in 2.0% — the same order as the already accepted scripts/build/ rule (1.9%). Blast radius 199/417 Linux, 26/56 Darwin; the ~21 core-server-only pkg subtrees still rebuild nothing, and neither do _test.go files. .github/backend-matrix.yml was excluded wholesale because matching its path would rebuild all 417 entries on every new-backend PR. That hid a real hole: editing an existing entry's base-image, build-type or cuda version changes the image it produces while touching no file the filter can see. Since the change is within a structured file, compare it against the base revision and rebuild only the entries whose fields actually differ — 1 entry for a base-image edit, 0 for a comment or whitespace edit, and all 417 only when the previous revision cannot be resolved. This also closes a third hole: a new matrix entry for an existing backend (a new CUDA variant, say) touches nothing under that backend's directory and previously rebuilt nothing. changed-backends.js fetches the base revision via the contents API, and only when the changed-file list actually names the matrix file, so the common path costs no extra request. Signed-off-by: Ettore Di Giacinto Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash] --- scripts/changed-backends.js | 52 ++++++- scripts/lib/backend-filter.mjs | 118 +++++++++++++++- scripts/lib/backend-filter_test.mjs | 212 +++++++++++++++++++++++++++- 3 files changed, 371 insertions(+), 11 deletions(-) diff --git a/scripts/changed-backends.js b/scripts/changed-backends.js index f9d0899f523f..1c87d0247965 100644 --- a/scripts/changed-backends.js +++ b/scripts/changed-backends.js @@ -5,13 +5,14 @@ import { Octokit } from "@octokit/core"; import { getAllBackendPaths, filterMatrix, + BACKEND_MATRIX_FILE, } from "./lib/backend-filter.mjs"; // Matrix data lives in a small data-only YAML so both backend.yml (master push) // and backend_pr.yml (pull_request) can use a dynamic `matrix: ${{ fromJson(...) }}` // for the live job, while this script remains the single source of truth for // "what backends does the project know about". -const matrixYml = yaml.load(fs.readFileSync(".github/backend-matrix.yml", "utf8")); +const matrixYml = yaml.load(fs.readFileSync(BACKEND_MATRIX_FILE, "utf8")); const includes = matrixYml.include; const includesDarwin = matrixYml.includeDarwin; @@ -76,6 +77,45 @@ async function getChangedFilesForPush(event) { return res.data.files.map(f => f.filename); } +// The matrix file's contents at the base revision, so filterMatrix can rebuild +// only the entries whose fields actually changed instead of all 417 (or, as +// before, none of them). Returns null when the previous revision cannot be +// resolved, which filterMatrix treats as "rebuild everything". +// +// Only called when the changed-file list actually names the matrix file, so the +// common path costs no extra API request. +async function getPreviousMatrix(event) { + const ref = event.pull_request ? event.pull_request.base.sha : event.before; + if (!ref || /^0+$/.test(ref)) return null; + const owner = event.repository.owner.login; + const repo = event.repository.name; + try { + const res = await octokit.request('GET /repos/{owner}/{repo}/contents/{path}', { + owner, + repo, + path: BACKEND_MATRIX_FILE, + ref, + mediaType: { format: 'raw' }, + }); + // `format: raw` yields a string; fall back to the JSON representation in + // case a proxy or a future Octokit version ignores the media type. + const raw = typeof res.data === 'string' + ? res.data + : Buffer.from(res.data.content, 'base64').toString('utf8'); + const previous = yaml.load(raw); + return { + include: previous.include || [], + includeDarwin: previous.includeDarwin || [], + }; + } catch (err) { + console.log( + `could not read ${BACKEND_MATRIX_FILE} at ${ref}, falling back to run-all:`, + err.message + ); + return null; + } +} + // Group matrix entries by tag-suffix and emit a merge-matrix entry per group. // Both multi-leg groups (per-arch fan-out) and singletons get one entry each: // the build job pushes by digest only with no tags applied, so every backend @@ -200,13 +240,14 @@ function emitFullMatrix() { } } -function emitFilteredMatrix(changedFiles) { +function emitFilteredMatrix(changedFiles, previousMatrix) { console.log("Changed files:", changedFiles); const { filtered, filteredDarwin, changedBackends } = filterMatrix({ includes, includesDarwin, changedFiles, + previousMatrix, }); console.log("Filtered files:", filtered); @@ -260,5 +301,10 @@ function emitFilteredMatrix(changedFiles) { emitFullMatrix(); return; } - emitFilteredMatrix(changedFiles); + + const previousMatrix = changedFiles.includes(BACKEND_MATRIX_FILE) + ? await getPreviousMatrix(event) + : null; + + emitFilteredMatrix(changedFiles, previousMatrix); })(); diff --git a/scripts/lib/backend-filter.mjs b/scripts/lib/backend-filter.mjs index 994329c83ce8..1054b0d2fd89 100644 --- a/scripts/lib/backend-filter.mjs +++ b/scripts/lib/backend-filter.mjs @@ -137,9 +137,39 @@ const isDarwinGenericGo = item => const isLinuxPython = item => item.dockerfile.endsWith("python"); +// Dockerfile.golang is the only Linux dockerfile that compiles Go (it runs +// `make protogen-go && make -C backend/go/ build`); every other one +// builds C++, Rust or a Python venv and links no Go code at all. +const isLinuxGo = item => item.dockerfile.endsWith("golang"); + const never = () => false; const always = () => true; +// The pkg/ subtrees that end up inside a Go backend binary. Not a guess: +// +// go list -deps ./backend/go/... | grep LocalAI/pkg +// +// returns exactly pkg/audio, pkg/grpc (+ base, grpcerrors, proto), pkg/httpclient, +// pkg/sound, pkg/store and pkg/utils — identical for GOOS/GOARCH in +// {linux,darwin} x {amd64,arm64}, so one list covers both matrices. Notably +// absent: pkg/model, pkg/downloader, pkg/functions and the other ~21 pkg +// subtrees, which are core-server-only. +// +// Enumerating rather than taking all of pkg/ is the whole point. All of pkg/ +// changes in ~8.6% of commits and would fire a 199-entry Go matrix that often; +// these six subtrees change in 2.0%, which is the same order as the already +// accepted scripts/build/ rule (1.9%). If the enumeration ever drifts from the +// `go list` output the tests pin both directions — a listed subtree must +// trigger, an unlisted one must not. +const GO_BACKEND_PKG_PREFIXES = [ + "pkg/audio/", + "pkg/grpc/", // covers base/, grpcerrors/ and the generated proto/ + "pkg/httpclient/", + "pkg/sound/", + "pkg/store/", + "pkg/utils/", +]; + // Shared build inputs: files that end up in, or decide the contents of, images // belonging to backends whose own directory they do not live under. The // per-backend prefix match in filterMatrix() structurally cannot see these, so @@ -173,6 +203,21 @@ export const SHARED_BUILD_INPUTS = [ linux: isLinuxPython, darwin: isDarwinPython, }, + { + // Compiled into every Go backend binary (see GO_BACKEND_PKG_PREFIXES). + // `_test.go` files never reach a binary, same carve-out as the + // `*_test.sh` one on the scripts/build/ catch-all below. + // + // Darwin's bespoke builders (llama-cpp, ds4, privacy-filter) carry + // lang=go for runner selection only — their sources are C++ and link no + // Go, so isDarwinGenericGo is the correct predicate here, exactly as it + // is for scripts/build/golang-darwin.sh. + matches: file => + GO_BACKEND_PKG_PREFIXES.some(prefix => file.startsWith(prefix)) && + !file.endsWith("_test.go"), + linux: isLinuxGo, + darwin: isDarwinGenericGo, + }, { // The reusable build workflows own build-args, packaging and push for // their whole OS family, so a change there can alter what lands in every @@ -247,6 +292,51 @@ function dockerfileChanged(item, changedFiles) { return df !== "" && changedFiles.includes(df); } +// .github/backend-matrix.yml is a shared build input like the ones above, but +// it cannot be handled by a path rule: every entry lives in it, so matching the +// path would rebuild all 417 Linux entries on every PR that adds a backend — +// and new backends already trigger via their own directory. Excluding it +// wholesale is what the rest of this file does, and that leaves a real hole: +// editing an existing entry's base-image / build-type / cuda version changes +// the image that entry produces while touching no file the prefix match can +// see, so it rebuilt nothing. +// +// The fix needs the file's *previous* contents, which a changed-path list +// cannot carry. changed-backends.js fetches the base revision via the GitHub +// contents API and hands it in as `previousMatrix`; everything below stays +// pure. When the file did not change, `previousMatrix` is never consulted. +export const BACKEND_MATRIX_FILE = ".github/backend-matrix.yml"; + +// Identity of a matrix entry across revisions. tag-suffix names the image; +// per-arch legs of the same image are distinguished by platform-tag. Verified +// unique across all 417 Linux and 56 Darwin entries. +export function matrixEntryKey(item) { + return JSON.stringify([item["tag-suffix"] || "", item["platform-tag"] || ""]); +} + +// Full field set, order-independent, so a reordered or reindented YAML entry +// compares equal and only a real value change counts. +function entryFingerprint(item) { + return JSON.stringify( + Object.keys(item).sort().map(key => [key, item[key]]) + ); +} + +// Keys of entries that are new or whose fields differ from the previous +// revision. Removed entries produce nothing to build. +function changedEntryKeys(current, previous) { + const before = new Map(); + for (const item of previous) { + before.set(matrixEntryKey(item), entryFingerprint(item)); + } + const keys = new Set(); + for (const item of current) { + const key = matrixEntryKey(item); + if (before.get(key) !== entryFingerprint(item)) keys.add(key); + } + return keys; +} + function matchedSharedRules(changedFiles) { const rules = []; for (const file of changedFiles) { @@ -259,20 +349,46 @@ function matchedSharedRules(changedFiles) { // Filter both matrices against a changed-file list. Returns the surviving // entries plus the set of backend names considered changed, which drives the // per-backend boolean outputs consumed by test-extra.yml. -export function filterMatrix({ includes, includesDarwin, changedFiles }) { +export function filterMatrix({ + includes, + includesDarwin, + changedFiles, + previousMatrix, +}) { const sharedRules = matchedSharedRules(changedFiles); + const matrixFileChanged = changedFiles.includes(BACKEND_MATRIX_FILE); + // The matrix file changed but we could not resolve what it used to say (API + // failure, shallow history, first push). Claiming "no entry changed" would + // reintroduce exactly the silent-empty-matrix failure this file exists to + // prevent, so fall back to rebuilding everything — the same posture + // changed-backends.js takes for a truncated or unavailable diff. + const matrixDiffUnavailable = matrixFileChanged && !previousMatrix; + + const changedLinuxKeys = + matrixFileChanged && previousMatrix + ? changedEntryKeys(includes, previousMatrix.include || []) + : null; + const changedDarwinKeys = + matrixFileChanged && previousMatrix + ? changedEntryKeys(includesDarwin, previousMatrix.includeDarwin || []) + : null; + const filtered = includes.filter(item => { const backendPath = inferBackendPath(item); if (!backendPath) return false; if (backendChanged(item.backend, backendPath, changedFiles)) return true; if (dockerfileChanged(item, changedFiles)) return true; + if (matrixDiffUnavailable) return true; + if (changedLinuxKeys && changedLinuxKeys.has(matrixEntryKey(item))) return true; return sharedRules.some(rule => rule.linux(item)); }); const filteredDarwin = includesDarwin.filter(item => { const backendPath = inferBackendPathDarwin(item); if (changedFiles.some(file => file.startsWith(backendPath))) return true; + if (matrixDiffUnavailable) return true; + if (changedDarwinKeys && changedDarwinKeys.has(matrixEntryKey(item))) return true; return sharedRules.some(rule => rule.darwin(item)); }); diff --git a/scripts/lib/backend-filter_test.mjs b/scripts/lib/backend-filter_test.mjs index a2b582da93fa..0d99e1e108c0 100644 --- a/scripts/lib/backend-filter_test.mjs +++ b/scripts/lib/backend-filter_test.mjs @@ -19,44 +19,50 @@ const includes = [ backend: "diffusers", dockerfile: "./backend/Dockerfile.python", "tag-suffix": "-nvidia-cuda-13-diffusers", + "base-image": "nvidia/cuda:13.0.0-devel-ubuntu24.04", }, { backend: "vllm", dockerfile: "./backend/Dockerfile.python", "tag-suffix": "-nvidia-cuda-13-vllm", + "base-image": "nvidia/cuda:13.0.0-devel-ubuntu24.04", }, { backend: "ced", dockerfile: "./backend/Dockerfile.golang", "tag-suffix": "-ced", + "base-image": "ubuntu:24.04", }, { backend: "kokoros", dockerfile: "./backend/Dockerfile.rust", "tag-suffix": "-kokoros", + "base-image": "ubuntu:24.04", }, { backend: "llama-cpp", dockerfile: "./backend/Dockerfile.llama-cpp", "tag-suffix": "-llama-cpp", + "base-image": "ubuntu:24.04", }, { backend: "turboquant", dockerfile: "./backend/Dockerfile.turboquant", "tag-suffix": "-turboquant", + "base-image": "ubuntu:24.04", }, ]; const includesDarwin = [ - { backend: "diffusers", "tag-suffix": "-metal-darwin-arm64-diffusers" }, - { backend: "mlx", "tag-suffix": "-metal-darwin-arm64-mlx" }, - { backend: "whisper", lang: "go", "tag-suffix": "-metal-darwin-arm64-whisper" }, - { backend: "llama-cpp", lang: "go", "tag-suffix": "-metal-darwin-arm64-llama-cpp" }, - { backend: "ds4", lang: "go", "tag-suffix": "-metal-darwin-arm64-ds4" }, + { backend: "diffusers", "tag-suffix": "-metal-darwin-arm64-diffusers", "build-type": "mps" }, + { backend: "mlx", "tag-suffix": "-metal-darwin-arm64-mlx", "build-type": "mps" }, + { backend: "whisper", lang: "go", "tag-suffix": "-metal-darwin-arm64-whisper", "build-type": "metal" }, + { backend: "llama-cpp", lang: "go", "tag-suffix": "-metal-darwin-arm64-llama-cpp", "build-type": "metal" }, + { backend: "ds4", lang: "go", "tag-suffix": "-metal-darwin-arm64-ds4", "build-type": "metal" }, ]; -const run = changedFiles => - filterMatrix({ includes, includesDarwin, changedFiles }); +const run = (changedFiles, previousMatrix) => + filterMatrix({ includes, includesDarwin, changedFiles, previousMatrix }); const names = entries => entries.map(e => e.backend).sort(); @@ -187,3 +193,195 @@ test("turboquant still retriggers on llama-cpp source changes", () => { assert.deepEqual(names(filtered), ["llama-cpp", "turboquant"]); }); + +// --------------------------------------------------------------------------- +// pkg/ subtrees linked into every Go backend binary +// --------------------------------------------------------------------------- + +test("a pkg/ subtree Go backends link rebuilds Go images on both OSes", () => { + // `go list -deps ./backend/go/...` puts pkg/grpc in every Go backend binary; + // editing it changes the shipped binary but lives outside every backend + // directory, so the prefix match never saw it. + const { filtered, filteredDarwin, changedBackends } = run([ + "pkg/grpc/server.go", + ]); + + assert.notEqual(filtered.length, 0, "expected a non-empty Linux matrix"); + assert.deepEqual(names(filtered), ["ced"]); + // Darwin's bespoke builders (llama-cpp, ds4) are C++ and link no Go. + assert.deepEqual(names(filteredDarwin), ["whisper"]); + assert.ok(changedBackends.has("ced")); +}); + +test("pkg/grpc subpackages rebuild Go images too", () => { + const { filtered, filteredDarwin } = run(["pkg/grpc/base/base.go"]); + + assert.deepEqual(names(filtered), ["ced"]); + assert.deepEqual(names(filteredDarwin), ["whisper"]); +}); + +test("every enumerated Go-backend pkg subtree triggers a Go rebuild", () => { + // Pins the enumeration itself: if a subtree is dropped from the rule but the + // Go backends still import it, this fails rather than silently shipping to + // nothing (the #10946 failure mode). + for (const file of [ + "pkg/audio/convert.go", + "pkg/grpc/client.go", + "pkg/httpclient/client.go", + "pkg/sound/sound.go", + "pkg/store/client.go", + "pkg/utils/path.go", + ]) { + const { filtered } = run([file]); + assert.deepEqual(names(filtered), ["ced"], `expected ${file} to rebuild Go images`); + } +}); + +test("a pkg/ subtree no Go backend imports rebuilds nothing", () => { + // The negative pin that keeps this rule honest: including all of pkg/ would + // fire a 199-entry Go matrix on ~9% of commits. pkg/model and friends are + // core-server-only — no backend/go/ binary links them. + for (const file of [ + "pkg/model/loader.go", + "pkg/xio/reader.go", + "pkg/downloader/download.go", + "pkg/functions/parse.go", + "pkg/vram/vram.go", + ]) { + const { filtered, filteredDarwin, changedBackends } = run([file]); + assert.deepEqual(filtered, [], `${file} must not rebuild Linux images`); + assert.deepEqual(filteredDarwin, [], `${file} must not rebuild Darwin images`); + assert.equal(changedBackends.size, 0, `${file} must not mark any backend changed`); + } +}); + +test("tests for the linked pkg subtrees do not rebuild anything", () => { + // _test.go files never reach a backend binary, same reasoning as the + // scripts/build/*_test.sh exclusion above. + const { filtered, filteredDarwin } = run(["pkg/grpc/client_busy_test.go"]); + + assert.deepEqual(filtered, []); + assert.deepEqual(filteredDarwin, []); +}); + +// --------------------------------------------------------------------------- +// Diff-aware .github/backend-matrix.yml handling +// --------------------------------------------------------------------------- + +// A matrix file is only ever compared against a previous revision of itself, +// so build the "before" side by mutating a copy of the fixtures. +const previousOf = (linuxPatch = e => e, darwinPatch = e => e) => ({ + include: includes.map(e => linuxPatch({ ...e })), + includeDarwin: includesDarwin.map(e => darwinPatch({ ...e })), +}); + +test("editing an existing entry's base-image rebuilds exactly that entry", () => { + // Before this, backend-matrix.yml was excluded wholesale: retagging vllm onto + // a different CUDA base image produced an empty matrix and shipped nothing. + const previousMatrix = previousOf(e => + e.backend === "vllm" ? { ...e, "base-image": "nvidia/cuda:12.8.0-devel-ubuntu24.04" } : e + ); + + const { filtered, filteredDarwin, changedBackends } = run( + [".github/backend-matrix.yml"], + previousMatrix + ); + + assert.notEqual(filtered.length, 0, "expected a non-empty Linux matrix"); + assert.deepEqual(names(filtered), ["vllm"]); + assert.deepEqual(filteredDarwin, []); + assert.deepEqual([...changedBackends], ["vllm"]); +}); + +test("editing a non-base-image field of an entry rebuilds it too", () => { + const previousMatrix = previousOf(e => + e.backend === "ced" ? { ...e, "build-type": "cublas" } : e + ); + + const { filtered } = run([".github/backend-matrix.yml"], previousMatrix); + + assert.deepEqual(names(filtered), ["ced"]); +}); + +test("a brand-new matrix entry for an existing backend rebuilds only itself", () => { + // Adding a CUDA variant of an already-present backend touches no file under + // that backend's directory, so its own-directory trigger never fires either. + const previousMatrix = { + include: includes.filter(e => e.backend !== "kokoros"), + includeDarwin: includesDarwin, + }; + + const { filtered, filteredDarwin } = run( + [".github/backend-matrix.yml"], + previousMatrix + ); + + assert.deepEqual(names(filtered), ["kokoros"]); + assert.deepEqual(filteredDarwin, []); +}); + +test("editing a Darwin entry rebuilds only that Darwin entry", () => { + const previousMatrix = previousOf( + e => e, + e => (e.backend === "mlx" ? { ...e, "build-type": "cpu" } : e) + ); + + const { filtered, filteredDarwin } = run( + [".github/backend-matrix.yml"], + previousMatrix + ); + + assert.deepEqual(filtered, []); + assert.deepEqual(names(filteredDarwin), ["mlx"]); +}); + +test("a backend-matrix.yml edit that changes no entry rebuilds nothing", () => { + // The negative pin that makes the diff-aware rule worth having: comment and + // whitespace edits (and the reordering a formatter does) must stay free. A + // wholesale include of this file would rebuild all 417 entries here. + const { filtered, filteredDarwin, changedBackends } = run( + [".github/backend-matrix.yml"], + previousOf() + ); + + assert.deepEqual(filtered, []); + assert.deepEqual(filteredDarwin, []); + assert.equal(changedBackends.size, 0); +}); + +test("removing an entry rebuilds nothing", () => { + const previousMatrix = { + include: [...includes, { + backend: "vllm", + dockerfile: "./backend/Dockerfile.python", + "tag-suffix": "-nvidia-cuda-11-vllm", + "base-image": "nvidia/cuda:11.8.0-devel-ubuntu22.04", + }], + includeDarwin: includesDarwin, + }; + + const { filtered } = run([".github/backend-matrix.yml"], previousMatrix); + + assert.deepEqual(filtered, []); +}); + +test("a previous matrix is ignored when backend-matrix.yml did not change", () => { + // Guards against the diff leaking into unrelated runs: the previous revision + // is only ever consulted for a changed-file list that names the matrix file. + const { filtered, filteredDarwin } = run( + ["README.md"], + { include: [], includeDarwin: [] } + ); + + assert.deepEqual(filtered, []); + assert.deepEqual(filteredDarwin, []); +}); + +test("an unavailable previous matrix conservatively rebuilds everything", () => { + // Same posture as changed-backends.js's run-all fallbacks: if we cannot + // resolve what the entries used to be, we must not claim nothing changed. + const { filtered, filteredDarwin } = run([".github/backend-matrix.yml"], null); + + assert.equal(filtered.length, includes.length); + assert.equal(filteredDarwin.length, includesDarwin.length); +});