diff --git a/.github/buildcheck/tests/test_verify_classifier_fatjars.py b/.github/buildcheck/tests/test_verify_classifier_fatjars.py new file mode 100644 index 0000000..2aad20b --- /dev/null +++ b/.github/buildcheck/tests/test_verify_classifier_fatjars.py @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 +"""Drives .github/verify-classifier-fatjars.sh against fixture jars built here. + +srcmorph's own test, not a shared file: the script is srcmorph's alone. Every case of a check the +script makes itself asserts the exit code AND the ::error:: line it promises, so a silent exit cannot +pass for a diagnosed one; the corrupt-jar case asserts unzip's own exit code and that the script does +not pass the file off as a jar without natives. +""" + +import os +import shutil +import subprocess +import tempfile +import unittest +import zipfile + +from buildcheck.tests.helpers import REPO + +SCRIPT = os.path.join(REPO, ".github", "verify-classifier-fatjars.sh") +BASH = shutil.which("bash") +UNZIP = shutil.which("unzip") + +PREFIX = "net/ladenthin/llama/" +LINUX_CPU = PREFIX + "Linux/x86_64/cpu/libjllama.so" +WINDOWS_CPU = PREFIX + "Windows/x86_64/cpu/jllama.dll" +LINUX_CUDA = PREFIX + "Linux/x86_64/cuda13/libggml-cuda.so" +NON_NATIVE = "net/ladenthin/srcmorph/cli/Main.class" +DEFAULT = "srcmorph-cli-1.3.0-SNAPSHOT-jar-with-dependencies.jar" + + +def classifier_jar(classifier): + return f"srcmorph-cli-1.3.0-SNAPSHOT-jar-with-dependencies-{classifier}.jar" + + +@unittest.skipUnless(BASH and UNZIP, "needs bash and unzip") +class VerifyClassifierFatjarsTest(unittest.TestCase): + + def setUp(self): + self.dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.dir) + + def jar(self, name, entries): + with zipfile.ZipFile(os.path.join(self.dir, name), "w") as z: + z.writestr("META-INF/MANIFEST.MF", "Manifest-Version: 1.0\n") + for entry in entries: + z.writestr(entry, b"x") + + def run_script(self, *classifiers): + return subprocess.run([BASH, SCRIPT, self.dir, *classifiers], capture_output=True, text=True) + + def assert_fails_with(self, result, message): + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("::error::", result.stderr) + self.assertIn(message, result.stderr) + + def test_a_classifier_jar_with_its_backend_and_the_cpu_natives_passes(self): + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU, NON_NATIVE]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [LINUX_CPU, WINDOWS_CPU, LINUX_CUDA, NON_NATIVE]) + result = self.run_script("cuda13-linux-x86-64") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("matches Linux/x86_64/cuda13/", result.stdout) + self.assertEqual(result.stderr, "") + + def test_a_classifier_jar_holding_only_the_cpu_natives_fails(self): + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [LINUX_CPU, WINDOWS_CPU]) + self.assert_fails_with(self.run_script("cuda13-linux-x86-64"), "does not contain what its name promises") + + def test_a_classifier_jar_that_lost_a_default_native_fails(self): + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [LINUX_CPU, LINUX_CUDA]) + self.assert_fails_with(self.run_script("cuda13-linux-x86-64"), "lost natives of the default jar") + + def test_an_unmapped_classifier_shape_fails(self): + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU]) + self.jar(classifier_jar("cuda13-macos-aarch64"), [LINUX_CPU, WINDOWS_CPU, LINUX_CUDA]) + self.assert_fails_with(self.run_script("cuda13-macos-aarch64"), "has no expected native path") + + def test_a_classifier_jar_without_any_native_fails_naming_it(self): + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [NON_NATIVE]) + self.assert_fails_with(self.run_script("cuda13-linux-x86-64"), + classifier_jar("cuda13-linux-x86-64") + " carries no native library at all") + + def test_a_default_jar_without_any_native_fails_naming_it(self): + self.jar(DEFAULT, [NON_NATIVE]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [LINUX_CPU, WINDOWS_CPU, LINUX_CUDA]) + self.assert_fails_with(self.run_script("cuda13-linux-x86-64"), DEFAULT + " carries no native library at all") + + def test_a_default_jar_with_natives_of_one_os_fails(self): + self.jar(DEFAULT, [LINUX_CPU]) + self.jar(classifier_jar("cuda13-linux-x86-64"), [LINUX_CPU, LINUX_CUDA]) + self.assert_fails_with(self.run_script("cuda13-linux-x86-64"), "carries natives for only 1 OS") + + def test_a_corrupt_jar_fails_with_unzip_s_own_error_not_as_a_jar_without_natives(self): + """A jar unzip cannot read must end the script with unzip's own failure, not read as a jar + without natives: were the helper's failure masked (a `|| true` on its call, or a lost + pipefail), the script would report "carries no native library at all" with exit code 1 and + an ::error:: line, a wrong diagnosis. So the case pins unzip's exit code (9, + "End-of-central-directory signature not found") and the absence of the script's diagnosis, + not merely a non-zero exit. (A `|| true` on the grep branch alone is not observable this way: + pipefail carries unzip's status whatever that branch does; the branch only decides whether + grep's own error, exit 2, is tolerated, and nothing a fixture can provoke makes grep fail.)""" + self.jar(DEFAULT, [LINUX_CPU, WINDOWS_CPU]) + with open(os.path.join(self.dir, classifier_jar("cuda13-linux-x86-64")), "wb") as f: + f.write(b"not a zip") + result = self.run_script("cuda13-linux-x86-64") + self.assertEqual(result.returncode, 9, result.stdout + result.stderr) + self.assertIn("End-of-central-directory signature not found", result.stderr) + self.assertNotIn("::error::", result.stderr) + self.assertNotIn("carries no native library", result.stderr) + self.assertNotIn("ok: ", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/verify-classifier-fatjars.sh b/.github/verify-classifier-fatjars.sh index 63f6e06..08efa17 100755 --- a/.github/verify-classifier-fatjars.sh +++ b/.github/verify-classifier-fatjars.sh @@ -20,7 +20,7 @@ # 2. every jar carries at least one native library (a jar with none is a broken assembly that # would fail at runtime on every platform); # 3. every classifier jar carries the backend directory its NAME promises: net.ladenthin:llama -# natives jars (5.2.0 on) are named -- and hold /// -- +# natives jars (6.0.0 on) are named -- and hold /// -- # for a GPU jar one ggml backend module (libggml-.so / ggml-.dll), which the # loader puts next to the CPU library of llama-platform's jar for that platform. # This is the one that catches the failure the whole loop is exposed to. If @@ -53,9 +53,12 @@ fail() { [ -d "$ASSET_DIR" ] || fail "asset directory '$ASSET_DIR' does not exist" [ "${#CLASSIFIERS[@]}" -gt 0 ] || fail "no classifiers given -- pass the same list the build loop used" -# Sorted list of native-library entries in a jar, one per line. +# Sorted list of native-library entries in a jar, one per line; empty for a jar without any. +# grep's "no match" (exit 1) is tolerated so the explicit checks below name the jar -- under pipefail +# it used to end the script on the assignment, silently. An unzip failure or a grep error (exit 2) +# still fails the pipeline. natives_of() { - unzip -Z1 "$1" | grep -E "^${NATIVE_PREFIX}.*\.(so|dll|dylib)$" | sort + unzip -Z1 "$1" | { grep -E "^${NATIVE_PREFIX}.*\.(so|dll|dylib)$" || [ "$?" -eq 1 ]; } | sort } # "///" directory a classifier's name promises (--). The diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f29948..596fc73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,6 +196,21 @@ The release procedure (prompt template and step-by-step instructions) lives in [ a configured value. ### Fixed +- **CI: a fat jar without any native library is named in the release check again.** + `.github/verify-classifier-fatjars.sh` runs under `set -euo pipefail`, and its `natives_of` helper + piped `unzip -Z1` through `grep`: for a jar with no native entry `grep` exits 1, the assignment of + the helper's output failed and the script ended right there -- red, but with no `::error::` line and + no jar name, so its two "carries no native library at all" checks (default jar and classifier jar) + could never run. The helper now tolerates only grep's "no match"; an `unzip` failure or a grep error + still fails it. The script had no test at all: the new srcmorph-local + `.github/buildcheck/tests/test_verify_classifier_fatjars.py` builds fixture jars and asserts exit code + and `::error::` message for every check the script makes itself (a matching classifier jar passes; + CPU-only under a GPU name, a lost default native, an unmapped classifier shape, a jar without natives + -- classifier and default -- and a single-OS default jar fail), and for a corrupt jar unzip's own exit + code 9 without the script's diagnosis (a helper whose failure were masked, `|| true` on its call or a + lost pipefail, would report the file as a jar without natives and is caught; a `|| true` on the grep + branch alone is not observable through a corrupt jar, pipefail carries unzip's status either way). + The two no-natives cases were red before the fix. - **CI: the GitHub `snapshot` pre-release holds exactly the current build, and the release assets include the pom.** `gh release upload --clobber` replaces same-named assets and never removes any other, so the `snapshot` release accumulated: all 16 of its assets were stale -- the 0.1.0-SNAPSHOT and diff --git a/TODO.md b/TODO.md index 8212f16..bca8c67 100644 --- a/TODO.md +++ b/TODO.md @@ -10,28 +10,20 @@ everything below is genuinely still open. ## Open -- **`main` does not build: `net.ladenthin:llama` 5.2.0 was never released.** `srcmorph/pom.xml` - pins `5.2.0` (commit `5b4abeb`, 2026-09-01, *"pin - net.ladenthin:llama to the 5.2.0 release, not the snapshot"*), but that release does not exist: - Central's newest is `5.1.0` and `llama-5.2.0.pom` returns HTTP 404. This reactor declares no - repository besides Central, so nothing can resolve it. **Every PR run since 2026-09-01 is red** - at the first Maven step — run `33962068246`, job `101297417993`: `Could not find artifact - net.ladenthin:llama:jar:5.2.0 in central`, BUILD FAILURE after 3.35 s — while the matching `main` - runs were all `cancelled` by the start gate, so no red `main` was ever visible and the cause went - unattributed for over a week. It also passes locally on any machine where an earlier `mvn install` - of java-llama.cpp left 5.2.0 in `~/.m2`, which is why "verified locally: reactor mvn clean verify - green" appears in commit messages from that window; reproduce the real state with - `mvn -Dmaven.repo.local=/tmp/empty dependency:get -Dartifact=net.ladenthin:llama:5.2.0`. - **Reverting to `5.1.0` is not obviously correct** — `229903c` had raised the pin to - `5.2.0-SNAPSHOT` precisely because the newer binding "can express" flashAttn, so 5.1.0 may not - carry the API `LlamaCppJniConfigFactory` uses. java-llama.cpp's `main` sits at `5.2.0-SNAPSHOT`, - so publishing that release is the likelier fix. Decide which, then re-run CI to confirm — this is - the one item here that blocks everything else in the repo. Reverting got harder since: the reactor - now depends on 5.2.0's natives-jar split (`net.ladenthin:llama-platform`, GPU natives jars added via - `-Dllama.classifier`), which 5.1.0 does not have. Against a locally installed 5.2.0-SNAPSHOT - (`-Dllama.version=5.2.0-SNAPSHOT install`) the whole reactor is green, spotbugs included - (658 / 39 / 32 tests, the 36 model-backed knob-sweep cases among them), so the release is the only - thing missing. +- **`main` does not build until `net.ladenthin:llama` 6.0.0 is released.** `srcmorph/pom.xml` pins + `6.0.0` (`b710796`; jllama renumbered its next release from 5.2.0 to + 6.0.0), and Central's newest is still `5.1.0` + (`llama-6.0.0.pom` is HTTP 404, checked 2026-10-20). This reactor declares no repository besides + Central, so every CI run is red at the first Maven step until that release exists. It passes locally + wherever an earlier `mvn install` of java-llama.cpp left the version in `~/.m2`; reproduce the real + state with `mvn -Dmaven.repo.local=/tmp/empty dependency:get -Dartifact=net.ladenthin:llama:6.0.0`. + **Reverting is not an option**: the pom comment explains why 6.0.0 is a breaking bump this module + needs (the binding removed `enableFlashAttn()`/`FLASH_ATTN` and renamed `setTensorReadLazy` to + `setLazyMode`; the provider uses the new forms, so it cannot compile against 5.1.0), and the reactor additionally depends on 6.0.0's + natives-jar split (`net.ladenthin:llama-platform`, GPU natives jars via `-Dllama.classifier`). Against a locally installed snapshot of it + (`-Dllama.version=-SNAPSHOT install`, measured under the old 5.2.0 number) the whole reactor was + green, so the jllama release is the + only thing missing -- this item closes with it, nothing to change here. - **The GPU classifier fat jars are verified structurally, never launched.** Since 1.2.0 `.github/verify-classifier-fatjars.sh` asserts each is the artifact its name claims (one jar per @@ -53,11 +45,11 @@ everything below is genuinely still open. the publish jobs' loop), while the `srcmorph-cli` jar that goes to Central is never launched by any job -- a smoke over the thin jar plus its resolved runtime classpath (what java-llama.cpp's smoke sets are) would test what is published. Decide before the next release that bumps to a published - llama 5.2.0; the classifier list is current for 5.2.0 either way. + llama 6.0.0; the classifier list is current for 6.0.0 either way. - **jqwik pin policy** — see [`../workspace/policies/jqwik-prompt-injection.md`](../workspace/policies/jqwik-prompt-injection.md). `jqwik.version ≤ 1.9.3` is mandatory (declared in `srcmorph/pom.xml`, the only reactor module with a jqwik test dependency). -- **`@VisibleForTesting` audit.** Nothing is annotated, but the members exist: `provider.LlamaCppJniAiGenerationProvider` has four (`buildChatTemplateKwargs`, `buildInferenceParameters`, `warnOnTruncatedAnswer`, `logPromptCacheReuse`) plus the static `tensorReadLazyMode`/`cacheType`, `document.AiMdDocumentCodec` has `read(List)`/`write`, `prompt.AiPromptPreparationSupport` has `trimSourceAtLineBreak`, and the three mojos have their `build*Configuration()`. Guava is not a dependency, so closing this means either a project-local marker annotation (there is precedent: `support.ConvertToRecord`) or recording that the convention is not adopted here. Decide and act rather than re-auditing. +- **`@VisibleForTesting` audit.** Nothing is annotated, and the inventory has two kinds of package-private members that must not be lumped together. **Production API, not candidates:** the four of `provider.LlamaCppJniProviderSupport` (`buildChatTemplateKwargs`, `buildInferenceParameters`, `warnOnTruncatedAnswer`, `logPromptCacheReuse`) and the static `lazyMode`/`cacheType` are package-private because `LlamaCppJniAiGenerationProvider` calls them (they were moved out of it so the PIT gate reaches them); their visibility serves the provider, the tests only ride along. **Test seams, the actual candidates:** `document.AiMdDocumentCodec.read(List)` and the `String write(AiMdDocument)` overload (their Javadoc says so; production goes through the `Path` overloads), `prompt.AiPromptPreparationSupport.trimSourceAtLineBreak` (called only inside its own class and by `AiPromptPreparationSupportTest`), and the three mojos' `build*Configuration()` (called only by their own `execute()` and by `MojoConfigurationMappingTest`). Guava is not a dependency, so closing this means either a project-local marker annotation on the second group (there is precedent: `support.ConvertToRecord`) or recording that the convention is not adopted here. Decide and act rather than re-auditing. - **Null-safety refinement.** JSpecify + NullAway are enforced at compile time in strict JSpecify mode in every module (see each module's own `pom.xml`); `@NullMarked` on the package; framework-populated POJOs carry class-level `@SuppressWarnings({"NullAway.Init","initialization.fields.uninitialized"})`. Open follow-up: review remaining unannotated public API surfaces for places where `@Nullable` would be more precise than the implicit non-null default. diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java index fcf6de0..bf121cb 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java @@ -300,7 +300,7 @@ public AiGenerationConfig() { * {@code --flash-attn} takes a mandatory {@code [on|off|auto]}, and the binding used to offer only a * bare-flag setter, which emitted the key alone and made llama.cpp's parser consume the next argv * token — killing the load while naming a flag the user never set. {@code net.ladenthin:llama} - * 5.2.0 added {@code ModelParameters.setFlashAttn(FlashAttn)}, which is what the provider uses.

+ * 6.0.0 added {@code ModelParameters.setFlashAttn(FlashAttn)}, which is what the provider uses.

*/ public static final boolean DEFAULT_FLASH_ATTN = false;