cmake : only take build info from the llama.cpp source tree by apollo-2006 · Pull Request #28462 · ggml-org/llama.cpp

apollo-2006 · 2026-09-06T02:02:29Z

Overview

cmake/build-info.cmake ran git rev-parse --short HEAD and git rev-list --count HEAD and accepted the result on RES EQUAL 0 alone. git searches parent directories, so a source tree with no repository of its own picks up whatever repository sits above it. A release tarball unpacked inside any unrelated git working tree therefore stamps that repository's HEAD into --version. The failure is silent: you get a well-formed 7-hex string that resolves to nothing, which reads like a broken lookup rather than a wrong hash.

Both values were affected, not just the commit. BUILD_NUMBER came from the same unverified repository, so the whole b<number>-<commit> string could be fabricated.

Second site: `cmake -P`

The issue reports the tarball case. There is a second one. scripts/ui-assets.cmake:421 includes this file, and it is run in script mode from tools/ui/CMakeLists.txt:56 (cmake ... -P "${PROJECT_SOURCE_DIR}/scripts/ui-assets.cmake"). In script mode CMAKE_CURRENT_SOURCE_DIR is the invoking working directory, not the source tree, so the git lookup ran from the build directory and walked up from there. resolve_version() uses the resulting BUILD_NUMBER to choose which prebuilt UI bundle to fetch, so this one is not only cosmetic. Pinning the lookup to CMAKE_CURRENT_LIST_DIR fixes both sites at once and needs no change in ui-assets.cmake.

Verification

Real git archive tarball of 74a7c897f extracted inside an unrelated repository whose HEAD is 0c71b95, configured with -DLLAMA_CURL=OFF -DGGML_CUDA=OFF:

	`LLAMA_COMMIT`	`LLAMA_BUILD_NUMBER`
before	`0c71b95` (the outer repo)	`1` (the outer repo)
after	`unknown`	`0`

The real checkout is unchanged, still 74a7c897f / 10820.

Placement matrix, run against both the old and the new file:

case	before	after
tree is its own git repo	correct	correct
tarball inside an unrelated repo	foreign commit + count	`unknown` / `0`
tarball outside any repo	`unknown` / `0`	`unknown` / `0`
`cmake -P`, cwd inside an unrelated repo	foreign commit + count	correct

git rev-parse --show-toplevel and CMAKE_CURRENT_LIST_DIR are both passed through REALPATH before comparison, so a symlinked source or build path does not produce a spurious mismatch.

Behavior change worth flagging

A tree that is genuinely vendored into a larger repository, copied in rather than added as a submodule, previously reported the parent repository's commit and now reports unknown. Submodules and git worktree checkouts are unaffected, since --show-toplevel returns their own path. FetchContent with GIT_REPOSITORY is also unaffected, as the fetched tree carries its own .git. The only case that loses a value is the one where that value named a repository that has never contained llama.cpp, and unknown is the honest answer there. This repo's issue templates ask for the --version string, so a wrong hash costs more than a missing one.

Requirements

I have read and agree with the contributing guidelines
AI usage disclosure: Yes. Used AI to do an overpass after the code was written, as well as to review the PR body.

git searches parent directories for a repository, so a source tree with none of its own (a release tarball, or vendored sources) picked up whatever repository sat above it and reported that one's HEAD and commit count as llama.cpp's. The result is a plausible-looking hash that resolves to nothing. Verify the repository git finds is this source tree before trusting it, and locate that tree from CMAKE_CURRENT_LIST_DIR rather than CMAKE_CURRENT_SOURCE_DIR, which is the invoking working directory when build-info.cmake is reached through `cmake -P`, as scripts/ui-assets.cmake does.

Copilot

🟡 Changes recommended

Several new/modified execute_process(WORKING_DIRECTORY ...) uses pass an unquoted path variable, which can break configuration when the source path contains spaces (notably on Windows).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens CMake build metadata generation so cmake/build-info.cmake only uses Git information when the Git repository discovered by git is actually the llama.cpp source tree (preventing foreign commit/count values when building from release tarballs inside unrelated Git working trees, including cmake -P invocation paths).

Changes:

Derive the source root for build-info from CMAKE_CURRENT_LIST_DIR (file location) instead of CMAKE_CURRENT_SOURCE_DIR (invocation cwd in cmake -P).
Validate git rev-parse --show-toplevel matches the llama.cpp source tree before trusting rev-parse / rev-list results.
Fall back to unknown / 0 when the repository is missing or does not match the source tree.

File summaries

File	Description
cmake/build-info.cmake	Anchors Git lookup to the build-info file's source tree and rejects Git data from unrelated parent repositories.

Review details

Suppressed comments (2)

cmake/build-info.cmake:55

