fix: dynamic tools CMakeLists.txt for selective MTMD building#810
Open
hongkongkiwi wants to merge 1 commit intoutilityai:mainfrom
Open
fix: dynamic tools CMakeLists.txt for selective MTMD building#810hongkongkiwi wants to merge 1 commit intoutilityai:mainfrom
hongkongkiwi wants to merge 1 commit intoutilityai:mainfrom
Conversation
… building Replace static tools CMakeLists.txt approach with dynamic generation based on enabled features. This creates a scalable, merge-friendly solution for our feature branches. Key improvements: - Dynamic CMakeLists.txt generation via generate_tools_cmake() function - Only builds tools for enabled features (currently just MTMD) - Designed for easy extension by other feature branches - No static files to conflict during merges - Clear extension points with commented examples Architecture: - generate_tools_cmake() creates tools/CMakeLists.txt at build time - any_tool_features flag determines if tools directory should be built - Each feature branch can add their tool by uncommenting their section This solves the original PR utilityai#806 issue (avoiding building all tools) while providing a foundation for RPC, server, quantize, and other tool features. Future branches can easily add their tools by: 1. Adding their feature to any_tool_features check 2. Uncommenting their add_subdirectory() line in generate_tools_cmake() 3. Including their tool's CMakeLists.txt in Cargo.toml 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Author
|
This is a solution where we can fix the build errors identified in #810 but in a way which doesn't try to build the whole tools dir. |
Contributor
|
does the llama.cpp Cmake build the tools dir when there are no features enabled right now? |
4 tasks
|
I can confirm the same build issues (macos), and this pr solves my problem |
MarcusDunn
reviewed
Aug 26, 2025
Comment on lines
+240
to
+242
| let tools_cmake_path = Path::new("llama.cpp/tools/CMakeLists.txt"); | ||
| fs::write(tools_cmake_path, cmake_content) | ||
| .expect("Failed to write generated tools CMakeLists.txt"); |
Contributor
There was a problem hiding this comment.
I believe this fail when building docs because it's built on a read-only file system.
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
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
mtmdfeature is enabled, avoiding building all 15+ unnecessary toolsKey Changes
generate_tools_cmake()creates tools/CMakeLists.txt at build time based on enabled featuresTechnical Implementation
Benefits
mtmdfeature enabledTest Plan
cargo build --features mtmdResolves #806
🤖 Generated with Claude Code