Skip to content

Commit 5664957

Browse files
committed
CI fixes: SpotBugs suppressions, spirv-headers, clang-cl for Windows arm64
Fixes three PR #293 CI failures. 1. SpotBugs (spotbugs-exclude.xml): the branch's new server classes tripped 14 fb-contrib/findsecbugs findings not yet covered by the exclude file. All are false-positives or intentional tradeoffs mirroring suppressions the project already applies elsewhere. Added six method-scoped Match blocks with rationale: - NativeServer: IMC_IMMATURE_CLASS_NO_EQUALS, MDM_THREAD_YIELD (main() poll), UVA_USE_VAR_ARGS (native JNI method), WEM_WEAK_EXCEPTION_MESSAGING. - ServerLauncher.main: THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION. - OpenAiServerCli parse/usage/parseChatTemplateKwargs: ENMI_NULL_ENUM_VALUE (@nullable CacheType unset sentinel), POTENTIAL_XML_INJECTION + PRMC (plain- text usage() help, no XML), EXS_EXCEPTION_SOFTENING (Jackson->CLI error, cause chained), PSC_PRESIZE_COLLECTIONS. - OpenAiServerCli$Options.getChatTemplateKwargs: EI_EXPOSE_REP (returns an already-unmodifiable map). Verified: mvn spotbugs:check -> BugInstance size 0, BUILD SUCCESS. 2. Linux Vulkan (publish.yml): both build-linux-*-vulkan jobs failed find_package(SPIRV-Headers CONFIG REQUIRED) because the apt install omitted it. Added spirv-headers to both apt lines (exact parity with upstream llama.cpp's build-vulkan.yml: 'glslc libvulkan-dev spirv-headers'). 3. Windows arm64 (publish.yml + CLAUDE.md): ggml aborts 'MSVC is not supported for ARM, use clang'. The generator (Ninja) was never the issue — the compiler was. Switched the job to clang-cl (-DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl): it satisfies ggml's guard (if (MSVC AND NOT CMAKE_C_COMPILER_ID STREQUAL "Clang")) while keeping CMake's MSVC=TRUE, so our static /MT CRT block still applies and the runner + Ninja + ctest all stay. msvc-dev-cmd (arm64) supplies the MSVC headers/libs. First-run watch item: clang-cl must be on PATH (VS Clang component / LLVM); if CI reports it missing, add an LLVM setup step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HL7d4uQ3cKR5HwYFPvZvv7
1 parent dca1b20 commit 5664957

3 files changed

Lines changed: 113 additions & 4 deletions

File tree

.github/workflows/publish.yml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ jobs:
467467
- name: Install Vulkan SDK (headers + loader + glslc shader compiler)
468468
run: |
469469
sudo apt-get update
470-
sudo apt-get install -y libvulkan-dev glslc glslang-tools
470+
sudo apt-get install -y libvulkan-dev glslc glslang-tools spirv-headers
471471
- name: Build libraries
472472
shell: bash
473473
run: |
@@ -506,7 +506,7 @@ jobs:
506506
- name: Install toolchain (GCC 14) + Vulkan SDK
507507
run: |
508508
sudo apt-get update
509-
sudo apt-get install -y gcc-14 g++-14 libvulkan-dev glslc glslang-tools
509+
sudo apt-get install -y gcc-14 g++-14 libvulkan-dev glslc glslang-tools spirv-headers
510510
echo "CC=gcc-14" >> "$GITHUB_ENV"
511511
echo "CXX=g++-14" >> "$GITHUB_ENV"
512512
- name: Build libraries
@@ -881,6 +881,15 @@ jobs:
881881
# src/main/resources. sccache is intentionally omitted (the existing install step pulls the
882882
# x86_64 sccache zip; an arm64 build would need the aarch64 release — not worth the extra path
883883
# for one CPU job, so build.bat just builds uncached when sccache is absent).
884+
#
885+
# Compiler: clang-cl, NOT MSVC cl.exe. ggml's ggml-cpu/CMakeLists.txt aborts with "MSVC is not
886+
# supported for ARM, use clang" via `if (MSVC AND NOT CMAKE_C_COMPILER_ID STREQUAL "Clang")`.
887+
# clang-cl (LLVM's MSVC-compatible driver) satisfies that guard (its compiler id is "Clang")
888+
# while still leaving CMake's MSVC=TRUE, so our static /MT CRT block (CMAKE_MSVC_RUNTIME_LIBRARY
889+
# in CMakeLists.txt) keeps applying and the generator stays Ninja Multi-Config. msvc-dev-cmd
890+
# (arm64) supplies the MSVC headers/libs/linker that clang-cl links against. NOTE: clang-cl must
891+
# be on PATH (the VS "C++ Clang tools" component / LLVM); if a first CI run reports it missing,
892+
# add an LLVM setup step here.
884893
runs-on: windows-11-arm
885894
steps:
886895
- uses: actions/checkout@v7
@@ -898,8 +907,9 @@ jobs:
898907
# No mvn compile needed: the JNI header (jllama.h) is committed and the native build
899908
# uses the bundled JNI headers in .github/include, and OS_NAME/OS_ARCH are passed
900909
# explicitly (so the OSInfo-class OS-detection path is skipped) — same as the x86_64 job.
910+
# clang-cl (see the job comment) is required: ggml refuses MSVC cl.exe on ARM.
901911
run: |
902-
.github\build.bat -G "Ninja Multi-Config" -DOS_NAME=Windows -DOS_ARCH=aarch64 -DBUILD_TESTING=ON
912+
.github\build.bat -G "Ninja Multi-Config" -DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl -DOS_NAME=Windows -DOS_ARCH=aarch64 -DBUILD_TESTING=ON
903913
- name: Run C++ unit tests
904914
run: ctest --test-dir llama/build --output-on-failure
905915
- name: Upload artifacts

