Skip to content

cmake : fall back to empty UI on stale assets when provisioning is disabled#26104

Draft
wuisabel-gif wants to merge 2 commits into
ggml-org:masterfrom
wuisabel-gif:fix/stale-ui-assets
Draft

cmake : fall back to empty UI on stale assets when provisioning is disabled#26104
wuisabel-gif wants to merge 2 commits into
ggml-org:masterfrom
wuisabel-gif:fix/stale-ui-assets

Conversation

@wuisabel-gif

Copy link
Copy Markdown

Overview

With LLAMA_BUILD_UI=OFF and LLAMA_USE_PREBUILT_UI=OFF, a leftover tools/ui/dist (gitignored, so it silently survives git pull) still reached llama-ui-embed, which hard-fails on an incomplete asset set and broke the whole llama-server build.

This PR makes the embed step, when both provisioning options are disabled, warn and fall back to the empty-UI embed (the same path taken when no assets exist) instead of failing the build. The fatal error is preserved whenever npm building or prebuilt downloads are enabled, and a valid local dist is still embedded as before.

Also adds a CTest (tests/test-ui-assets.cmake) that runs ui-assets.cmake in script mode against temporary directories and checks the four flag/asset combinations: incomplete assets with provisioning disabled falls back and warns, incomplete assets with either provisioning option enabled still fails, and complete local assets still embed. The required asset list is derived from llama-ui-embed's own error output so the test doesn't go stale when the list changes.

Fixes #25443

Additional information

The fix is based on the approach proposed by the reporter in #25443. Reproduced on current master before the change: with both flags OFF, a dist containing only a stale index.html fails the build with UI: llama-ui-embed failed (1); after the change it warns and builds successfully. Verified a full llama-server build with the fix applied.

Requirements

…bled

A leftover tools/ui/dist with LLAMA_BUILD_UI=OFF and
LLAMA_USE_PREBUILT_UI=OFF reached llama-ui-embed, which hard-fails
on incomplete assets and broke the llama-server build. Now warn and
fall back to the empty-UI embed in that case. Fatal error preserved
when provisioning is enabled; valid local assets embed as before.

Add a CTest that runs ui-assets.cmake in script mode and checks the
four flag/asset combinations.

Fixes ggml-org#25443
@notwitcheer

Copy link
Copy Markdown

Verified this against the reproducer from #25443. It fixes both of the paths that issue described.

Environment: Ubuntu Server 26.04 LTS, gcc 15.2.0, cmake 4.2.3, no npm/node on the box. Configured CPU-only (-DGGML_CUDA=OFF) since this is a cmake-path bug. -DLLAMA_BUILD_UI=OFF -DLLAMA_USE_PREBUILT_UI=OFF throughout. master at fb92d8f, this PR at 5978cf5.

case master fb92d8f this PR 5978cf5
clean tree builds builds
stale tools/ui/dist build fails builds, warns
stale build/tools/ui/dist build fails builds, warns

On master both failures land on ui-assets.cmake:291, reached from two different callers: :300 for the pre-built source dist, and :367 for the build-directory fallback. Both go through emit_files, so the single guard covers both. That was the part I wanted to confirm rather than assume, because the issue reported the two paths separately and a fix at either call site alone would have left one open.

On the PR the same two cases produce the warning at :292 from :310 and the build completes to [100%] Built target llama-server.

ctest -R test-ui-assets passes in 0.02 s. Probing llama-ui-embed for the required-asset list rather than hardcoding it is better than the fix I sketched in the issue. A growing required set is what leaves people with a stale dist, so a test pinning today's list would need updating on exactly the commits that matter.

One small thing, not a blocker. The warning says the build is proceeding without an embedded UI, but not what to do about it. tools/ui/dist is gitignored, so someone who hits this has no particular reason to suspect the directory exists, and the old failure at least printed a path. A closing clause along the lines of "remove ${dist_dir} to silence this" would cover it.

@wuisabel-gif

Copy link
Copy Markdown
Author

@notwitcheer Thanks for testing both paths. Good catch on the warning. c525974 adds the flag names and a note to remove the directory. Since the directory is gitignored, nothing else would ever surface it and the warning is the only place a user learns it exists.

@notwitcheer

Copy link
Copy Markdown

Confirmed on c525974. Both callers emit the flags and the removal hint, each naming its own directory.

Same environment as the previous run: Ubuntu Server 26.04 LTS, gcc 15.2.0, cmake 4.2.3, no npm/node on the box, -DGGML_CUDA=OFF, -DLLAMA_BUILD_UI=OFF -DLLAMA_USE_PREBUILT_UI=OFF.

stale dist warning raised from directory named result
tools/ui/dist ui-assets.cmake:310 .../tools/ui/dist Built target llama-server
build/tools/ui/dist ui-assets.cmake:377 .../build/tools/ui/dist Built target llama-server

ctest -R test-ui-assets passes in 0.02 s.

I ran the two callers as separate cases rather than relying on both reaching the shared guard in emit_files, since #25443 reported the paths separately. The ${dist_dir} interpolation resolves to the right directory in each.

One practical note: cmake wraps the message, so LLAMA_BUILD_UI=OFF, and LLAMA_USE_PREBUILT_UI=OFF) land on separate lines in the build log. Anything grepping for the full flag pair will miss it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server/ui testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compile bug: stale tools/ui/dist fails the whole llama-server build even with LLAMA_BUILD_UI=OFF and LLAMA_USE_PREBUILT_UI=OFF

2 participants