WORKING_DIRECTORY should quote BUILD_INFO_SOURCE_DIR to avoid breaking when the source path contains spaces (CMake otherwise tokenizes it into multiple arguments).

    execute_process(
        COMMAND ${GIT_EXECUTABLE} rev-parse --short HEAD
        WORKING_DIRECTORY ${BUILD_INFO_SOURCE_DIR}
        OUTPUT_VARIABLE HEAD
        OUTPUT_STRIP_TRAILING_WHITESPACE
        RESULT_VARIABLE RES

cmake/build-info.cmake:66

WORKING_DIRECTORY should quote BUILD_INFO_SOURCE_DIR to avoid breaking when the source path contains spaces (CMake otherwise tokenizes it into multiple arguments).

    execute_process(
        COMMAND ${GIT_EXECUTABLE} rev-list --count HEAD
        WORKING_DIRECTORY ${BUILD_INFO_SOURCE_DIR}
        OUTPUT_VARIABLE COUNT
        OUTPUT_STRIP_TRAILING_WHITESPACE
        RESULT_VARIABLE RES
    )

Files reviewed: 1/1 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    execute_process(
+        COMMAND ${GIT_EXECUTABLE} rev-parse --show-toplevel
+        WORKING_DIRECTORY ${BUILD_INFO_SOURCE_DIR}
+        OUTPUT_VARIABLE GIT_TOPLEVEL
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+        ERROR_QUIET
+        RESULT_VARIABLE RES
+    )


+            message(STATUS "Git repository at ${GIT_TOPLEVEL} is not the llama.cpp source tree "
+                           "(${BUILD_INFO_SOURCE_DIR}). Build info will not be accurate.")


ggml-gh-bot · 2026-09-06T02:07:01Z

Hi @apollo-2006, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.
Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

An unquoted variable reference is expanded as a CMake list, so a source path containing a semicolon is split into several arguments and execute_process fails with "given unknown argument". Paths containing spaces were never affected.

apollo-2006 · 2026-09-09T16:27:45Z

Both flags were correct when they fired. Both are resolved now.

PR Template not respected: the body was missing ## Overview and ## Requirements. I added both, with the AI usage disclosure filled in, roughly 12 minutes after this comment posted. The check has not run again since.

Multiple open PRs from a new contributor: I had two open at the time. #28463 was closed on 09-06 and #28261 on 09-08, so this is now my only open PR here.

apollo-2006 · 2026-09-13T01:34:27Z

@angt #28445 landed shortly after I opened this and moved both references in my Second site section: scripts/ui-assets.cmake:172 is now :421, and tools/ui/CMakeLists.txt:106 is now :56. I will update the body.

The bug survives the move. On cmake 4.4.3, -P script mode leaves CMAKE_CURRENT_SOURCE_DIR set to the invoking working directory even inside an include()d file, so the lookup in build-info.cmake still walks up from the build directory.

Your change passes -DLLAMA_SOURCE_DIR into the script, but build-info.cmake never reads it. Was that meant to cover this lookup too, or would you rather the fix be pinning to CMAKE_CURRENT_LIST_DIR, as this PR does?

apollo-2006 requested a review from ggerganov as a code owner September 6, 2026 02:02

Copilot AI lite review requested due to automatic review settings September 6, 2026 02:02

github-actions Bot added the build Compilation issues label Sep 6, 2026

Copilot started reviewing on behalf of apollo-2006 September 6, 2026 02:03 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

ggml-gh-bot Bot added the draft PR will be changed to draft by github-actions bot label Sep 6, 2026

github-actions Bot marked this pull request as draft September 6, 2026 02:07

github-actions Bot removed the draft PR will be changed to draft by github-actions bot label Sep 6, 2026

cmake : quote the working directory passed to git

66064d3

An unquoted variable reference is expanded as a CMake list, so a source path containing a semicolon is split into several arguments and execute_process fails with "given unknown argument". Paths containing spaces were never affected.

apollo-2006 marked this pull request as ready for review September 6, 2026 02:19

apollo-2006 closed this Sep 6, 2026

apollo-2006 reopened this Sep 8, 2026

apollo-2006 mentioned this pull request Sep 16, 2026

Misc. bug: Vulkan im2col / im2col_3d shaders write through buffer_reference without buffer_reference_align (VUID-RuntimeSpirv-PhysicalStorageBuffer64-06315) #28960

Closed

Navigation Menu

cmake : only take build info from the llama.cpp source tree - #28462

cmake : only take build info from the llama.cpp source tree#28462
apollo-2006 wants to merge 2 commits into
ggml-org:masterfrom
apollo-2006:fix/build-info-foreign-repo

apollo-2006 commented Sep 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

ggml-gh-bot Bot commented Sep 6, 2026

Uh oh!

apollo-2006 commented Sep 9, 2026

Uh oh!

apollo-2006 commented Sep 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		message(STATUS "Git repository at ${GIT_TOPLEVEL} is not the llama.cpp source tree "
		"(${BUILD_INFO_SOURCE_DIR}). Build info will not be accurate.")

Conversation

apollo-2006 commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Second site: cmake -P

Verification

Behavior change worth flagging

Requirements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

ggml-gh-bot Bot commented Sep 6, 2026

Uh oh!

apollo-2006 commented Sep 9, 2026

Uh oh!

apollo-2006 commented Sep 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

apollo-2006 commented Sep 6, 2026 •

edited

Loading

Second site: `cmake -P`