ci: fix 3rd-party Werror, android omnivoice-dac-parity link, vulkan test-gguf#17
Merged
Merged
Conversation
…se1d The output_pad arg affects T_out (computed by caller) but doesn't change the per-element accumulation, so the body legitimately doesn't reference it. Keep the parameter as part of the PyTorch convention and explicit-void the unused-param warning that fails -Werror builds on 3rd-party / android / vulkan CI lanes.
The parity harness calls ggml_backend_cpu_init() and ggml_backend_cpu_set_n_threads() directly. On Android NDK cross-compile the ggml-cpu CMake target is not defined, so the executable was being built with an unresolved link to ggml-cpu, producing: ld.lld: error: undefined symbol: ggml_backend_cpu_init ld.lld: error: undefined symbol: ggml_backend_cpu_set_n_threads Move the entire add_executable + add_test under `if(TARGET ggml-cpu)` so the test is silently skipped on backends without a CPU target, matching how tools/kokoro/CMakeLists.txt already gates kokoro_lib's ggml-cpu link.
tools/omnivoice/CMakeLists.txt bumps GGML_MAX_NAME to 128 PUBLIC on ggml + llama so audio tokenizer tensor names load correctly. That propagates to test-gguf.cpp, where the hand-crafted long-name test case had a hardcoded ~88-char string. Under GGML_MAX_NAME=128 the string was shorter than the limit, the BAD_NAME_SIZE branch failed to actually generate an oversize name, and the in-test sanity assert fired: GGML_ASSERT(name.length() >= GGML_MAX_NAME) failed Append the long suffix in a loop until name strictly exceeds GGML_MAX_NAME, so the oversize-name failure path is always exercised regardless of the GGML_MAX_NAME value the test is compiled with.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three CI fixes for failures on main HEAD (22582b2):
CI (3rd-party)—tools/kokoro/include/kokoro-layers.hhad twooutput_padparameters that go unused in the per-element body (the caller uses them to sizeT_out).-Werror=unused-parameterfailed the build on ubuntu-24-llguidance. Cast to(void)and document why.CI (android)—omnivoice-dac-paritywas being added unconditionally and calledggml_backend_cpu_init/ggml_backend_cpu_set_n_threads, but android NDK cross-compile doesn't define aggml-cpuCMake target, producingundefined symbollink errors. Gate the wholeadd_executable+add_testonif(TARGET ggml-cpu)so backends without CPU silently skip it. Matches the patterntools/kokoro/CMakeLists.txtalready uses forkokoro_lib.CI (vulkan)—tests/test-gguf.cpphad a hard-coded ~88-char long-name string for theHANDCRAFTED_TENSORS_BAD_NAME_SIZEcase. omnivoice's CMakeLists bumpsGGML_MAX_NAMEto 128 PUBLIC on the ggml/llama targets, so the test name was no longer longer than the limit and the in-test sanityGGML_ASSERT(name.length() >= GGML_MAX_NAME)fired. Pad the suffix in a loop so the oversize-name path is always exercised under anyGGML_MAX_NAME.Test plan