Skip make on CMake-only llama.cpp checkouts in install_llama_cpp#763
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function _is_cmake_only_llama_cpp to detect if a llama.cpp checkout is CMake-only by inspecting its Makefile. This helper is used in install_llama_cpp to skip the make build step when appropriate, preventing misleading build errors. The review feedback suggests formatting improvements to comply with PEP 8, specifically removing spaces around keyword arguments and wrapping a long return statement to adhere to the 79-character line limit.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _is_cmake_only_llama_cpp(llama_cpp_folder): | ||
| """True if llama.cpp's Makefile is the post-CMake-migration deprecation | ||
| stub (or missing entirely), so `make` cannot build it.""" | ||
| makefile = os.path.join(llama_cpp_folder, "Makefile") | ||
| if not os.path.exists(makefile): | ||
| return True | ||
| try: | ||
| with open(makefile, "r", encoding = "utf-8", errors = "ignore") as f: | ||
| content = f.read(4096) | ||
| except OSError: | ||
| return False | ||
| lowered = content.lower() | ||
| return "build system changed" in lowered or ("cmake" in lowered and "deprecated" in lowered) |
There was a problem hiding this comment.
According to the PEP 8 style guide, there should be no spaces around the = sign when used to indicate a keyword argument (e.g., encoding="utf-8" instead of encoding = "utf-8"). Additionally, the return statement exceeds the recommended 79-character line limit. Wrapping the return expression and removing the spaces around keyword arguments will improve readability and PEP 8 compliance.
| def _is_cmake_only_llama_cpp(llama_cpp_folder): | |
| """True if llama.cpp's Makefile is the post-CMake-migration deprecation | |
| stub (or missing entirely), so `make` cannot build it.""" | |
| makefile = os.path.join(llama_cpp_folder, "Makefile") | |
| if not os.path.exists(makefile): | |
| return True | |
| try: | |
| with open(makefile, "r", encoding = "utf-8", errors = "ignore") as f: | |
| content = f.read(4096) | |
| except OSError: | |
| return False | |
| lowered = content.lower() | |
| return "build system changed" in lowered or ("cmake" in lowered and "deprecated" in lowered) | |
| def _is_cmake_only_llama_cpp(llama_cpp_folder): | |
| """True if llama.cpp's Makefile is the post-CMake-migration deprecation | |
| stub (or missing entirely), so `make` cannot build it.""" | |
| makefile = os.path.join(llama_cpp_folder, "Makefile") | |
| if not os.path.exists(makefile): | |
| return True | |
| try: | |
| with open(makefile, "r", encoding="utf-8", errors="ignore") as f: | |
| content = f.read(4096) | |
| except OSError: | |
| return False | |
| lowered = content.lower() | |
| return ( | |
| "build system changed" in lowered | |
| or ("cmake" in lowered and "deprecated" in lowered) | |
| ) |
References
- PEP 8: Don't use spaces around the = sign when used to indicate a keyword argument. Limit all lines to a maximum of 79 characters. (link)
Summary
llama.cpp replaced its Makefile build with CMake; the Makefile that remains is a stub that errors with "Build system changed: The Makefile build has been replaced by CMake". install_llama_cpp() on Linux/macOS still probes with make clean / make all first, so every source build against a modern checkout surfaces that misleading error in build_errors and wastes a build attempt before falling back to cmake.
This adds _is_cmake_only_llama_cpp(), which detects the deprecation stub (or a missing Makefile) and goes straight to cmake. Legacy checkouts with a real Makefile keep the make-first path unchanged.
Companion to unslothai/unsloth#5957, which guards the legacy build paths in unsloth/save.py the same way. Together they cover unslothai/unsloth#5832.
Testing