CLAUDE.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,15 @@ tree — so it ships in the **default** JAR alongside Windows x86-64 / x86 (like
295295
classifier). No Java change was needed: `OSInfo` already maps a Windows-on-ARM JVM (`os.arch=aarch64`)
296296
to `Windows/aarch64` (it isn't in `archMapping`, so it falls through `translateArchNameToFolderName`).
297297
sccache is intentionally omitted (the shared install step pulls the x86_64 sccache zip; not worth an
298-
arm64 path for one CPU job — `build.bat` just builds uncached).
298+
arm64 path for one CPU job — `build.bat` just builds uncached). **Compiler: `clang-cl`, not MSVC
299+
`cl.exe`.** ggml's `ggml-cpu/CMakeLists.txt` aborts with *"MSVC is not supported for ARM, use clang"*
300+
via `if (MSVC AND NOT CMAKE_C_COMPILER_ID STREQUAL "Clang")`; `clang-cl` (LLVM's MSVC-compatible driver)
301+
satisfies that guard (compiler id `"Clang"`) while keeping CMake's `MSVC=TRUE`, so the static `/MT` CRT
302+
block still applies and the generator stays Ninja Multi-Config. The job passes
303+
`-DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl`; `msvc-dev-cmd` supplies the MSVC
304+
headers/libs/linker `clang-cl` links against. (Upstream llama.cpp instead cross-compiles arm64 from an
305+
x64 runner with `vcvarsall amd64_arm64` + a `clang`/`clang++` toolchain file and no arm64 tests; the
306+
native-runner + `clang-cl` route here keeps the `/MT` CRT and lets `ctest` run on real ARM hardware.)
299307

300308
## WebUI (llama.cpp Svelte UI) embedding
301309

llama/spotbugs-exclude.xml

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,4 +622,95 @@ SPDX-License-Identifier: MIT
622622
<Method name="~\$default\$.*"/>
623623
</Match>
624624

625+
<!--
626+
NativeServer (net.ladenthin.llama.server.NativeServer) is the native-transport server-mode
627+
entry point: it owns a background worker thread, three native JNI methods and a blocking
628+
main(). Its SpotBugs findings are all fb-contrib style artifacts, not defects:
629+
- IMC_IMMATURE_CLASS_NO_EQUALS: an identity/lifecycle-managed handle class, not a value
630+
type — same rationale as the other server identity classes above.
631+
- MDM_THREAD_YIELD: main() polls `while (isRunning()) Thread.sleep(...)` to keep the JVM
632+
alive until the native server stops — a legitimate blocking wait in a process entry
633+
point, not manual thread scheduling.
634+
- UVA_USE_VAR_ARGS: startNativeServer(String[]) is a `native` method; its signature must
635+
match the C/JNI side exactly and therefore cannot be varargs.
636+
- WEM_WEAK_EXCEPTION_MESSAGING: the two start() guards throw with fixed messages
637+
(single-instance-per-process / already-running), which carry no extra state worth
638+
formatting — same category as the server request-parser WEM suppressions above.
639+
-->
640+
<Match>
641+
<Class name="net.ladenthin.llama.server.NativeServer"/>
642+
<Or>
643+
<Bug pattern="IMC_IMMATURE_CLASS_NO_EQUALS"/>
644+
<Bug pattern="MDM_THREAD_YIELD"/>
645+
<Bug pattern="UVA_USE_VAR_ARGS"/>
646+
<Bug pattern="WEM_WEAK_EXCEPTION_MESSAGING"/>
647+
</Or>
648+
</Match>
649+
650+
<!--
651+
ServerLauncher.main(String[]) is the fat-jar dispatcher: it forwards to
652+
OpenAiCompatServer.main / NativeServer.main, both of which declare `throws Exception` (they
653+
block until server shutdown and surface whatever the underlying server throws). Declaring
654+
`throws Exception` on the dispatcher mirrors that contract; narrowing it would force the
655+
dispatcher to wrap arbitrary checked exceptions for no benefit — same rationale as the
656+
ToolHandler.invoke THROWS suppression above.
657+
-->
658+
<Match>
659+
<Class name="net.ladenthin.llama.server.ServerLauncher"/>
660+
<Bug pattern="THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION"/>
661+
<Method name="main"/>
662+
</Match>
663+
664+
<!--
665+
OpenAiServerCli additions (the cache-type-k / cache-type-v, jinja and chat-template-kwargs
666+
flags plus the extended usage() help). All fb-contrib/findsecbugs artifacts, not defects; scoped
667+
to the specific methods that trigger them:
668+
- parse(): ENMI_NULL_ENUM_VALUE — `@Nullable CacheType cacheTypeK/V = null`; null is the
669+
documented "unset -> use the llama.cpp default" sentinel, explicitly @Nullable and
670+
NullAway-checked.
671+
- usage(): POTENTIAL_XML_INJECTION / PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS — usage() builds
672+
the plain-text console help string, appending the fixed cacheTypeChoices() enum list for
673+
the -ctk and -ctv lines. There is no XML anywhere (false positive), and cacheTypeChoices()
674+
is a cheap pure helper returning a constant, so calling it twice is harmless.
675+
- parseChatTemplateKwargs(): EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS converts Jackson's
676+
checked JsonProcessingException into a CLI usage error (cause chained) — the same intended
677+
boundary softening already accepted for LlamaModel's JSON methods above; and
678+
PSC_PRESIZE_COLLECTIONS on the tiny short-lived LinkedHashMap of CLI kwargs is not
679+
worthwhile.
680+
-->
681+
<Match>
682+
<Class name="net.ladenthin.llama.server.OpenAiServerCli"/>
683+
<Bug pattern="ENMI_NULL_ENUM_VALUE"/>
684+
<Method name="parse"/>
685+
</Match>
686+
<Match>
687+
<Class name="net.ladenthin.llama.server.OpenAiServerCli"/>
688+
<Or>
689+
<Bug pattern="POTENTIAL_XML_INJECTION"/>
690+
<Bug pattern="PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS"/>
691+
</Or>
692+
<Method name="usage"/>
693+
</Match>
694+
<Match>
695+
<Class name="net.ladenthin.llama.server.OpenAiServerCli"/>
696+
<Or>
697+
<Bug pattern="EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS"/>
698+
<Bug pattern="PSC_PRESIZE_COLLECTIONS"/>
699+
</Or>
700+
<Method name="parseChatTemplateKwargs"/>
701+
</Match>
702+
703+
<!--
704+
OpenAiServerCli$Options.getChatTemplateKwargs() returns the chatTemplateKwargs field
705+
directly, which fb-contrib flags as EI_EXPOSE_REP. The field only ever holds the
706+
Collections.unmodifiableMap(...) returned by parseChatTemplateKwargs (or null when unset),
707+
so the getter cannot leak a mutable map — the detector does not track that the stored map is
708+
already unmodifiable.
709+
-->
710+
<Match>
711+
<Class name="net.ladenthin.llama.server.OpenAiServerCli$Options"/>
712+
<Bug pattern="EI_EXPOSE_REP"/>
713+
<Method name="getChatTemplateKwargs"/>
714+
</Match>
715+
625716
</FindBugsFilter>

0 commit comments

Comments
 (0)