diff --git a/.github/build.bat b/.github/build.bat index 5b819d30..e4991a1a 100755 --- a/.github/build.bat +++ b/.github/build.bat @@ -4,9 +4,49 @@ REM REM SPDX-License-Identifier: MIT @echo off +setlocal enabledelayedexpansion + +REM --------------------------------------------------------------------------- +REM Optional shared compiler cache: sccache fronting Depot Cache (WebDAV). +REM Mirrors build.sh's sccache_can_wrap_compiler() probe. Because sccache *is* +REM the compiler launcher (cmake runs `sccache cl.exe ...` for every TU), a +REM present-but-crashing sccache would red every build; so we trust it only after +REM a trivial TU compiles *through* it. Enabled only when USE_CACHE=true AND +REM sccache is on PATH AND the probe succeeds; otherwise the build proceeds +REM uncached and green. The Visual Studio generator jobs do NOT set USE_CACHE, so +REM this stays inert for them (and the VS generator ignores +REM CMAKE_{C,CXX}_COMPILER_LAUNCHER anyway -- only Ninja/Makefiles honor it). +REM --------------------------------------------------------------------------- +set "LAUNCH=" +if /I "%USE_CACHE%"=="true" ( + where sccache >nul 2>&1 + if errorlevel 1 ( + echo build.bat: USE_CACHE=true but sccache not on PATH; building WITHOUT cache. + ) else ( + set "PROBE_DIR=%TEMP%\sccache-probe-%RANDOM%" + mkdir "!PROBE_DIR!" >nul 2>&1 + (echo int sccache_probe_verify = 0;)> "!PROBE_DIR!\probe.c" + sccache cl.exe /nologo /c "!PROBE_DIR!\probe.c" /Fo"!PROBE_DIR!\probe.obj" > "!PROBE_DIR!\probe.log" 2>&1 + if errorlevel 1 ( + echo build.bat: sccache probe FAILED wrapping cl.exe -- building WITHOUT cache. + type "!PROBE_DIR!\probe.log" + ) else ( + echo build.bat: sccache probe OK ^(wrapped cl.exe^). + set "LAUNCH=-DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache" + ) + rmdir /s /q "!PROBE_DIR!" >nul 2>&1 + ) +) mkdir build -cmake -Bbuild %* +cmake -Bbuild %LAUNCH% %* +if errorlevel 1 exit /b %ERRORLEVEL% cmake --build build --config Release +if errorlevel 1 exit /b %ERRORLEVEL% -if errorlevel 1 exit /b %ERRORLEVEL% \ No newline at end of file +REM Only query stats when sccache was actually wired in as the launcher; re-invoking +REM a rejected/crashing sccache here would just repeat its failure output. +if defined LAUNCH ( + echo build.bat: sccache --show-stats + sccache --show-stats +) diff --git a/.github/build.sh b/.github/build.sh index dec29e86..5b30ef9d 100755 --- a/.github/build.sh +++ b/.github/build.sh @@ -19,18 +19,24 @@ fi # Fetch sccache when caching is requested but the runner/container doesn't ship it — the # dockcross cross-compile containers (manylinux/Android) and Linux hosts have no sccache, # while macOS installs it via brew in the workflow. Best-effort and inert-safe: any failure -# leaves sccache absent, so the build just proceeds uncached. The static musl binary runs in -# any x86_64 Linux container (the cross-compile host is always x86_64). +# leaves sccache absent, so the build just proceeds uncached. A static musl release exists for +# both Linux arches we build on: x86_64 (the dockcross cross-compile hosts) and aarch64 (the +# native ubuntu-24.04-arm runner); any other arch leaves SCCACHE_DL_ARCH empty and is skipped. # # SCCACHE_DL_VERSION is overridable per-job, so a container that crashes one sccache build can # try another without editing this script (the in-container panic that stalled phase 2 was on # v0.8.2; v0.16.0 is the latest release and the default). A wrong/unavailable version just fails # the `curl -f` and falls back to an uncached build, so bumping it can never red a build. SCCACHE_DL_VERSION="${SCCACHE_DL_VERSION:-0.16.0}" +case "$(uname -m)" in + x86_64) SCCACHE_DL_ARCH="x86_64" ;; + aarch64|arm64) SCCACHE_DL_ARCH="aarch64" ;; + *) SCCACHE_DL_ARCH="" ;; +esac if [ "${USE_CACHE:-true}" = "true" ] && [ -n "${SCCACHE_WEBDAV_TOKEN:-}${SCCACHE_GHA_ENABLED:-}" ] \ && ! command -v sccache >/dev/null 2>&1 \ - && [ "$(uname -s)" = "Linux" ] && [ "$(uname -m)" = "x86_64" ]; then - SCCACHE_REL="sccache-v${SCCACHE_DL_VERSION}-x86_64-unknown-linux-musl" + && [ "$(uname -s)" = "Linux" ] && [ -n "$SCCACHE_DL_ARCH" ]; then + SCCACHE_REL="sccache-v${SCCACHE_DL_VERSION}-${SCCACHE_DL_ARCH}-unknown-linux-musl" echo "build.sh: fetching ${SCCACHE_REL} (no sccache on PATH)..." if curl -fsSL --proto =https --proto-redir =https \ "https://github.com/mozilla/sccache/releases/download/v${SCCACHE_DL_VERSION}/${SCCACHE_REL}.tar.gz" \ diff --git a/.github/workflows/clang-format.yml b/.github/workflows/clang-format.yml index ecd28782..ddc6f1c7 100644 --- a/.github/workflows/clang-format.yml +++ b/.github/workflows/clang-format.yml @@ -23,7 +23,11 @@ jobs: with: python-version: "3.x" - name: Install pinned clang-format - run: pip install "clang-format==${CLANG_FORMAT_VERSION}" + # --only-binary :all: forces pip to install from a prebuilt wheel and never run a + # source package's setup.py at install time (SonarCloud GHA security rule). clang-format + # ships manylinux wheels, so this resolves normally on the ubuntu runner. + run: | + pip install --only-binary :all: "clang-format==${CLANG_FORMAT_VERSION}" - name: Check C++ formatting run: | clang-format --version diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml index 38a987fe..2adcb882 100644 --- a/.github/workflows/osv-scanner.yml +++ b/.github/workflows/osv-scanner.yml @@ -13,7 +13,8 @@ on: - cron: '0 6 * * 1' workflow_dispatch: -permissions: read-all +permissions: + contents: read jobs: scan-scheduled: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 0bada429..36f52e71 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -261,17 +261,22 @@ jobs: path: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/ crosscompile-linux-aarch64: - name: Cross-Compile Linux aarch64 (LTS) + name: Build and Test Linux aarch64 needs: [startgate, build-webui] - runs-on: ubuntu-latest - # Phase 2 dockcross cache rollout — job 3. Same steady-state env as manylinux2014 (job 1); - # the build.sh probe makes it safe to enable without a separate verification run. Inert - # without DEPOT_TOKEN (fork PRs) or use_cache=false. + # Native ARM64 build on GitHub's free arm64 runner, mirroring upstream llama.cpp's + # `ubuntu-cpu` aarch64 release job (ubuntu-24.04-arm + GCC 14). Replaces the former dockcross + # `linux-arm64-lts` cross-compile (GCC 8.5, glibc 2.17), which can no longer compile llama.cpp + # b9739 — its C++17 CTAD-in-`new` needs GCC >= 12. Building natively also lets us run the C++ + # unit suite (ctest) on real ARM hardware for the first time (the cross build ran no tests). + # Trade-off: the glibc floor rises 2.17 -> ~2.39, the same envelope upstream's own ARM binaries + # require. GGML_NATIVE=OFF keeps the artifact portable across ARMv8 CPU generations (no + # build-host -march baked in). The job id is kept (a `needs:` target downstream); only the + # display name changed, so update any branch-protection required-check that pinned the old name. + runs-on: ubuntu-24.04-arm env: USE_CACHE: ${{ github.event_name != 'workflow_dispatch' || inputs.use_cache }} SCCACHE_WEBDAV_ENDPOINT: https://cache.depot.dev SCCACHE_WEBDAV_TOKEN: ${{ secrets.DEPOT_TOKEN }} - DOCKCROSS_ARGS: "-e SCCACHE_WEBDAV_ENDPOINT -e SCCACHE_WEBDAV_TOKEN -e USE_CACHE" steps: - uses: actions/checkout@v7 - name: Download shared WebUI assets @@ -279,6 +284,16 @@ jobs: with: name: webui-generated path: ${{ github.workspace }}/webui-generated/ + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: ${{ env.JAVA_VERSION }} + - name: Install toolchain (GCC 14, mirrors upstream llama.cpp ARM release) + run: | + sudo apt-get update + sudo apt-get install -y gcc-14 g++-14 + echo "CC=gcc-14" >> "$GITHUB_ENV" + echo "CXX=g++-14" >> "$GITHUB_ENV" - name: Display CPU Info shell: bash run: | @@ -290,7 +305,10 @@ jobs: - name: Build libraries shell: bash run: | - .github/dockcross/dockcross-linux-arm64-lts .github/build.sh "-DOS_NAME=Linux -DOS_ARCH=aarch64" + mvn --no-transfer-progress compile + .github/build.sh "-DOS_NAME=Linux -DOS_ARCH=aarch64 -DGGML_NATIVE=OFF -DBUILD_TESTING=ON" + - name: Run C++ unit tests + run: ctest --test-dir build --output-on-failure - name: Upload artifacts uses: actions/upload-artifact@v7 with: @@ -526,6 +544,128 @@ jobs: name: Windows-x86-libraries path: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/ + # --------------------------------------------------------------------------- + # Windows Ninja Multi-Config + sccache — EVALUATION jobs (not yet released). + # The Visual Studio generator ignores CMAKE_{C,CXX}_COMPILER_LAUNCHER, so the two + # build-windows-* jobs above are the only uncached native builds. Upstream + # llama.cpp ships its windows-cuda artifact with "Ninja Multi-Config" + MSVC, + # which proves the combination works on the same tree. These two jobs run that + # combination in parallel with the trusted VS jobs and front cl.exe with sccache + # over Depot WebDAV (build.bat probe-guards it). Artifacts are named + # `Windows-*-ninja` (NOT `*-libraries`) so the package job's `pattern: "*-libraries"` + # does NOT pick them up — they are evaluation-only until cache hits are confirmed, + # at which point the release path is switched over (see TODO.md). The package job's + # `needs:` is intentionally left unchanged. + # --------------------------------------------------------------------------- + + build-windows-x86_64-ninja: + name: Build and Test Windows 2025 x86_64 (Ninja Multi-Config + sccache, eval) + needs: [startgate, build-webui] + runs-on: windows-2025-vs2026 + env: + USE_CACHE: ${{ github.event_name != 'workflow_dispatch' || inputs.use_cache }} + SCCACHE_WEBDAV_ENDPOINT: https://cache.depot.dev + SCCACHE_WEBDAV_TOKEN: ${{ secrets.DEPOT_TOKEN }} + steps: + - uses: actions/checkout@v7 + - name: Download shared WebUI assets + uses: actions/download-artifact@v8 + with: + name: webui-generated + path: ${{ github.workspace }}/webui-generated/ + - name: Set up MSVC developer environment (x64) + uses: ilammy/msvc-dev-cmd@v1 + with: + arch: x64 + - name: Install sccache (shared compiler cache) + if: env.USE_CACHE == 'true' + continue-on-error: true + shell: pwsh + run: | + $ver = "0.16.0" + $rel = "sccache-v$ver-x86_64-pc-windows-msvc" + $url = "https://github.com/mozilla/sccache/releases/download/v$ver/$rel.zip" + Write-Host "Downloading $url" + Invoke-WebRequest -Uri $url -OutFile "$env:RUNNER_TEMP\sccache.zip" + Expand-Archive -Path "$env:RUNNER_TEMP\sccache.zip" -DestinationPath "$env:RUNNER_TEMP\sccache" -Force + Add-Content -Path $env:GITHUB_PATH -Value "$env:RUNNER_TEMP\sccache\$rel" + - name: Display CPU Info + shell: pwsh + run: | + Write-Host "=== CPU Information (Get-CimInstance - All Properties) ===" + Get-CimInstance Win32_Processor | Select-Object * | Format-List + Write-Host "" + Write-Host "=== CPU Information (systeminfo) ===" + systeminfo | Select-String "Processor" + Write-Host "" + Write-Host "=== CPU Information (Get-ComputerInfo) ===" + Get-ComputerInfo -Property "CsProcessors*" 2>$null || Write-Host "Get-ComputerInfo not available" + - name: Build libraries + shell: cmd + run: | + .github\build.bat -G "Ninja Multi-Config" -DOS_NAME=Windows -DOS_ARCH=x86_64 -DBUILD_TESTING=ON + - name: Run C++ unit tests + run: ctest --test-dir build --output-on-failure + - name: Upload artifacts + uses: actions/upload-artifact@v7 + with: + name: Windows-x86_64-ninja + path: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/ + + build-windows-x86-ninja: + name: Build and Test Windows 2025 x86 (Ninja Multi-Config + sccache, eval) + needs: [startgate, build-webui] + runs-on: windows-2025-vs2026 + env: + USE_CACHE: ${{ github.event_name != 'workflow_dispatch' || inputs.use_cache }} + SCCACHE_WEBDAV_ENDPOINT: https://cache.depot.dev + SCCACHE_WEBDAV_TOKEN: ${{ secrets.DEPOT_TOKEN }} + steps: + - uses: actions/checkout@v7 + - name: Download shared WebUI assets + uses: actions/download-artifact@v8 + with: + name: webui-generated + path: ${{ github.workspace }}/webui-generated/ + - name: Set up MSVC developer environment (x86) + uses: ilammy/msvc-dev-cmd@v1 + with: + arch: x86 + - name: Install sccache (shared compiler cache) + if: env.USE_CACHE == 'true' + continue-on-error: true + shell: pwsh + run: | + $ver = "0.16.0" + $rel = "sccache-v$ver-x86_64-pc-windows-msvc" + $url = "https://github.com/mozilla/sccache/releases/download/v$ver/$rel.zip" + Write-Host "Downloading $url" + Invoke-WebRequest -Uri $url -OutFile "$env:RUNNER_TEMP\sccache.zip" + Expand-Archive -Path "$env:RUNNER_TEMP\sccache.zip" -DestinationPath "$env:RUNNER_TEMP\sccache" -Force + Add-Content -Path $env:GITHUB_PATH -Value "$env:RUNNER_TEMP\sccache\$rel" + - name: Display CPU Info + shell: pwsh + run: | + Write-Host "=== CPU Information (Get-CimInstance - All Properties) ===" + Get-CimInstance Win32_Processor | Select-Object * | Format-List + Write-Host "" + Write-Host "=== CPU Information (systeminfo) ===" + systeminfo | Select-String "Processor" + Write-Host "" + Write-Host "=== CPU Information (Get-ComputerInfo) ===" + Get-ComputerInfo -Property "CsProcessors*" 2>$null || Write-Host "Get-ComputerInfo not available" + - name: Build libraries + shell: cmd + run: | + .github\build.bat -G "Ninja Multi-Config" -DOS_NAME=Windows -DOS_ARCH=x86 -DBUILD_TESTING=ON + - name: Run C++ unit tests + run: ctest --test-dir build --output-on-failure + - name: Upload artifacts + uses: actions/upload-artifact@v7 + with: + name: Windows-x86-ninja + path: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/ + # --------------------------------------------------------------------------- # CI-only jobs — no release artifact, purely for test coverage # --------------------------------------------------------------------------- @@ -1069,6 +1209,99 @@ jobs: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/**/* if-no-files-found: warn + # Java/inference validation of the Ninja-built x86_64 DLL (the analogue of + # test-java-windows-x86_64 for the MSVC build). Loads the Ninja jllama.dll via + # JNI and runs the full model-backed suite, so both Windows generators are + # validated end-to-end before the `ninja-windows` classifier JAR ships. + test-java-windows-x86_64-ninja: + name: Java Tests Windows 2025 x86_64 (Ninja, eval) + needs: build-windows-x86_64-ninja + runs-on: windows-2025-vs2026 + steps: + - uses: actions/checkout@v7 + - name: Display CPU Info + shell: pwsh + run: | + Write-Host "=== CPU Information (Get-CimInstance - All Properties) ===" + Get-CimInstance Win32_Processor | Select-Object * | Format-List + Write-Host "" + Write-Host "=== CPU Information (systeminfo) ===" + systeminfo | Select-String "Processor" + Write-Host "" + Write-Host "=== CPU Information (Get-ComputerInfo) ===" + Get-ComputerInfo -Property "CsProcessors*" 2>$null || Write-Host "Get-ComputerInfo not available" + - uses: actions/download-artifact@v8 + with: + name: Windows-x86_64-ninja + path: ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/ + - name: Download text generation model + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:MODEL_URL --create-dirs -o models/$env:MODEL_NAME + - name: Download reranking model + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:RERANKING_MODEL_URL --create-dirs -o models/$env:RERANKING_MODEL_NAME + - name: Download draft model + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:DRAFT_MODEL_URL --create-dirs -o models/$env:DRAFT_MODEL_NAME + - name: Download reasoning model + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:REASONING_MODEL_URL --create-dirs -o models/$env:REASONING_MODEL_NAME + - name: Download tool-calling model + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:TOOL_MODEL_URL --create-dirs -o models/$env:TOOL_MODEL_NAME + - name: Download vision model (issues #103 / #34) + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:VISION_MODEL_URL --create-dirs -o models/$env:VISION_MODEL_NAME + - name: Download vision mmproj + run: curl -L --proto =https --proto-redir =https --fail --retry 5 --retry-all-errors $env:VISION_MMPROJ_URL --create-dirs -o models/$env:VISION_MMPROJ_NAME + - name: List files in models directory + run: ls -l models/ + - name: Validate model files + run: .github\validate-models.bat + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: ${{ env.JAVA_VERSION }} + - name: Memory before tests + run: Get-CimInstance Win32_OperatingSystem | Select-Object FreePhysicalMemory,TotalVisibleMemorySize | Format-List + shell: pwsh + - name: Enable WER LocalDumps for java.exe + # Windows Error Reporting writes minidumps when java.exe (or any other + # registered process) crashes via __fastfail / abort / unhandled SEH. + # We use it as the Windows analogue of Linux core dumps so that a JVM + # crash inside the JNI layer leaves us a real native callstack instead + # of just surefire's "VM terminated without saying goodbye" line. + # DumpType=2 == MiniDumpWithFullMemory; the workspace dumps/ folder is + # globbed by the failure-upload step below. + shell: pwsh + run: | + $key = 'HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\java.exe' + New-Item -Path $key -Force | Out-Null + New-Item -Path "${{ github.workspace }}\dumps" -ItemType Directory -Force | Out-Null + New-ItemProperty -Path $key -Name 'DumpFolder' -Value "${{ github.workspace }}\dumps" -PropertyType ExpandString -Force | Out-Null + New-ItemProperty -Path $key -Name 'DumpType' -Value 2 -PropertyType DWord -Force | Out-Null + New-ItemProperty -Path $key -Name 'DumpCount' -Value 5 -PropertyType DWord -Force | Out-Null + Get-ItemProperty -Path $key | Format-List + - name: Run tests + run: | + mvn -e --no-transfer-progress test ` + "-Dnet.ladenthin.llama.tool.model=models/$env:TOOL_MODEL_NAME" ` + "-Dnet.ladenthin.llama.vision.model=models/$env:VISION_MODEL_NAME" ` + "-Dnet.ladenthin.llama.vision.mmproj=models/$env:VISION_MMPROJ_NAME" ` + "-Dnet.ladenthin.llama.vision.image=$env:VISION_IMAGE_PATH" + - name: Memory after tests + if: always() + run: Get-CimInstance Win32_OperatingSystem | Select-Object FreePhysicalMemory,TotalVisibleMemorySize | Format-List + shell: pwsh + - if: failure() + uses: actions/upload-artifact@v7 + with: + name: windows-output-ninja + path: | + ${{ github.workspace }}\hs_err_pid*.log + ${{ github.workspace }}\*.hprof + ${{ github.workspace }}\dumps\*.dmp + ${{ github.workspace }}\target\surefire-reports\*.dump + ${{ github.workspace }}\target\surefire-reports\*.dumpstream + ${{ github.workspace }}\target\surefire-reports\*.txt + ${{ github.workspace }}\target\surefire-reports\TEST-*.xml + ${{ github.workspace }}/src/main/resources/net/ladenthin/llama/**/* + if-no-files-found: warn + # --------------------------------------------------------------------------- # Package and publish # --------------------------------------------------------------------------- @@ -1081,6 +1314,8 @@ jobs: - crosscompile-android-aarch64 - crosscompile-android-aarch64-opencl - build-windows-x86 + - build-windows-x86_64-ninja + - build-windows-x86-ninja - test-cpp-linux-x86_64 - build-macos-arm64-metal-15 - test-java-linux-x86_64 @@ -1088,6 +1323,7 @@ jobs: - test-java-macos-arm64-no-metal - test-java-macos-arm64-metal-15 - test-java-windows-x86_64 + - test-java-windows-x86_64-ninja runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -1104,6 +1340,17 @@ jobs: with: name: android-libraries-opencl path: ${{ github.workspace }}/src/main/resources_android_opencl/net/ladenthin/llama/ + # Ninja-built Windows natives -> separate tree consumed by the `windows-ninja` + # Maven profile (the `ninja-windows` classifier JAR). The default JAR keeps the + # MSVC `*-libraries` natives downloaded above. + - uses: actions/download-artifact@v8 + with: + name: Windows-x86_64-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ + - uses: actions/download-artifact@v8 + with: + name: Windows-x86-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ - uses: actions/setup-java@v5 with: distribution: 'temurin' @@ -1114,7 +1361,8 @@ jobs: # default-platform native libs in one drop-on-classpath JAR, runnable via its # OpenAiCompatServer Main-Class). It lands in target/ and is uploaded in the `llama-jars` # artifact below - a CI run artifact only, not a Maven Central / GitHub-Release asset. - run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android,assembly -Dmaven.test.skip=true -Dgpg.skip=true package + # `windows-ninja` attaches the `ninja-windows` classifier JAR (Ninja-built Windows natives). + run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android,windows-ninja,assembly -Dmaven.test.skip=true -Dgpg.skip=true package - name: Upload JARs uses: actions/upload-artifact@v7 with: @@ -1192,6 +1440,14 @@ jobs: with: name: android-libraries-opencl path: ${{ github.workspace }}/src/main/resources_android_opencl/net/ladenthin/llama/ + - uses: actions/download-artifact@v8 + with: + name: Windows-x86_64-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ + - uses: actions/download-artifact@v8 + with: + name: Windows-x86-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ - name: Set up Maven Central Repository uses: actions/setup-java@v5 with: @@ -1212,7 +1468,7 @@ jobs: *) echo "::error::Refusing to publish non-SNAPSHOT version '$VERSION' from the snapshot job. Snapshot publishing requires a -SNAPSHOT version; releases go through the v* tag path."; exit 1 ;; esac - name: Publish snapshot - run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android -Dmaven.test.skip=true deploy + run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android,windows-ninja -Dmaven.test.skip=true deploy env: MAVEN_USERNAME: ${{ secrets.CENTRAL_USERNAME }} MAVEN_PASSWORD: ${{ secrets.CENTRAL_TOKEN }} @@ -1276,6 +1532,14 @@ jobs: with: name: android-libraries-opencl path: ${{ github.workspace }}/src/main/resources_android_opencl/net/ladenthin/llama/ + - uses: actions/download-artifact@v8 + with: + name: Windows-x86_64-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ + - uses: actions/download-artifact@v8 + with: + name: Windows-x86-ninja + path: ${{ github.workspace }}/src/main/resources_windows_ninja/net/ladenthin/llama/ - name: Set up Maven Central Repository uses: actions/setup-java@v5 with: @@ -1287,7 +1551,7 @@ jobs: gpg-private-key: ${{ secrets.GPG_PRIVATE_KEY }} gpg-passphrase: MAVEN_GPG_PASSPHRASE - name: Publish release - run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android -Dmaven.test.skip=true deploy + run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android,windows-ninja -Dmaven.test.skip=true deploy env: MAVEN_USERNAME: ${{ secrets.CENTRAL_USERNAME }} MAVEN_PASSWORD: ${{ secrets.CENTRAL_TOKEN }} diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index d0007805..618b9af4 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -12,7 +12,8 @@ on: branches: [ main ] workflow_dispatch: -permissions: read-all +permissions: + contents: read jobs: analysis: diff --git a/.gitignore b/.gitignore index d22d4433..8aabd814 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ replay_pid* models/*.gguf src/main/cpp/net_ladenthin_llama_*.h src/main/resources_cuda_linux/ +src/main/resources_windows_ninja/ src/main/resources/**/*.so src/main/resources/**/*.dylib src/main/resources/**/*.dll diff --git a/CLAUDE.md b/CLAUDE.md index 69d41dfe..90c08879 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -160,6 +160,51 @@ At runtime the device must provide its own OpenCL ICD (`libOpenCL.so`); Qualcomm Adreno drivers do. Devices without an ICD should use the default CPU-only Android JAR. +## Windows Ninja artifact (sccache-cached, parallel to the MSVC build) + +The Visual Studio generator ignores `CMAKE_{C,CXX}_COMPILER_LAUNCHER`, so the two MSVC Windows +jobs (`build-windows-x86_64`, `build-windows-x86`) **cannot** use the sccache/Depot cache. Rather +than switch the trusted MSVC build, the repo builds the **same CPU natives a second time** with the +**`Ninja Multi-Config`** generator (which *does* honor the launcher) and ships them as a separate +**`ninja-windows`** Maven classifier JAR. **The MSVC build is the default JAR and is kept +permanently** — the Ninja artifact is an additional, cache-accelerated, independently +end-to-end-tested option, not a replacement. (Upstream llama.cpp ships its `windows-cuda` artifact +with Ninja Multi-Config + MSVC, proving the combination works on the same tree.) + +Unlike the CUDA / OpenCL classifiers — which differ by a **GGML backend flag** and route their +output in `CMakeLists.txt` — the Ninja Windows build differs only by **generator/toolchain**, so +there is **no `CMakeLists.txt` change**: both generators emit to the canonical +`src/main/resources/.../Windows/{x86_64,x86}/`. Routing to the classifier tree happens purely at the +CI-download + pom-profile level. Four places wire it together: + +1. **`.github/build.bat`** — sccache probe guard mirroring `build.sh`'s `sccache_can_wrap_compiler()`: + when `USE_CACHE=true` and `sccache` is on PATH, it compiles a trivial TU through `sccache cl.exe`; + only on success does it pass `-DCMAKE_{C,CXX}_COMPILER_LAUNCHER=sccache` and print + `sccache --show-stats`. A missing/crashing sccache falls back to a green uncached build. The MSVC + jobs do not set `USE_CACHE`, so the guard is inert for them. +2. **`.github/workflows/publish.yml`** — build jobs `build-windows-x86_64-ninja` / + `build-windows-x86-ninja` (`windows-2025-vs2026`, `ilammy/msvc-dev-cmd@v1` for the arch env, + sccache v0.16.0 from the GitHub release **zip** + Depot WebDAV, `build.bat -G "Ninja Multi-Config"`), + uploading artifacts `Windows-{x86_64,x86}-ninja` (**not** `*-libraries`, so the `package` job's + `pattern: "*-libraries"` ignores them). `test-java-windows-x86_64-ninja` loads the Ninja DLL via + JNI and runs the full model-backed suite. The `package`, `publish-snapshot`, and `publish-release` + jobs download `Windows-*-ninja` into `src/main/resources_windows_ninja/` and activate the + `windows-ninja` Maven profile. +3. **`pom.xml`** — the `windows-ninja` profile produces a second JAR with `ninja-windows` + from the `${project.build.outputDirectory}_windows_ninja` tree (separate compile pass + resource + copy + classified jar; mirrors the `cuda` / `opencl-android` profiles). Activated only in CI. +4. **`README.md`** — the `ninja-windows` row + dependency snippet in "Choosing the right classifier". + +`src/main/resources_windows_ninja/` is git-ignored (staged by CI, never committed — same policy as +the native libs and the CUDA/OpenCL trees). + +**Local sanity build** (needs MSVC + a Ninja on PATH; sccache optional): +```bat +mvn -q compile +.github\build.bat -G "Ninja Multi-Config" -DOS_NAME=Windows -DOS_ARCH=x86_64 -DBUILD_TESTING=ON +ctest --test-dir build --output-on-failure +``` + ## WebUI (llama.cpp Svelte UI) embedding The llama.cpp WebUI is **built once in CI and shared to every native build**, then @@ -261,7 +306,10 @@ v0.16.0 + the probe this is no longer a risk.) Job-by-job status: + ggml + httplib); the nvcc `.cu` kernels won't (limited sccache nvcc support) — still a large partial win on the ~70 min full-arch job; the fast single-arch (sm_120) validation path cuts nvcc time independently of sccache. -3. `crosscompile-linux-aarch64` — ✅ **enabled** (same steady-state env; probe guards it). +3. `crosscompile-linux-aarch64` — ✅ **enabled**, now a **native `ubuntu-24.04-arm` build** (not + dockcross): `build.sh` self-fetches the aarch64 static-musl sccache (the fetch block in + `build.sh` maps `uname -m` → `x86_64`/`aarch64`) and the probe guards it. See "Linux aarch64: + native ARM build" below for why it moved off the cross-compiler. 4. `crosscompile-android-aarch64` — ✅ **enabled** (same steady-state env; probe guards it). 5. `crosscompile-android-aarch64-opencl` — ✅ **enabled**. `build_opencl_android.sh` stages the OpenCL headers/loader, then delegates the jllama cmake build to `build.sh` via `exec` @@ -271,9 +319,11 @@ Per-job recipe: add `env:` { `USE_CACHE`, `SCCACHE_WEBDAV_ENDPOINT`, `SCCACHE_WE `DOCKCROSS_ARGS: "-e SCCACHE_WEBDAV_ENDPOINT -e SCCACHE_WEBDAV_TOKEN -e USE_CACHE"` — the dockcross wrapper only forwards host env it is explicitly told to via `-e`. The fetched sccache version is the `SCCACHE_DL_VERSION` knob in `build.sh` (default **0.16.0**; overridable per-job -to try a different build against a container that crashed another). **Windows** (`build.bat` + -MSVC) is separate and last: use `mozilla-actions/sccache-action` / sccache's MSVC support, not -the `build.sh` musl fetch. +to try a different build against a container that crashed another). **Windows** is handled +separately (the Visual Studio generator ignores `CMAKE_*_COMPILER_LAUNCHER`): see +"Windows Ninja artifact" below — the cached path uses the **Ninja Multi-Config** generator with a +`build.bat` sccache probe and a direct sccache zip download (not `mozilla-actions/sccache-action`), +shipped as a parallel `ninja-windows` classifier JAR while the MSVC default stays the trusted build. **Cross-repo scope.** This Depot/sccache compiler cache makes sense only for java-llama.cpp — it is the only sibling repo with a native (C++/JNI) build. It does not apply to the pure-Maven @@ -281,9 +331,35 @@ siblings; why (and why the `DEPOT_TOKEN` org secret and the README "Build cache are kept jllama-only) is explained in the cross-repo status under "Deliberate non-parity": [`../workspace/crossrepostatus.md`](../workspace/crossrepostatus.md). +## Local llama.cpp source patches (`patches/`) + +The fetched llama.cpp source is patched before it compiles, via a generic mechanism: + +- **`patches/`** (repo root) — drop any number of `*.patch` / `*.diff` files here. They are applied + in **filename order** (use a numeric prefix, e.g. `0001-`, `0002-`), so keep them independent or + ordered. Each must be a `git apply`-compatible unified diff with paths relative to the llama.cpp + source root (`a/common/arg.cpp` / `b/common/arg.cpp`, i.e. `-p1`). +- **`cmake/apply-llama-patches.cmake`** — the applier. Cross-platform (`cmake -P`, so identical on + Linux/macOS/Windows), **idempotent** (`git apply --reverse --check` skips already-applied patches + so a reconfigure never double-applies) and **fail-loud** (a patch that no longer applies aborts + the configure — a stale patch can't be silently dropped from a release build). +- **`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. + +**On a llama.cpp version bump, every patch must still apply** — if a bump shifts the patched code, +the configure fails with an "does not apply cleanly" error; refresh the diff against the new source +and recommit. Treat `patches/` as part of the upgrade checklist below. + +Current patches: + +| Patch | Fixes | +|-------|-------| +| `0001-win32-arg-parse-embed-guard.patch` | Windows JNI regression from llama.cpp **#24779** (b9739): `common_params_parse` unconditionally replaced the caller's argv with the process command line (`GetCommandLineW`), so an embedded/JNI caller (`java.exe`) lost its `--model …` args → "Failed to parse model parameters". The patch **drops the override for our build** (keeps the `make_utf8_argv()` call referenced so there's no `-Wunused-function`, but never adopts its result), so the caller's already-UTF-8 argv is always used. This is **deterministic** — an earlier count-guard variant (only override when the re-derived arg count equals `argc`) collided on the server-integration tests whose argv length happened to equal `java.exe`'s and kept them failing. The upstream PR can instead expose an opt-out / `common_params_parse_argv` that preserves the standalone tools' UTF-8 fix. | + ## Upgrading/Downgrading llama.cpp Version -To change the llama.cpp version, update the following **three** files: +To change the llama.cpp version, update the following **three** files (and re-verify `patches/`): 1. **CMakeLists.txt** — the `GIT_TAG` line for llama.cpp: `GIT_TAG b8831` 2. **README.md** — the badge and link line with the version number @@ -723,7 +799,35 @@ Java parameters are serialized to JSON strings and passed to native code, which 3. Extracts from JAR resources at `net/ladenthin/llama/{os}/{arch}/` ### Cross-compilation -Docker-based cross-compilation scripts are in `.github/dockcross/` for ARM/Android targets. CI workflows use these for non-x86 Linux builds. +Docker-based cross-compilation scripts are in `.github/dockcross/` for **Android** targets (and the +x86_64 manylinux jobs). **Linux `aarch64` is no longer cross-compiled** — it builds natively on a +GitHub `ubuntu-24.04-arm` runner (see "Linux aarch64: native ARM build" below). The +`.github/dockcross/dockcross-linux-arm64-lts` wrapper is now unused by CI (left in place; harmless). + +### Linux aarch64: native ARM build + +The `crosscompile-linux-aarch64` job (id kept for its downstream `needs:` reference; display name is +now **"Build and Test Linux aarch64"**) builds **natively on `ubuntu-24.04-arm`**, mirroring upstream +llama.cpp's own `ubuntu-cpu` aarch64 release job (`ubuntu-24.04-arm` + **GCC 14**). + +**Why it moved off dockcross.** The old `dockcross/linux-arm64-lts` image ships **GCC 8.5 / glibc +2.17**; llama.cpp **b9739** uses C++17 CTAD-in-`new`, which needs **GCC ≥ 12**, so the cross build +stopped compiling. Upstream solved the same problem by building natively on `ubuntu-24.04-arm` with +GCC 14 and ships a **glibc ≈ 2.39** ARM binary with no old-glibc compatibility layer. This repo now +does the same: the aarch64 artifact's **glibc floor rises 2.17 → ~2.39** — the same envelope +upstream's own ARM binaries require (the x86_64 artifact stays at manylinux2014 / glibc 2.17). + +Wiring (mirrors the macOS native jobs, not the dockcross jobs): +- `runs-on: ubuntu-24.04-arm`; `setup-java` → `mvn compile` (generates the JNI header) → `build.sh`. +- Installs `gcc-14`/`g++-14` and exports `CC`/`CXX` (upstream parity). +- `build.sh` flags: `-DGGML_NATIVE=OFF` (portable across ARMv8 CPU generations — no build-host + `-march` baked in) `-DBUILD_TESTING=ON`, then **`ctest` runs the C++ unit suite on real ARM + hardware** (the cross build ran no tests at all). +- sccache: `build.sh`'s Linux auto-fetch now covers `aarch64` as well as `x86_64` (it maps + `uname -m` to the matching static-musl release); the probe still gates it, so a miss just builds + uncached. +- Branch protection: if a required check pinned the old name "Cross-Compile Linux aarch64 (LTS)", + repoint it to "Build and Test Linux aarch64". ## Testing diff --git a/CMakeLists.txt b/CMakeLists.txt index 0578ca7e..37724e54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -136,10 +136,18 @@ set(GGML_AVX512 OFF CACHE BOOL "" FORCE) set(LLAMA_BUILD_UI OFF CACHE BOOL "" FORCE) # b9284 flipped LLAMA_BUILD_APP default to ON; we don't build the unified binary set(LLAMA_BUILD_APP OFF CACHE BOOL "" FORCE) +# Local source patches for the fetched llama.cpp tree. Every patches/*.patch|*.diff is applied +# (sorted, idempotently, fail-loud) by cmake/apply-llama-patches.cmake — see that file's header. +# This runs for every C++ build (all CI jobs + local) from one place. is substituted +# by FetchContent/ExternalProject to the fetched llama.cpp source root. FetchContent_Declare( llama.cpp GIT_REPOSITORY https://github.com/ggerganov/llama.cpp.git GIT_TAG b9739 + PATCH_COMMAND ${CMAKE_COMMAND} + -DPATCH_DIR=${CMAKE_CURRENT_SOURCE_DIR}/patches + -DLLAMA_SRC= + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/apply-llama-patches.cmake ) FetchContent_MakeAvailable(llama.cpp) @@ -408,6 +416,7 @@ if(BUILD_TESTING) ${llama.cpp_SOURCE_DIR}/tools/server/server-context.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-queue.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-task.cpp + ${llama.cpp_SOURCE_DIR}/tools/server/server-schema.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-models.cpp ) diff --git a/README.md b/README.md index 3c5244a8..21f3a88f 100644 --- a/README.md +++ b/README.md @@ -157,14 +157,16 @@ If any of these match your platform, you can include the Maven dependency and ge ### Choosing the right classifier The Maven coordinate `net.ladenthin:llama` publishes one default JAR (CPU-only) -plus two optional GPU/accelerator JARs selected via a Maven ``. -Pick at most one — they are mutually exclusive. +plus optional JARs selected via a Maven ``: two GPU/accelerator +builds and one alternate-toolchain Windows build. Pick at most one GPU/accelerator +classifier — those are mutually exclusive — and optionally the Windows build. | Classifier | Backend | Target platform | Runtime requirement | |---|---|---|---| -| _(none)_ | CPU | Linux x86-64 / aarch64, macOS x86-64 / aarch64, Windows x86-64, Android aarch64 (CPU) | None beyond a JDK 8+ JVM | +| _(none)_ | CPU | Linux x86-64 / aarch64, macOS x86-64 / aarch64, Windows x86-64 (MSVC / Visual Studio generator), Android aarch64 (CPU) | A JDK 8+ JVM. **Linux `aarch64` additionally requires glibc ≥ 2.39** (e.g. Ubuntu 24.04+, Debian 13+) — it is built natively on `ubuntu-24.04-arm`, matching upstream llama.cpp's own ARM binaries; older-glibc ARM hosts (Ubuntu 22.04, Debian 12, RHEL 8/9, Amazon Linux 2023) are not supported. Linux x86-64 keeps a glibc 2.17 floor (manylinux2014). | | `cuda13-linux-x86-64` | CUDA 13 | Linux x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 runtime libraries (`libcudart.so.13`, `libcublas.so.13`) installed on the host. The shared library is dynamically linked against them and will fail to `dlopen` if they are absent — there is no automatic fallback to CPU. | | `opencl-android-aarch64` | OpenCL (Adreno) | Android aarch64 with Qualcomm Adreno GPU | A device-supplied OpenCL ICD (`libOpenCL.so`). Devices without an ICD (e.g. most non-Snapdragon Android hardware) must use the default CPU JAR. | +| `ninja-windows` | CPU (Ninja Multi-Config + MSVC) | Windows x86-64 and x86 | None beyond a JDK 8+ JVM. Same CPU backend as the default JAR's Windows natives, but compiled with the `Ninja Multi-Config` generator (sccache-cached in CI) instead of the Visual Studio generator. Provided so both Windows builds are available; functionally equivalent for normal use. | ```xml @@ -189,6 +191,14 @@ Pick at most one — they are mutually exclusive. 5.0.2 opencl-android-aarch64 + + + + net.ladenthin + llama + 5.0.2 + ninja-windows + ``` > [!IMPORTANT] diff --git a/REUSE.toml b/REUSE.toml index 2211a30b..7fc518c1 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -74,6 +74,14 @@ SPDX-FileCopyrightText = [ ] SPDX-License-Identifier = "MIT" +# llama.cpp source patches (unified-diff format cannot carry inline SPDX without corrupting the +# diff). Glob covers every current and future patch dropped into patches/ — see CLAUDE.md +# "Local llama.cpp source patches". +[[annotations]] +path = "patches/**" +SPDX-FileCopyrightText = "2026 Bernard Ladenthin " +SPDX-License-Identifier = "MIT" + # Test image (binary, cannot carry inline SPDX) [[annotations]] path = "src/test/resources/images/test-image.jpg" diff --git a/TODO.md b/TODO.md index 15233961..2405ddb5 100644 --- a/TODO.md +++ b/TODO.md @@ -30,7 +30,7 @@ NanoHTTPD server; NanoHTTPD + its dependency deleted.) primary goal: agentic tool-calling with Qwen): - Agentic tool-calling verified wire-correct: C++ guard pins `tool_calls.function.arguments` as a JSON - **string** (not object) at b9682 (llama.cpp #20198), plus the existing `finish_reason:"tool_calls"` + **string** (not object) at b9739 (llama.cpp #20198), plus the existing `finish_reason:"tool_calls"` test. - `stream_options.include_usage` forwarded (new `InferenceParameters.withStreamOptions`) so the trailing usage chunk is emitted, and `OpenAiSseFormatter.ensureUsageCachedTokens` guarantees @@ -82,58 +82,180 @@ primary goal: agentic tool-calling with Qwen): What remains is manual validation against the actual editor clients — point Copilot's Ollama provider / a Custom Endpoint, Claude Code, and a Responses client at the running server — since a server-side round-trip confirms the wire shapes but not each client's own parser. -- **Gemma 4 tool-calling validation.** Confirm the pinned llama.cpp (`b9682`) includes the Gemma 4 +- **Gemma 4 tool-calling validation.** Confirm the pinned llama.cpp (`b9739`) includes the Gemma 4 tool-call parser fixes; if not, bump per the upgrade procedure. - -### Windows compiler cache (sccache) — deferred: needs Ninja; evaluate dual-artifact - -The two Windows native build jobs (`build-windows-x86_64`, `build-windows-x86`) are the **only -remaining uncached** native builds — the 3 macOS jobs and all 5 dockcross jobs now cache via -sccache + Depot. Windows is not yet wired up because of a hard CMake constraint, and the chosen -path is to validate it carefully rather than flip the working build in place. - -**Why the obvious fix doesn't work.** Our cache mechanism is the CMake *compiler launcher* -(`-DCMAKE_C_COMPILER_LAUNCHER=sccache`, set by `build.sh`). ggml has its own equivalent -(`GGML_CCACHE` → `RULE_LAUNCH_COMPILE`). **Both are honored only by the Ninja and Makefile -generators — the Visual Studio generator ignores them entirely.** Our Windows jobs use -`-G "Visual Studio 18 2026" -A x64|Win32`, so just adding `mozilla-actions/sccache-action` -caches nothing. (The CLAUDE.md "use sccache-action / MSVC support" note predates hitting this.) - -**Upstream evidence (llama.cpp `b9682`, `.github/workflows/release.yml`).** ggml-org ships its -Windows artifacts with Ninja, not the VS generator: -- `windows-cpu` (the main CPU artifact, our analogue) — **Ninja Multi-Config** + clang toolchain - (`cmake/x64-windows-llvm.cmake`) + ccache. -- `windows-cuda` — **Ninja Multi-Config** + MSVC + ccache (proves Ninja Multi-Config + MSVC works - on the same llama.cpp + BoringSSL tree we build). -- `windows-sycl` — Ninja; `windows-hip` — Unix Makefiles; legacy `windows` + `windows-openvino` — - Visual Studio 17 2022. All jobs cache via `ggml-org/ccache-action@v1.2.21`. -- Important detail: it is **"Ninja Multi-Config"**, not plain Ninja — it keeps multi-config - semantics, so `cmake --build … --config Release` and our config-specific - `RUNTIME_OUTPUT_DIRECTORY_RELEASE` properties (`CMakeLists.txt:363-365`) behave exactly as they - do under the VS generator. The diff vs today is small: swap `-G`/`-A` for `-G "Ninja - Multi-Config"` + an MSVC env step (`vcvarsall` / `ilammy/msvc-dev-cmd`); `/MT` runtime and the - x64-vs-x86 arch gating are unchanged. - -**Chosen approach — do NOT switch the working build blindly.** Instead either (a) prove the Ninja -Multi-Config build in a **separate/experimental job first**, or preferably (b) **ship two Windows -artifacts in parallel — one Ninja-built, one MSVC(VS-generator)-built — so end users can test both** -and we can compare them before committing to one. That means the Windows native build runs **twice** -(once per generator) for a transition period; keep the MSVC/VS artifact as the trusted default and -add the Ninja one alongside until it's proven equivalent. Only after the Ninja artifact is validated -should we consider making it the sole Windows build (and retiring the second run). - -**Implementation notes for when this is picked up:** -- Cache backend: prefer **sccache + Depot WebDAV** (consistent with the other 8 jobs — one token, - shared cross-branch) over upstream's ccache (GitHub per-branch cache, a second cache system). - sccache supports MSVC `cl.exe`; Release config emits no debug info, so the `/Zi`→`/Z7` PDB caveat - doesn't apply. -- `build.bat` needs a Ninja path: pass `-G "Ninja Multi-Config"` + `-DCMAKE_BUILD_TYPE` is *not* - needed (multi-config keeps `--config Release`); add an sccache presence/probe guard mirroring - `build.sh` so a missing/crashing sccache falls back to a green uncached build. -- Files to touch: `.github/workflows/publish.yml` (the two `build-windows-*` jobs — add the MSVC env - step, the cache action, and the second artifact), `.github/build.bat` (generator + launcher wiring). -- Risk is bounded: a broken Ninja build shows up as a red Windows job, and publishing is gated behind - `publish_to_central`, so no broken artifact can reach Central/GitHub Releases. +- **NativeServer — wire upstream `server.cpp` routes to JNI (in progress; scaffold landed `dd264b2`).** + The upstream HTTP transport (`tools/server/server-http.cpp` + the cpp-httplib backend) is already + compiled into `libjllama`, and a `server.NativeServer` Java scaffold + `NativeServerSmokeTest` landed + in `dd264b2`. **Remaining:** wire the upstream `server.cpp` route table (the one upstream TU still + excluded from the build — it carries `main()` + route wiring) to JNI so the native HTTP server (and the + embedded WebUI) can be started/stopped from Java. This is the **native-transport alternative** to the + JDK-based `OpenAiCompatServer` (which is complete and the primary surface); value is shipping the full + llama.cpp server + WebUI in-process without a separate `llama-server` binary. JNI + C++ work. + +### Windows compiler cache (sccache) — dual build shipped (MSVC default + Ninja classifier) + +**Design decision (do not revisit without the owner): the MSVC / Visual Studio build is the +default JAR and is kept permanently — never retired.** The Ninja Multi-Config build is shipped +*alongside* it as the `ninja-windows` classifier JAR, never as a replacement. The loss of the +sccache cache on the MSVC build is accepted; the Ninja build exists so a cache-accelerated, +independently validated second Windows artifact is available for users to compare/adopt. + +**Why two builds.** The cache mechanism is the CMake *compiler launcher* +(`-DCMAKE_C_COMPILER_LAUNCHER=sccache`). **The Visual Studio generator ignores it entirely** +(only Ninja/Makefile generators honor it), so the MSVC jobs can never cache. The Ninja +Multi-Config generator *does* honor it (upstream llama.cpp `b9739` ships `windows-cuda` this way, +proving Ninja Multi-Config + MSVC works on the same tree). The two builds produce **different +`jllama.dll`s**, so they cannot coexist at the same resource path in one JAR — hence the classifier. + +**What shipped (this branch):** +- **4 Windows build jobs, all permanent:** `build-windows-x86_64`, `build-windows-x86` (MSVC, + default JAR) and `build-windows-x86_64-ninja`, `build-windows-x86-ninja` (Ninja + sccache/Depot). +- **Both tested end-to-end:** all four run the C++ unit tests (`ctest`); `test-java-windows-x86_64` + (MSVC) and the new `test-java-windows-x86_64-ninja` (Ninja) both load the DLL via JNI and run the + full model-backed Java suite. +- **`.github/build.bat`** — sccache probe guard (mirrors `build.sh`'s `sccache_can_wrap_compiler()`): + `USE_CACHE=true` + `sccache` on PATH + a trivial TU compiling through `sccache cl.exe` ⇒ + `-DCMAKE_{C,CXX}_COMPILER_LAUNCHER=sccache` + `sccache --show-stats`; else green uncached. Inert + for the MSVC jobs (they don't set `USE_CACHE`). +- **`pom.xml`** — `windows-ninja` profile → `ninja-windows` JAR from + `${project.build.outputDirectory}_windows_ninja` (mirrors the `cuda` / `opencl-android` profiles). +- **`publish.yml`** — the `package`, `publish-snapshot`, `publish-release` jobs download + `Windows-{x86_64,x86}-ninja` into `src/main/resources_windows_ninja/` and activate the + `windows-ninja` profile; the Ninja build + Java-test jobs are in the `package` `needs:` graph. +- Docs: `README.md` classifier table + `CLAUDE.md` "Windows Ninja artifact" section. + +**Verification — DONE (PR #248).** The Ninja jobs are green and cache-warm: `Build and Test +Windows … (Ninja … sccache, eval)` builds + `ctest` pass, and `Java Tests Windows 2025 x86_64 +(Ninja, eval)` loads the DLL via JNI and runs the full model-backed suite green (after the b9739 +arg-parse patch landed). `sccache --show-stats` confirms cache hits on the Ninja jobs. + +**Optional follow-up:** smoke-test that the *published* `ninja-windows` classifier JAR loads its DLL +on a clean Windows host. Publishing is gated behind `publish_to_central`, so a broken Windows job +blocks the release before any artifact reaches Central/GitHub Releases. + +**Reference notes:** +- Cache backend is **sccache + Depot WebDAV** (consistent with the other 8 jobs — one token, shared + cross-branch) rather than upstream's per-branch ccache. sccache supports MSVC `cl.exe`; the + Release config emits no debug info, so the `/Zi`→`/Z7` PDB caveat doesn't apply. +- It is **"Ninja Multi-Config"**, not plain Ninja — it keeps multi-config semantics, so + `cmake --build … --config Release` and the config-specific `RUNTIME_OUTPUT_DIRECTORY_RELEASE` + properties behave exactly as under the VS generator; `/MT` runtime and x64-vs-x86 gating unchanged. +- The arch (`x64`/`x86`) comes from `ilammy/msvc-dev-cmd@v1`, not a `-A` flag (Ninja takes no `-A`). + +### Known regression (b9739) — Windows JNI: `common_params_parse` ignores caller argv + +**Status: FIXED via local source patch (`patches/0001-win32-arg-parse-embed-guard.patch`).** Surfaced +while bringing PR #248 green (the b9739 build fixes let the Windows Java jobs run to completion and +exposed this). Applied through the generic `patches/` mechanism (see CLAUDE.md "Local llama.cpp source +patches"), so it covers every C++ build and re-applies on each clean build. + +**Note on the fix shape (count-guard → deterministic removal).** The first patch used fix option 1 +below — the count-guard (override only when the re-derived arg count equals `argc`). It fixed 21/25 +Windows Java tests, but **collided** on the 4 server-integration setups (`OpenAiServerRerank*`, +`OpenAiServerToolCalling*`, `MultimodalIntegrationTest`, `OpenAiCompatServerIntegrationTest`) whose +argv length happened to equal `java.exe`'s, so they kept failing with the same parse error. The patch +was changed to **fix option 2** (drop the override entirely for our build — a JNI library is never the +process, so the override is pure liability), which is deterministic. Still worth upstreaming as an +opt-out / `common_params_parse_argv` that preserves the standalone tools' UTF-8 fix, so the patch can +eventually be dropped; until then it must be re-verified on each llama.cpp bump (the applier fails loud +if it no longer applies). + +**Symptom.** On **Windows x86_64 only**, every Java test that loads a real model fails in +`LlamaModel.loadModel` (native) with `LlamaException: "Failed to parse model parameters"` +(25 errors in `Java Tests Windows 2025 x86_64`, both the VS *and* Ninja DLLs). macOS and Linux Java +tests pass. The argv we build is platform-neutral (`--model models/.gguf`, relative, forward +slashes — `TestConstants.MODEL_PATH`), so it is **not** the Windows-Ninja build, **not** our argv, +and **not** a path/escaping issue. + +**Root cause (upstream llama.cpp, new in b9739).** `jllama.cpp` (`load_model_impl`, ~line 606) builds +a CLI argv from `ModelParameters` and calls upstream +`common_params_parse(argc, argv, params, LLAMA_EXAMPLE_SERVER)`. In b9739, `common/arg.cpp`'s +`common_params_parse` gained a **Windows-only** prologue (arg.cpp:924-931): + +```cpp +bool common_params_parse(int argc, char ** argv, ...) { +#ifdef _WIN32 + auto utf8 = make_utf8_argv(); // = CommandLineToArgvW(GetCommandLineW()) + if (!utf8.ptrs.empty()) { // always non-empty under a JVM + argc = (int) utf8.buf.size(); + argv = utf8.ptrs.data(); // DISCARDS the caller-supplied argv + } +#endif + ... common_params_parse_ex(argc, argv, ctx_arg) ... +} +``` + +It unconditionally replaces the caller's argv with the host **process** command line +(`GetCommandLineW()`). For the standalone `llama-server.exe` this is correct (fixes UTF-8 CLI args). +For an **embedded/JNI** caller the process is **`java.exe`**, whose command line has no `--model`, so +`common_params_parse_ex` fails and `common_params_parse` returns `false` → our "Failed to parse model +parameters". `common_params_parse_ex` is `static`, so we cannot bypass the block by calling the inner +parser. Our JNI already passes correct UTF-8 argv (`GetStringUTFChars`), so the re-derivation is +unnecessary for us. **This is an upstream bug affecting every embedded Windows consumer of +`common_params_parse`.** + +**Fix options (history — option 2 chosen).** (1) guard the block by arg-count — *tried first, it +collided* (see the count-guard note above); (2) **remove the `_WIN32` override for our build — CHOSEN** +(deterministic; our JNI always passes correct UTF-8 argv); (3) file an upstream PR and wait. The patch +re-applies on every llama.cpp bump and the applier fails loud if it stops applying — it is part of the +upgrade checklist. Pre-existing on `main` since #247 (b9682→b9739); independent of the Windows-Ninja +classifier work. **Remaining open item: the upstream PR** (see "Upstream llama.cpp PR" below) so the +local patch can eventually be dropped. + +### SonarCloud "Security Rating on New Code" gate — PR #248 (open) + +The PR's **only** red is SonarCloud's "Security Rating on New Code" gate (every build/test job is +green; SonarCloud is **not** a merge-blocking build job). The findings are GitHub-Actions/Java +analyzer issues from the Maven scanner — **"C" is the rating *grade* (A–E), not the C language**; +there is no CFamily/C-C++ scan configured. Addressed: + +- **`clang-format.yml`** — `pip install` without `--only-binary :all:` can run a package's `setup.py`; + forced wheels-only (`84297e0`, block scalar so `:all:` doesn't break YAML). *If Sonar still flags it, + try the `--only-binary=:all:` equals form.* +- **`osv-scanner.yml` / `scorecard.yml`** — top-level `permissions: read-all` → `contents: read` + (`84297e0`); safe because every job in both files already declares its own exact permissions. +- **`publish.yml`** — workflow-level `permissions: contents: read` (Sonar wants it per-job); **owner + marked it Accept/"Won't fix" on the dashboard** rather than spreading perms across ~25 release jobs. + Alternative if ever desired: add `permissions: contents: read` to the ~19 read-only jobs (the 5 + publish/report jobs already declare `contents: write`) and drop the top-level block. +- **`PairTest.java`** — 3 Critical *Reliability* bugs (`assertNotNull` on the primitive `hashCode()`) + replaced with a determinism check (`9f0d377`). Reliability rating, **not** the Security gate. + +**Still open:** the gate was still red as of `9f0d377`. SonarCloud's issues API is auth-gated (403 from +CI), so the exact remaining new-code Vulnerability must be read off the dashboard. Resolve the last +finding, accept it on the dashboard, or merge on the green build/test checks. + +### License Compliance (FOSSA-style dependency-license gate) — PR #248 (open) + +Separate from the FSFE **REUSE** check (which is green — `reuse lint` reports 266/266 files compliant) +and from SonarCloud: the PR's combined commit status shows a **"License Compliance" check failing with +"17 issues found"** (an error-state commit status posted by a license-scanner GitHub App, not a +workflow in `.github/workflows/`). It contributes to the `mergeable_state: blocked` on #248. + +- **Almost certainly pre-existing**, not introduced by this PR: #248 changes **no dependencies** (the + `pom.xml` edit only adds the `windows-ninja` build profile), so the 17 are dependency-license policy + findings already present on `main` (e.g. GPL-2.0 carried by the llama.cpp sources). +- **Not yet inspected** — the scanner's dashboard/host is outside this sandbox's egress allowlist, same + as `sonarcloud.io`. To triage: open the check's details link from the PR (or allowlist the host), read + the 17 findings, then accept policy-OK licenses on the dashboard or adjust the policy. Confirm whether + it is a *required* status (if so it blocks merge; if advisory it does not). + +### Upstream llama.cpp PR — drop the local Windows arg-parse patch (open) + +`patches/0001-win32-arg-parse-embed-guard.patch` is a **local** fix re-applied on every build. To drop +it, PR upstream (against #24779): add a `common_params_parse_argv` companion (or a +`common_params_parse` opt-out flag) that trusts the caller's argv — preserving the standalone tools' +UTF-8 fix while letting embedders (JNI, and any FFI binding) pass their own argv. Ship with the +standalone-safe repro (a plain exe that passes a synthetic argv and shows it gets discarded on Windows +because `GetCommandLineW()` returns the host process line). Once merged and the pin is bumped past it, +delete the patch. + +### Branch protection — aarch64 job renamed (open, owner action) + +The native aarch64 switch renamed the check **`Cross-Compile Linux aarch64 (LTS)` → `Build and Test +Linux aarch64`**. If a required status check pinned the old name, repoint it or it will sit pending +forever. ### llama.cpp upstream feature exposure (queued, deferred by policy) @@ -203,6 +325,28 @@ These are JNI plumbing items for upstream API additions. Policy: add only after ## Done (kept for history) +### b9739 upgrade + PR #248 (Windows Ninja, native aarch64, patches mechanism) + +- **llama.cpp b9682 → b9739** (#247, merged) + build fixes: `server-schema.cpp` added to the + `jllama_test` sources (b9739 link fix, `38be6db`); `test_server.cpp` `ParamsFromJsonCmpl` + expectations updated to b9739 schema behavior (`aaba886`). +- **Windows Ninja artifact** — `ninja-windows` classifier JAR built with Ninja Multi-Config + sccache, + shipped alongside the permanent MSVC default; both build + Java-test jobs green (`e113ed3`, + `48f0863`). (See the open section above for the design rationale; verification is done.) +- **Linux aarch64 → native `ubuntu-24.04-arm` build** (`ed9ecbb`). The dockcross `linux-arm64-lts` + image (GCC 8.5 / glibc 2.17) could no longer compile b9739's C++17 CTAD-in-`new`; now builds natively + with GCC 14 (mirroring upstream), runs `ctest` on real ARM (446 tests green), and warms sccache + (99.66% hits). Trade-off: glibc floor 2.17 → ~2.39 (same envelope as upstream's ARM binaries); + documented in the README classifier table. `build.sh` sccache auto-fetch generalized to aarch64. +- **Generic `patches/` mechanism** — drop `*.patch`/`*.diff` in repo-root `patches/`, applied to the + FetchContent'd llama.cpp source by `cmake/apply-llama-patches.cmake` via the llama.cpp + `PATCH_COMMAND` (cross-platform, idempotent, fail-loud). Covers every C++ build from one place. + First patch fixes the Windows JNI arg-parse regression (`1d875b1` → deterministic form `f651b53`). + REUSE annotated via `patches/**` glob (`0cffac1`). +- **CUDA sccache verified** — the `manylinux_2_28 (CUDA)` job caches all gcc C/C++ TUs (247/248 hits, + 99.60%); the nvcc `.cu` kernels remain uncached (sccache limitation), and `CUDA_FAST_BUILD` keeps + PR/validation runs single-arch. (Doc/observation; no code change.) + ### Layered package restructure (flat root package → layered hierarchy) The flat `net.ladenthin.llama` root package was split (via `git mv`, history diff --git a/cmake/apply-llama-patches.cmake b/cmake/apply-llama-patches.cmake new file mode 100644 index 00000000..6c8c1293 --- /dev/null +++ b/cmake/apply-llama-patches.cmake @@ -0,0 +1,73 @@ +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT +# +# apply-llama-patches.cmake — applies every patch in the repo-root `patches/` directory to the +# llama.cpp source tree fetched by FetchContent. Wired as the llama.cpp `PATCH_COMMAND` in the +# top-level CMakeLists.txt, so it runs for EVERY C++ build (all CI jobs + local) from one place, +# rather than per-build-step. +# +# Design: +# * Cross-platform: invoked via `cmake -P`, so it behaves identically on Linux, macOS and +# Windows (the dockcross/native/MSVC jobs all call the same code path). +# * Every `patches/*.patch` and `patches/*.diff` is applied, sorted by filename (so a numeric +# prefix like 0001-, 0002- defines a deterministic order). +# * Idempotent: `git apply --reverse --check` detects an already-applied patch and skips it, so +# a CMake reconfigure over an already-patched source tree does not fail. +# * Fail-loud: a patch that no longer applies (e.g. after a llama.cpp version bump shifts the +# context) aborts the configure with a clear message, so a stale patch can never be silently +# dropped from a release build. +# +# Invoked as: +# cmake -DPATCH_DIR=/patches -DLLAMA_SRC= -P cmake/apply-llama-patches.cmake + +if(NOT DEFINED PATCH_DIR OR NOT DEFINED LLAMA_SRC) + message(FATAL_ERROR "apply-llama-patches: both PATCH_DIR and LLAMA_SRC must be defined") +endif() + +find_program(GIT_EXECUTABLE NAMES git) +if(NOT GIT_EXECUTABLE) + message(FATAL_ERROR "apply-llama-patches: 'git' not found on PATH (required to apply patches)") +endif() + +file(GLOB patch_files "${PATCH_DIR}/*.patch" "${PATCH_DIR}/*.diff") +list(SORT patch_files) + +if(NOT patch_files) + message(STATUS "apply-llama-patches: no patches in ${PATCH_DIR} (nothing to apply)") + return() +endif() + +foreach(patch IN LISTS patch_files) + get_filename_component(patch_name "${patch}" NAME) + + # Already applied? A successful reverse-apply check means the change is present already. + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${LLAMA_SRC}" apply --reverse --check "${patch}" + RESULT_VARIABLE reverse_rc + OUTPUT_QUIET ERROR_QUIET) + if(reverse_rc EQUAL 0) + message(STATUS "apply-llama-patches: ${patch_name} already applied — skipping") + continue() + endif() + + # Not applied yet — confirm it applies cleanly before touching the tree. + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${LLAMA_SRC}" apply --check "${patch}" + RESULT_VARIABLE check_rc + OUTPUT_QUIET ERROR_QUIET) + if(NOT check_rc EQUAL 0) + message(FATAL_ERROR + "apply-llama-patches: ${patch_name} does not apply cleanly to ${LLAMA_SRC}.\n" + " A llama.cpp version bump probably shifted the patched code — refresh the patch " + "against the new source and recommit it.") + endif() + + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${LLAMA_SRC}" apply "${patch}" + RESULT_VARIABLE apply_rc) + if(NOT apply_rc EQUAL 0) + message(FATAL_ERROR "apply-llama-patches: failed to apply ${patch_name}") + endif() + message(STATUS "apply-llama-patches: applied ${patch_name}") +endforeach() diff --git a/patches/0001-win32-arg-parse-embed-guard.patch b/patches/0001-win32-arg-parse-embed-guard.patch new file mode 100644 index 00000000..3ee802a7 --- /dev/null +++ b/patches/0001-win32-arg-parse-embed-guard.patch @@ -0,0 +1,25 @@ +diff --git a/common/arg.cpp b/common/arg.cpp +--- a/common/arg.cpp ++++ b/common/arg.cpp +@@ -924,10 +924,17 @@ bool common_params_parse(int argc, char ** argv, common_params & params, llama_e + bool common_params_parse(int argc, char ** argv, common_params & params, llama_example ex, void(*print_usage)(int, char **)) { + #ifdef _WIN32 + auto utf8 = make_utf8_argv(); +- if (!utf8.ptrs.empty()) { +- argc = static_cast(utf8.buf.size()); +- argv = utf8.ptrs.data(); +- } ++ // java-llama.cpp patch (PR #248): upstream (llama.cpp #24779) replaced the caller's argv with ++ // the process command line (GetCommandLineW) here to recover UTF-8 args for the standalone ++ // llama-* tools. libjllama is a JNI library, never the process: it already passes correct ++ // UTF-8 argv (GetStringUTFChars), and adopting GetCommandLineW discarded it -> common_params_parse ++ // parsed java.exe's command line and failed with "Failed to parse model parameters". We keep the ++ // make_utf8_argv() call (so it stays referenced -> -Wunused-function-clean) but do NOT adopt its ++ // result, so the caller's already-UTF-8 argv is always used. This is deterministic: a count-guard ++ // (only override when the re-derived arg count equals argc) collided on the server-integration ++ // tests whose argv length happened to equal java.exe's, so they kept failing. The upstream PR ++ // can instead expose an opt-out / a common_params_parse_argv that preserves the standalone fix. ++ (void) utf8; + #endif + + auto ctx_arg = common_params_parser_init(params, ex, print_usage); diff --git a/pom.xml b/pom.xml index 3dd10543..c3743c03 100644 --- a/pom.xml +++ b/pom.xml @@ -912,6 +912,95 @@ SPDX-License-Identifier: MIT + + + windows-ninja + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + windows-ninja + compile + + compile + + + + + module-info.java + + + -h + src/main/cpp + + + ${project.build.outputDirectory}_windows_ninja + + + + + + maven-resources-plugin + + + + copy-resources-windows-ninja + process-classes + + copy-resources + + + + ${project.build.outputDirectory}_windows_ninja + + + + ${basedir}/src/main/resources_windows_ninja/ + + **/*.* + + + + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + + windows-ninja + package + + jar + + + ninja-windows + + ${project.build.outputDirectory}_windows_ninja + + + + + + + + vmlens diff --git a/src/test/cpp/test_server.cpp b/src/test/cpp/test_server.cpp index 50179efd..a85dba4c 100644 --- a/src/test/cpp/test_server.cpp +++ b/src/test/cpp/test_server.cpp @@ -1710,10 +1710,10 @@ TEST(CmplFinalChatStream, IncludeUsageTrue_TrailingChunkHasEmptyChoicesAndUsage) // - repeat_last_n=-1 is expanded to n_ctx_slot // - dry_penalty_last_n=-1 is expanded to n_ctx_slot // - dry_base < 1.0 is reset to default -// - n_discard negative is clamped to 0 -// - empty dry_sequence_breakers throws std::runtime_error -// - lora field not an array throws std::runtime_error -// - repeat_last_n < -1 throws std::runtime_error +// - n_discard negative throws std::invalid_argument (b9739: range-checked, no longer clamped) +// - empty dry_sequence_breakers throws std::invalid_argument +// - lora field not an array throws std::invalid_argument +// - repeat_last_n < -1 throws std::invalid_argument // ============================================================ namespace { @@ -1749,21 +1749,23 @@ TEST(ParamsFromJsonCmpl, DryBase_BelowOne_ResetToDefault) { EXPECT_FLOAT_EQ(p.sampling.dry_base, defaults.sampling.dry_base); } -TEST(ParamsFromJsonCmpl, NDiscard_Negative_ClampedToZero) { - const auto p = parse_params({{"n_discard", -5}}); - EXPECT_EQ(p.n_discard, 0); +// b9739: negative n_discard is range-checked (0 <= value <= INT32_MAX) and now throws +// instead of being silently clamped to 0. The schema wraps every field-validation failure +// in std::invalid_argument ("Field '': ...", server-schema.cpp). +TEST(ParamsFromJsonCmpl, NDiscard_Negative_Throws) { + EXPECT_THROW(parse_params({{"n_discard", -5}}), std::invalid_argument); } TEST(ParamsFromJsonCmpl, EmptyDrySequenceBreakers_Throws) { - EXPECT_THROW(parse_params({{"dry_sequence_breakers", json::array()}}), std::runtime_error); + EXPECT_THROW(parse_params({{"dry_sequence_breakers", json::array()}}), std::invalid_argument); } TEST(ParamsFromJsonCmpl, LoraNotArray_Throws) { - EXPECT_THROW(parse_params({{"lora", "not-an-array"}}), std::runtime_error); + EXPECT_THROW(parse_params({{"lora", "not-an-array"}}), std::invalid_argument); } TEST(ParamsFromJsonCmpl, RepeatLastN_BelowMinusOne_Throws) { - EXPECT_THROW(parse_params({{"repeat_last_n", -2}}), std::runtime_error); + EXPECT_THROW(parse_params({{"repeat_last_n", -2}}), std::invalid_argument); } TEST(ParamsFromJsonCmpl, StreamOptions_IncludeUsage_Parsed) { diff --git a/src/test/java/net/ladenthin/llama/value/PairTest.java b/src/test/java/net/ladenthin/llama/value/PairTest.java index fdd0a66e..af0c8b62 100644 --- a/src/test/java/net/ladenthin/llama/value/PairTest.java +++ b/src/test/java/net/ladenthin/llama/value/PairTest.java @@ -94,17 +94,19 @@ public void testHashCodeSamePair() { public void testHashCodeDifferentPairs() { Pair pair1 = new Pair<>("key1", 123); Pair pair2 = new Pair<>("key2", 456); - // Different pairs may have different hash codes (not guaranteed, but likely) - // We mostly check that hashCode() doesn't throw - assertNotNull(pair1.hashCode()); - assertNotNull(pair2.hashCode()); + // hashCode() must not throw and must be deterministic (consistent across calls). + int hash1 = pair1.hashCode(); + int hash2 = pair2.hashCode(); + assertEquals(hash1, pair1.hashCode()); + assertEquals(hash2, pair2.hashCode()); } @Test public void testHashCodeWithNull() { Pair pair = new Pair<>(null, null); - // Should not throw when hashing null values - assertNotNull(pair.hashCode()); + // hashCode() must not throw when hashing null fields, and stays deterministic. + int hash = pair.hashCode(); + assertEquals(hash, pair.hashCode()); } @Test