Skip to content

fix: add -DGGML_NATIVE=OFF to macos-15-no-metal target and fix artifact name collision#6

Merged
bernardladenthin merged 1 commit into
masterfrom
claude/fix-macos-build-failure-ri4dX
Mar 22, 2026
Merged

fix: add -DGGML_NATIVE=OFF to macos-15-no-metal target and fix artifact name collision#6
bernardladenthin merged 1 commit into
masterfrom
claude/fix-macos-build-failure-ri4dX

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

The first macos-15 matrix entry (-DLLAMA_METAL=OFF) was missing -DGGML_NATIVE=OFF, causing the vmmlaq_s32/i8mm always_inline conflict on Apple Clang 16 (macOS 15). Also added a unique 'name' field to each matrix target to avoid artifact upload collisions when two macos-15 jobs both produced 'error-log-macos-15'.

https://claude.ai/code/session_01SZrxDKcGVoDkqrK9cdHp4L

…ct name collision

The first macos-15 matrix entry (-DLLAMA_METAL=OFF) was missing -DGGML_NATIVE=OFF,
causing the vmmlaq_s32/i8mm always_inline conflict on Apple Clang 16 (macOS 15).
Also added a unique 'name' field to each matrix target to avoid artifact upload
collisions when two macos-15 jobs both produced 'error-log-macos-15'.

https://claude.ai/code/session_01SZrxDKcGVoDkqrK9cdHp4L
@bernardladenthin
bernardladenthin merged commit 61a2812 into master Mar 22, 2026
5 checks passed
@bernardladenthin
bernardladenthin deleted the claude/fix-macos-build-failure-ri4dX branch March 23, 2026 10:05
vaiju1981 pushed a commit to vaiju1981/java-llama.cpp that referenced this pull request Jun 25, 2026
Re-checked all six findings raised after PR bernardladenthin#268 against the pinned upstream
tts.cpp @ b9739. Two were false alarms; four were genuine.

Verified upstream-faithful (NOT divergences) — added provenance comments only:
- bernardladenthin#1 llama_model_n_embd_out: upstream tts.cpp:1042 uses the exact same call.
  Comment now notes it reads the vocoder OUTPUT embedding width, matching upstream.
- bernardladenthin#2 0.25 s silence lead-in: upstream tts.cpp:1077-1080 zeroes the first 0.25 s
  identically. Comment notes it mirrors upstream and that our `i < audio.size()`
  bound is an added safety guard over upstream's fixed 24000/4.

Genuine findings fixed:
- bernardladenthin#3 heavy include in shared header: tts_upstream.h now includes
  <nlohmann/json_fwd.hpp> (forward-declares ordered_json) instead of the full
  <nlohmann/json.hpp>, and drops the json default argument. The single caller,
  tts_engine.cpp, includes the full json and passes an explicit empty object.
  Future includers of the shared interface no longer pull in ~25k lines of json.
- bernardladenthin#4 unasserted duplicate enum: generate-tts-upstream.cmake now captures the
  upstream `outetts_version` enum body and pins its enumerators + order against
  the hand-written copy in tts_upstream.h, so a reorder/rename fails the configure
  instead of silently assigning different integer values across the two TUs.
- bernardladenthin#5 prompt_add overload coverage: the bare `void prompt_add(` prefix de-statics
  all three upstream overloads but only proved >=1 existed. The generator now
  pins (whitespace-tolerant) both overloads the header declares, turning a future
  cryptic link error into a clear configure-time failure.
- bernardladenthin#6 weak WAV assertion: TtsIntegrationTest now parses the RIFF/WAVE header
  (PCM format, mono, 24 kHz, 16-bit), checks chunk-size self-consistency, and
  scans the PCM payload for non-zero samples — so a near-empty or all-silent
  result no longer passes the way `length > 44` did.

Generator regexes validated against the real b9739 source via `cmake -P`
(including a negative drift control); Java test compiles; clang-format 22.1.5
and Spotless both clean. Native build not run here (sandbox proxy blocks the
dependency FetchContent clones) — exercised in CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QTQ8mBM9tyKkHbXVBpGwET
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants