fork: surface HYDRA_BUILD_TYPE in --version (hydra_vortex #349)#10
Conversation
PR #5+#7 + 6e397fb wrapped in :f518cff16 hangs on RTX in the post-init phase on a -DBUILD_SHARED_LIBS=OFF build (see hydra_vortex#346). The fix is to use -DBUILD_SHARED_LIBS=ON; the build-type difference was invisible because both md5s looked like 'b9545-f518cff16'. Surface the build-type label in the binary so a fresh -DBUILD_SHARED_LIBS=OFF rebuild is self-identifying: ``` $ llama-server --version version: 9549 (f518cff) [shared] # OK on RTX version: 9549 (f518cff) [static] # will hang in post-init ``` Auto-derives from BUILD_SHARED_LIBS unless -DLLAMA_HYDRA_BUILD_TYPE= is set explicitly (used by build_sm60_v2 / build_sm120_v3 with [shared] label already wired through, and by abi-benchmark/air-gap bundles with custom labels). CI smoke test (in parent repo scripts/ci/check-build-type.sh): `BUILD_DIR/bin/llama-engine --version 2>&1 | grep -q '\\[shared\\]' || fail 'FATAL: static build, RTX will hang. See ggml-org#346.'` Tracks ddvnguyen/hydra_vortex#349. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l-org#349 review round 2) PR #10 round-2 review found a real 🟡 P2: The naive impl caches `LLAMA_HYDRA_BUILD_TYPE` (the *derived* label) in the CMake cache. After a first configure of a fresh build dir with `-DBUILD_SHARED_LIBS=ON`, the cache has `LLAMA_HYDRA_BUILD_TYPE=shared`. On a *reconfigure* of the same build dir with `-DBUILD_SHARED_LIBS=OFF`: if(DEFINED LLAMA_HYDRA_BUILD_TYPE) # TRUE — comes from cache set(_hydra_build_type "${LLAMA_HYDRA_BUILD_TYPE}") elseif(BUILD_SHARED_LIBS) # never reached ... The first branch takes the stale cached value. The new build is *actually* static but the [shared] label survives, sails through the ggml-org#351 check-build-type gate, and the hang that ggml-org#349 exists to prevent is back in production. Fix: cache the *override* (only set by the user), not the derived label. The override is an empty string by default. The derived label is a normal (non-cache) variable recomputed every configure from BUILD_SHARED_LIBS (or the override). configure_file() picks up the freshly-derived label at configure time. Tested: -D override=experimental → "experimental" (persists) -D override=experimental, no flag → "experimental" (persists) -D override=experimental, -DBUILD_SHARED_LIBS=OFF → "experimental" (override wins) cmake --fresh → "shared" (override cleared, re-derives) -DBUILD_SHARED_LIBS=ON, then -DBUILD_SHARED_LIBS=OFF → re-derives to "static" Also: unify the label style in llama_print_build_info(). Was `type=shared`, now `[shared]` to match --version and llama_build_info() so all sinks of the label are grep-friendly with the same regex (\\[\[^\\]\\]\\]). The 🟢 P3 about llama_build_info() format change (b<N>-<sha> → b<N>-<sha> [shared]) is kept as-is. This is a private fork; the user-facing parser (--version) still works. If a downstream consumer ever breaks, the fix is to read the new field separately rather than parsing the historical format. Tracks ddvnguyen/hydra_vortex#349. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ddvnguyen
left a comment
There was a problem hiding this comment.
Review — surface build-type in --version (ggml-org#349)
Clean, well-scoped fork change, and the round-2 cache fix is the right one. A couple of confirmations, no blockers.
The cache fix is correct
Caching the override (LLAMA_HYDRA_BUILD_TYPE_OVERRIDE, empty by default) and recomputing the derived LLAMA_HYDRA_BUILD_TYPE as a normal variable each configure is the correct way to avoid the stale-[shared]-on-reconfigure bug. Good.
One thing to confirm (the fix depends on it)
The whole cache fix rests on common/build-info.cpp.in being re-substituted via configure_file() at configure time, where LLAMA_HYDRA_BUILD_TYPE is in scope. Upstream llama.cpp generates build-info two different ways depending on version — a configure-time configure_file, or a build-time custom command running common/cmake/build-info-gen-cpp.cmake, which only passes LLAMA_BUILD_NUMBER/COMMIT/COMPILER/TARGET. If this fork uses the build-time-script path, @LLAMA_HYDRA_BUILD_TYPE@ would expand to empty (or fail to update on reconfigure) unless the script is also passed the new var. Your test plan shows [shared]/[static]/[experimental] all reporting correctly on fresh dirs, so the substitution works — but please confirm it also updates on an in-place reconfigure of the same build dir (cmake -B X -DBUILD_SHARED_LIBS=OFF over a dir previously built =ON), since that reconfigure case is exactly what round 2 was about. If build-info is build-time-generated, the gate could still see a stale label.
Minor
--versionformat changes to... (sha) [type]. You've flagged downstream parsers; the one in-tree consumer (check-build-type.sh) greps the bracket form, and you kept the bracket form consistent acrossllama_build_info()/llama_print_build_info()/ the TRACE log — good, all sinks share one regex.
LGTM pending the in-place-reconfigure confirmation. Merge-order note: parent ggml-org#351's deploy gate depends on this landing on hydra-fork first (see my review there).
Generated by Claude Code
ddvnguyen
left a comment
There was a problem hiding this comment.
Re-review — open item resolved, LGTM ✅ (comment; GitHub blocks self-approval)
No new commits since my first pass (head 746e585, 2 commits). My one outstanding question was whether the round-2 cache fix actually takes effect on an in-place reconfigure — i.e. whether build-info.cpp.in is re-substituted at configure time or baked by a build-time generator script that wouldn't see LLAMA_HYDRA_BUILD_TYPE. I checked the fork's common/CMakeLists.txt directly:
set(TEMPLATE_FILE "${CMAKE_CURRENT_SOURCE_DIR}/build-info.cpp.in")
set(OUTPUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/build-info.cpp")
configure_file(${TEMPLATE_FILE} ${OUTPUT_FILE}) # line 39 — configure-timeIt's a configure-time configure_file(), not a build-time script. So the chain is sound by construction:
LLAMA_HYDRA_BUILD_TYPEis a normal (non-cache) variable set in the top-levelCMakeLists.txtbeforeadd_subdirectory(common), so it's in scope here and inherited into the subdirectory.- Every reconfigure re-derives it from the
BUILD_SHARED_LIBScache value (or the override) and re-runsconfigure_file, regeneratingbuild-info.cppwith the fresh label. - Caching the override (empty by default) rather than the derived label is exactly what prevents a stale
[shared]surviving a=ON → =OFFreconfigure.
That matches the author's documented test (-DBUILD_SHARED_LIBS=ON then =OFF → re-derives to static). The cache fix is correct, and the #351 gate it feeds can't be silently bypassed by a reused build dir.
The other earlier points are all fine: the [bracket] label form is consistent across --version / llama_build_info() / llama_print_build_info() / the TRACE log (one regex matches all sinks), and the --version format change is documented for downstream parsers (check-build-type.sh greps the bracket).
LGTM for hydra-fork. (Parent ggml-org#349/ggml-org#351's deploy gate depends on this landing first — and note ggml-org#354 already merged #13 onto hydra-fork, so when this merges, double-check the parent submodule bump for ggml-org#351 lands a hydra-fork commit that contains both.)
Generated by Claude Code
Per docs/workflow/08-llama-fork.md, the submodule bump in the parent PR happens AFTER the fork PR is merged to hydra-fork. Fork PR ddvnguyen/llama.cpp#10 (build-type label) merged to hydra-fork at 84f5fccd8, so we bump the pointer to that commit. The pinned SHA (84f5fccd8) is reachable on hydra-fork (the submodule's tracked branch). Verification against the new submodule: -DBUILD_SHARED_LIBS=ON → [shared] → gate OK -DBUILD_SHARED_LIBS=OFF → [static] → gate FATAL (correctly rejected) (re-derivation on in-place reconfigure also works per #10 round-2 review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per docs/workflow/08-llama-fork.md, the submodule bump in the parent PR happens AFTER the fork PR is merged to hydra-fork. Fork PR ddvnguyen/llama.cpp#10 (build-type label) merged to hydra-fork at 84f5fccd8, so we bump the pointer to that commit. The pinned SHA (84f5fccd8) is reachable on hydra-fork (the submodule's tracked branch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tracks ddvnguyen/hydra_vortex#349.
What
The static llama-server build (default per
DevelopmentRunBook.md) hangs in the post-init phase on RTX sm_120 — see ddvnguyen/hydra_vortex#346. The fix is to use-DBUILD_SHARED_LIBS=ON, but the difference was invisible in CI: two builds of the same source with differentBUILD_SHARED_LIBSproduce different md5s but the same--versionoutput.This surfaces the build type in
llama-server --version:Auto-derives from
BUILD_SHARED_LIBSunless-DLLAMA_HYDRA_BUILD_TYPE=…is set explicitly (used by abi-benchmark/air-gap bundles with custom labels).Files changed
CMakeLists.txt: auto-deriveLLAMA_HYDRA_BUILD_TYPEfromBUILD_SHARED_LIBS(overridable via-DLLAMA_HYDRA_BUILD_TYPE=…)common/CMakeLists.txt: no change (build-info template is unchanged)common/build-info.cpp.in: addLLAMA_HYDRA_BUILD_TYPEmacro +llama_hydra_build_type()accessor; append[type]tollama_build_info(); include inllama_print_build_info()common/build-info.h: declarellama_hydra_build_type()common/arg.cpp: include[type]in the--versionoutput (line 1091)common/common.cpp: include[type]in the TRACE-level build log (line 382)Test plan
build_sm120_v3,-DBUILD_SHARED_LIBS=ON) reports[shared]build_sm120_static,-DBUILD_SHARED_LIBS=OFF) reports[static]build_sm120_override,-DLLAMA_HYDRA_BUILD_TYPE=experimental) reports[experimental]build_sm60_v2) reports[shared]Cross-repo links
08-llama-fork.mdpush-before-PR ruleRisks
--versionoutput format changes (now includes[shared]/[static]). Any tooling that parses the version line with a strict regex needs to be updated. The downstream consumerscripts/ci/check-build-type.shis updated in the parent PR to match.[type]token. Any user that grep'd the string for a SHA match should be fine; the SHA is unchanged in position.common_params_print_infonow includes[type]— only visible at-lv 4and above, no impact on default verbosity.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com