fix build-info.h for git submodules

[kuvaus]

If llama.cpp is added as a project submodule, .git is a text file containing gitdir. Adding extra backslash .git/ to if(EXISTS) makes it sure that .git/ is a folder containing index.

[kuvaus]

Fix issue with git submodule and CMake in llama.cpp

In the discussion on #1232 (comment) there was a suggestion to include llama.cpp as a git submodule and include the CMake via add_subdirectory(external/llama.cpp).

was thinking of including llama.cpp as a git submodule or git subtree and including the cmake via eg add_subirectory(external/llama.cpp) .

CMAKE_CURRENT_SOURCE_DIR

In the llama.cpp's project root directory is the way to go, I think.

However, when using a git submodule, the .git file in the submodule directory is actually a text file containing a reference to the actual git directory:

gitdir: ../.git/modules/llama.cpp

In that case, the current implementation in CMakeLists.txt causes an error. Line 93 checks for the existence of the .git file:

if(EXISTS "\${CMAKE_CURRENT_SOURCE_DIR}/.git")

Since the file exists, the condition passes. However, line 103 has a dependency on the .git/index file, which doesn't exist in this case:

DEPENDS "\${CMAKE_CURRENT_SOURCE_DIR}/.git/index"

This causes the following error:

No rule to make target 'project/llama.cpp/.git/index', needed by 'project/llama.cpp/build-info.h'. Stop.

To fix this issue, we can add backslash to line 93 to check that .git/ is a folder.

if(EXISTS "\${CMAKE_CURRENT_SOURCE_DIR}/.git/")

This ensures that the .git file is a directory that can contain the required index file.

[Green-Sky]

No. The git commands should work for submodules. However the observation, that the .git/index file in that case does not exist is correct. Can the DEPENDS be applied to directories too?

[Green-Sky]

I don't understand why we need those lines:

llama.cpp/CMakeLists.txt

Lines 97 to 105 in 67c7779

	# Add a custom command to rebuild build-info.h when .git/index changes
	add_custom_command(
	OUTPUT "${CMAKE_CURRENT_SOURCE_DIR}/build-info.h"
	COMMENT "Generating build details from Git"
	COMMAND ${CMAKE_COMMAND} -P "${CMAKE_CURRENT_SOURCE_DIR}/scripts/build-info.cmake"
	WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
	DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/.git/index"
	VERBATIM
	)

Is the build script not executed every time anyway?

llama.cpp/CMakeLists.txt

Lines 90 to 91 in 67c7779

	# Generate initial build-info.h
	include(${CMAKE_CURRENT_SOURCE_DIR}/scripts/build-info.cmake)

[kuvaus]

I removed the suggested lines 97-105 and the empty line at 106. Tested with a project that contained llama.cpp as a submodule and it compiles now. Good. I do get this warning:

CMake Warning at llama.cpp/CMakeLists.txt:97 (message):
  Git repository not found; to enable automatic generation of build info,
  make sure Git is installed and the project is a Git repository.

[Green-Sky]

I removed the suggested lines 97-105 and the empty line at 106. Tested with a project that contained llama.cpp as a submodule and it compiles now. Good. I do get this warning:

CMake Warning at llama.cpp/CMakeLists.txt:97 (message):
  Git repository not found; to enable automatic generation of build info,
  make sure Git is installed and the project is a Git repository.

not sure what your issue is, but I tried those changes and it works fine now for me.

eg output of benchmark:

$ bin/benchmark
main: build = 488 (67c7779)
Starting Test

[kuvaus]

Oh, my mistake. I had not properly cleaned build directory on rebuild. It works now without errors. Thank you!

[DannyDaemonic]

Perhaps this was a case of me trying to be too clever. We only need to reevaluate when git changes, but then when you first open the project a bunch of errors would show up because build-info.h wasn't found yet. So I added a line to generate it right away. Simpler is probably better if this just works.

[Green-Sky]

Actually, we do need the add_custom_command() with the index dependency. I will write some cmake to handle subdirectories.

[Green-Sky]

@DannyDaemonic the line

set(TEMPLATE_FILE "${CMAKE_BINARY_DIR}/BUILD_INFO.h.in")

causes issues

CMake Error: File /home/green/workspace/tmp_git_pr/external/llama.cpp/BUILD_INFO.h.in does not exist.
CMake Error at scripts/build-info.cmake:49 (configure_file):
  configure_file Problem configuring file

bacause

When run in cmake -P script mode, CMake sets the variables CMAKE_BINARY_DIR, CMAKE_SOURCE_DIR, CMAKE_CURRENT_BINARY_DIR and CMAKE_CURRENT_SOURCE_DIR to the current working directory.

so I am going to put the .in file next to the script file.

[Green-Sky]

@kuvaus please check my changes, I force pushed onto your branch.

[kuvaus]

Nice. Looks like it builds without errors for me.
(That regex replace is clever. So it reads the correct path from the .git text file and then uses the correct one)

[DannyDaemonic]

I had to find a project to test this on. Ended up using kuvaus' llama-chat.

Looks good to me; smart fix @Green-Sky.

[Green-Sky]

@kuvaus you merged in master literally seconds before I wanted to click merge. 🤣
now we have to wait on the ci again...