diff --git a/.claude/skills/java-tdd-guide/SKILL.pointer.md b/.claude/skills/java-tdd-guide/SKILL.pointer.md new file mode 100644 index 000000000..b74e8e20f --- /dev/null +++ b/.claude/skills/java-tdd-guide/SKILL.pointer.md @@ -0,0 +1,20 @@ +--- +name: java-tdd-guide-pointer +description: Marker file — the canonical Java TDD skill lives in the sibling workspace repo at .claude/skills/java-tdd-guide/SKILL.md and must be in the Claude session scope for the harness to load it. +--- + +# Pointer — Java TDD Guide + +The canonical skill content lives in the `workspace` sibling repo: + +`../../../../workspace/.claude/skills/java-tdd-guide/SKILL.md` + +For Claude sessions opened against this repo to find the skill, the +`workspace` repo must also be in the session's repository scope. The +standard remote-execution setup adds it automatically. + +If a session reports the skill as missing, verify the session scope +includes `bernardladenthin/workspace` and retry. + +This file exists so human readers and any future drift-detection tooling +can see the dependency from this repo to the canonical skill. diff --git a/.github/buildcheck/patches.py b/.github/buildcheck/patches.py new file mode 100644 index 000000000..6d1921730 --- /dev/null +++ b/.github/buildcheck/patches.py @@ -0,0 +1,115 @@ +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 +"""The llama/patches/*.patch files, checked as TEXT before any job applies them: every hunk header +declares exactly the lines its body carries. + +Why a check of the text, when the applier is fail-loud. `git apply` (and `git apply --check`) reads +a hunk by its header counts and then looks for the next header. Lines left over after the counted +body are not an error to it: they are skipped as the start of the next (non-existent) header. So a +new-file hunk that says `@@ -0,0 +1,25 @@` over 27 `+` lines applies "cleanly" and writes the first +25 lines -- the file is truncated, silently. That is what #489 shipped (two comment lines added to +`prefetch.h` inside patch 0017 without touching the count): `prefetch.h` ended in the middle of its +one function, every translation unit including it failed, and run 37922404867 went red in 24 jobs +before #492 corrected the count. Checking that a patch applies is not checking that it applies +correctly; this module checks the half `git apply --check` cannot, in the `code-style` job, in the +first minutes of every run and with no llama.cpp source needed. + +Two shapes are reported, for every hunk of every patch: + * the body ends before the header's counts are satisfied -- a header that claims too much, or a + patch cut short; `git apply` rejects this one as corrupt, so it is caught here earlier, not only; + * lines that look like body (`+`, `-` or context) follow a satisfied hunk before the next header -- + the #489 shape, which `git apply` accepts and truncates. +The patch header text before the first `diff --git` is free prose and is not inspected. +""" + +import glob +import os +import re + +PATCH_DIR = os.path.join("llama", "patches") +HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") +# What may follow a complete hunk: the next hunk, the next file's header, or the format-patch +# signature trailer (`-- ` alone or followed by the git version). +HEADER_PREFIXES = ("diff --git ", "index ", "--- ", "+++ ", "new file mode ", "deleted file mode ", "old mode ", + "new mode ", "similarity index ", "dissimilarity index ", "rename from ", "rename to ", + "copy from ", "copy to ", "Binary files ") +TRAILER = re.compile(r"^-- (\d|$)") + + +def _is_header(line): + return bool(HUNK.match(line)) or line.startswith(HEADER_PREFIXES) or bool(TRAILER.match(line)) + + +def audit(text, name=""): + """(problems, hunks) of one patch's text: the problems as messages naming the patch, the line and + the file the hunk belongs to, and the number of hunks seen.""" + problems = [] + lines = text.split("\n") + if lines and lines[-1] == "": + lines.pop() + hunks = 0 + target = "?" + i = 0 + while i < len(lines): + line = lines[i] + if line.startswith("+++ "): + target = line[4:].strip() + target = target[2:] if target.startswith("b/") else target + match = HUNK.match(line) + if not match: + i += 1 + continue + hunks += 1 + header_line = i + 1 + old = int(match.group(2)) if match.group(2) is not None else 1 + new = int(match.group(4)) if match.group(4) is not None else 1 + o = n = 0 + i += 1 + while i < len(lines) and (o < old or n < new): + body = lines[i] + if body.startswith("\\"): # "\ No newline at end of file" belongs to the line before it + i += 1 + continue + if body.startswith("+"): + n += 1 + elif body.startswith("-"): + o += 1 + elif body.startswith(" ") or body == "": + o += 1 + n += 1 + else: + break + i += 1 + if o != old or n != new: + problems.append(f"{name}:{header_line}: the hunk for {target} declares -{old},+{new} lines but its body " + f"ends after -{o},+{n} -- a header claiming too much or a patch cut short; `git apply` " + f"rejects it as corrupt") + continue + extra = 0 + while i < len(lines) and not _is_header(lines[i]): + body = lines[i] + if body.startswith(("+", "-", " ")): + extra += 1 + elif body != "" and not body.startswith("\\"): + break + i += 1 + if extra: + problems.append(f"{name}:{header_line}: {extra} line(s) after the hunk for {target} are not covered by " + f"its header (-{old},+{new}) -- `git apply` drops them silently and writes a truncated " + f"file; recount the header") + return problems, hunks + + +def check(root, patch_dir=PATCH_DIR): + """(problems, patches, hunks) over every *.patch of the directory, in name order.""" + problems = [] + paths = sorted(glob.glob(os.path.join(root, patch_dir, "*.patch"))) + hunks = 0 + for path in paths: + with open(path, "rb") as f: + text = f.read().decode("utf-8", errors="surrogateescape") + found, count = audit(text, os.path.relpath(path, root).replace(os.sep, "/")) + problems += found + hunks += count + return problems, len(paths), hunks diff --git a/.github/buildcheck/tests/test_patches.py b/.github/buildcheck/tests/test_patches.py new file mode 100644 index 000000000..bceb6933f --- /dev/null +++ b/.github/buildcheck/tests/test_patches.py @@ -0,0 +1,98 @@ +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 + +import unittest + +from buildcheck import patches +from buildcheck.tests.helpers import REPO + + +def new_file_patch(body, declared=None): + """A git patch adding `new.h` with the given lines; `declared` overrides the hunk's +count.""" + n = len(body) if declared is None else declared + return ("some prose above the diff, as the project's patch headers carry\n" + "diff --git a/dir/new.h b/dir/new.h\n" + "new file mode 100644\n" + "index 0000000..1234567\n" + "--- /dev/null\n" + "+++ b/dir/new.h\n" + f"@@ -0,0 +1,{n} @@\n" + "".join(f"+{line}\n" for line in body)) + + +MODIFY = ("diff --git a/a.c b/a.c\n" + "index 1111111..2222222 100644\n" + "--- a/a.c\n" + "+++ b/a.c\n" + "@@ -1,4 +1,5 @@\n" + " one\n" + "-two\n" + "+two changed\n" + "+two and a half\n" + " three\n" + " four\n" + "\\ No newline at end of file\n") + + +class AuditTest(unittest.TestCase): + + def test_a_new_file_hunk_whose_header_matches_its_body_passes(self): + problems, hunks = patches.audit(new_file_patch(["a", "b", "c"])) + self.assertEqual(problems, []) + self.assertEqual(hunks, 1) + + def test_lines_past_the_declared_count_are_reported_the_489_shape(self): + """Two comment lines were added to prefetch.h without recounting: 27 lines under a +25 + header. `git apply` writes 25 and drops the rest without a word.""" + problems, _ = patches.audit(new_file_patch(["a", "b", "c", "d", "e"], declared=3), "p.patch") + self.assertEqual(len(problems), 1, problems) + self.assertIn("p.patch:7", problems[0]) + self.assertIn("2 line(s) after the hunk for dir/new.h", problems[0]) + self.assertIn("-0,+3", problems[0]) + + def test_a_header_claiming_more_than_the_body_carries_is_reported(self): + problems, _ = patches.audit(new_file_patch(["a", "b"], declared=4), "p.patch") + self.assertEqual(len(problems), 1, problems) + self.assertIn("declares -0,+4 lines but its body ends after -0,+2", problems[0]) + + def test_a_modify_hunk_with_context_removals_additions_and_the_no_newline_marker_passes(self): + self.assertEqual(patches.audit(MODIFY), ([], 1)) + + def test_an_excess_removed_or_context_line_counts_as_well(self): + for extra in ("-gone\n", " ctx\n"): + problems, _ = patches.audit(MODIFY + extra, "p.patch") + self.assertEqual(len(problems), 1, (extra, problems)) + self.assertIn("1 line(s) after the hunk for a.c", problems[0]) + + def test_the_next_file_header_and_the_format_patch_trailer_end_a_hunk_cleanly(self): + two = new_file_patch(["a"]) + new_file_patch(["b", "c"]).split("\n", 1)[1] + "-- \n2.43.0\n\n" + problems, hunks = patches.audit(two) + self.assertEqual(problems, []) + self.assertEqual(hunks, 2) + + def test_the_prose_before_the_first_diff_is_not_inspected(self): + text = "+ a plus sign in prose\n- and a dash\n" + new_file_patch(["x"]) + self.assertEqual(patches.audit(text), ([], 1)) + + +class RepositoryTest(unittest.TestCase): + + def test_every_patch_of_this_repository_is_consistent(self): + problems, count, hunks = patches.check(REPO) + self.assertEqual(problems, []) + self.assertGreaterEqual(count, 11) + self.assertGreater(hunks, count) + + def test_the_broken_0017_of_pr_489_is_caught(self): + """The patch text as merged in #489 (main 4b32dae5..bc2c8af9): header +1,25 over 27 lines.""" + import subprocess + try: + text = subprocess.run(["git", "-C", REPO, "show", + "bc2c8af9:llama/patches/0017-ggml-cpu-x86-prefetch-byte-offset-and-msvc-type.patch"], + capture_output=True, check=True).stdout.decode("utf-8", errors="surrogateescape") + except (subprocess.CalledProcessError, FileNotFoundError): + self.skipTest("the historic commit is not available in this checkout") + problems, _ = patches.audit(text, "0017.patch") + self.assertEqual(len(problems), 1, problems) + self.assertIn("2 line(s) after the hunk for ggml/src/ggml-cpu/arch/x86/prefetch.h", problems[0]) + self.assertIn("-0,+25", problems[0]) diff --git a/.github/check-patches.py b/.github/check-patches.py new file mode 100755 index 000000000..094cde7b2 --- /dev/null +++ b/.github/check-patches.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 +"""Fail when a hunk of llama/patches/*.patch does not declare exactly the lines it carries. + +`git apply --check` does not catch this: lines past a hunk's declared count are skipped, which +truncates a new file silently (see buildcheck/patches.py for the incident). Runs in the +`code-style` job; needs no llama.cpp source. + +Usage: + check-patches.py check, exit 1 on any problem +""" + +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from buildcheck import patches # noqa: E402 + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def main(argv): + if argv[1:]: + print(__doc__, file=sys.stderr) + return 2 + problems, count, hunks = patches.check(ROOT) + for p in problems: + print(f"::error::{p}", file=sys.stderr) + print(f"{count} patches, {hunks} hunks, {len(problems)} problems") + if count == 0: + print("::error::no patches found -- an empty input is a failure, never a pass", file=sys.stderr) + return 2 + return 1 if problems else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/.github/shared-files.sha256 b/.github/shared-files.sha256 index fae60f7ba..da37c4caa 100644 --- a/.github/shared-files.sha256 +++ b/.github/shared-files.sha256 @@ -37,6 +37,7 @@ ab45f5c102b47dd16c325d4d9c283d158ba90c05f484eac45b2767885c4462f9 .github/signin 0f08122e597f93dbbdc9c80e88984b4bf4738951d5902813df3d4640cdb11bac .github/ISSUE_TEMPLATE/feature_request.md ebfcc0adf59f5858bbe4dc077c906304a197f72a55256f0d5aac669bee5e871f .github/PULL_REQUEST_TEMPLATE.md f2a516d98a4287486dac82fc2c1293d81e3a4a1087e3ace934da0af5caaeace5 CODE_OF_CONDUCT.md +b191c691d333249de1780e474817446cd94262e0bfa87e27cb6e412865f90943 .claude/skills/java-tdd-guide/SKILL.pointer.md 074e6e32c86a4c0ef8b3ed25b721ca23aca83df277cd88106ef7177c354615ff LICENSES/Apache-2.0.txt b05785f9f18e6716bab63424b11454513b9943a222595b70411009202fc592b5 LICENSES/MIT.txt edc34bc287c32b4d7ff970c0dd7a25e7d724cdd2775ef2b65aaccccd49c733c8 .editorconfig diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index ccd400894..53c49d6cc 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -292,6 +292,8 @@ jobs: # .github/buildcheck/natives.py; its unit tests run in the shared-files job. - name: Natives list check (everything that names a natives jar agrees with natives.csv) run: python3 .github/check-natives.py + - name: Patch hunk check (every hunk of llama/patches/*.patch declares exactly the lines it carries) + run: python3 .github/check-patches.py - name: Spotless check (fail fast on format violations) run: mvn -B --no-transfer-progress -f llama/pom.xml spotless:check - name: SpotBugs check (fail fast on static-analysis findings) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5123f247..d9d3ccde4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,12 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by GitHub retires the `macos-14` image by 2026-11-02 and fails every `macos-14` job during its October brownouts. The label is pinned rather than `macos-latest`, and it is not `macos-15`, which the shipped Metal build and the no-Metal build already run on. +- **CI: every hunk of `llama/patches/*.patch` is checked as text** (`.github/check-patches.py`, + `code-style` job): the header must declare exactly the lines the body carries. `git apply --check` + cannot see a header that undercounts -- it skips the surplus lines and writes a truncated file -- which + is how two comment lines added to `prefetch.h` in patch `0017` cut the header off inside its function + and reddened 24 jobs before the count was corrected. The check needs no llama.cpp source and runs in + the first minutes of a run; the historic broken patch text is its pinned negative case. - **Upgraded the pinned llama.cpp from b11476 to b11512**, in 7 reviewed steps, each ending at a tag. No carried patch needed a refresh, and every drop-check still finds its defect. The chat parser now receives the generated tokens next to the text (#29876, `common_chat_input`) -- a change in the C++ diff --git a/CLAUDE.md b/CLAUDE.md index fc19406eb..cca403231 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,6 +156,7 @@ shapes this repo writes and rejects anything else loudly. |---|---|---| | `natives.py` | `check-natives.py` (`code-style`) | everything that names a natives jar agrees with `natives.csv` (see "Natives jars") | | `models.py` | `check-natives.py` | every `*_MODEL_NAME` of publish.yml's `env:` is a filename of `models.csv` | +| `patches.py` | `check-patches.py` (`code-style`) | every hunk of `llama/patches/*.patch` declares exactly the lines it carries -- the half `git apply --check` does not see (see "Local llama.cpp source patches") | | `nativedeps.py` | `verify-native-deps.py` (`package`, `package-android-aar`) | dependency allowlists, 16 KB Android alignment | | `hipoffload.py` | `verify-hip-offload-compressed.py` (the two ROCm jobs) | no uncompressed GPU code bundle | | `workflow.py` | — | the job graph of a workflow (`needs`, uploads, transitive closure) | @@ -1125,6 +1126,15 @@ The fetched llama.cpp source is patched before it compiles, via a generic mechan aborted every reconfigure of an existing build dir with a misleading "does not apply cleanly". A source tree supplied via `-DFETCHCONTENT_SOURCE_DIR_LLAMA.CPP=` that is not a git work tree has neither oracle and falls back to the old per-patch path (same caveat as before). +- **`.github/check-patches.py`** (`buildcheck/patches.py`, the `code-style` job) -- audits every hunk of + every patch **as text**: its header must declare exactly the lines its body carries. `git apply --check` + does not see this -- it reads a hunk by its counts and skips what follows as the start of the next + header -- so a new-file hunk with more `+` lines than its header says applies "cleanly" and writes a + **truncated file**. That shipped in #489: two comment lines added inside `0017`'s `prefetch.h` without + recounting, `prefetch.h` cut off inside its one function, run 37922404867 red in 24 jobs, corrected by + #492. The check costs milliseconds, needs no llama.cpp source, and runs in the first minutes of every + run; `test_patches.py` pins the exact historic patch text as its negative case. Checking that a patch + applies is not checking that it applies correctly -- after editing a patch by hand, run it. - **`llama/CMakeLists.txt`** — wired as the llama.cpp `FetchContent_Declare(... PATCH_COMMAND ...)`, so it runs for **every** C++ build (all CI jobs *and* local `cmake -B build`) from one place — no per-build-step plumbing. diff --git a/REUSE.toml b/REUSE.toml index f7cf9344a..0622cfc41 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -22,6 +22,7 @@ path = [ "llama/src/test/resources/images/README.md", "llama/src/test/resources/audios/README.md", ".claude/commands/find-cpp-duplication.md", + ".claude/skills/java-tdd-guide/SKILL.pointer.md", "llama-langchain4j/README.md", "models/README.md", ]