docker/unified: build llama.cpp with static libraries#616
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe pull request transitions the Docker build and runtime configuration from dynamically linked binaries with shared libraries to statically linked binaries. The build script disables shared library compilation via CMake flag, removes shared library installation infrastructure, and the Docker runtime stage copies only the final binaries. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Use BUILD_SHARED_LIBS=OFF so llama.cpp's internal libraries (libggml, libllama) are statically linked into the binaries, eliminating the need to copy and manage shared libraries in the runtime container. - add -DBUILD_SHARED_LIBS=OFF to cmake flags - remove shared library copy step and /install/lib directory - remove CMAKE_SHARED_LINKER_FLAGS from CUDA build - remove COPY --from=llama-build /install/lib/ from Dockerfile Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/unified/install-sd.sh (1)
23-30:⚠️ Potential issue | 🟡 MinorAdd explicit
-DBUILD_SHARED_LIBS=OFFflag for clarity and consistency withinstall-llama.sh.
install-llama.shexplicitly uses-DBUILD_SHARED_LIBS=OFFfor static linking. While stable-diffusion.cpp defaults to static builds (SD_BUILD_SHARED_LIBS=OFFby default), adding the explicit flag here would improve consistency and make the intent clearer. Currently, line 65 copies.sofiles only if they exist, which is harmless but suggests shared library support; explicitly requesting static builds would eliminate any ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/unified/install-sd.sh` around lines 23 - 30, The CMAKE_FLAGS array in install-sd.sh should explicitly request static linking like install-llama.sh; add the -DBUILD_SHARED_LIBS=OFF flag to the CMAKE_FLAGS tuple so cmake is unambiguous about building static libs (update the CMAKE_FLAGS variable in install-sd.sh where it is defined and used, alongside existing flags such as -DSD_BUILD_EXAMPLES=ON), which prevents accidental shared .so output and clarifies intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/unified/Dockerfile`:
- Line 106: The runtime-vulkan stage uses an incompatible base image
"ubuntu:26.04"; update the Dockerfile's runtime-vulkan stage (the line "FROM
ubuntu:26.04 AS runtime-vulkan") to use the same supported base as the builder
stage (e.g., ubuntu:24.04 or the exact image used in the builder stage) so both
stages are consistent.
---
Outside diff comments:
In `@docker/unified/install-sd.sh`:
- Around line 23-30: The CMAKE_FLAGS array in install-sd.sh should explicitly
request static linking like install-llama.sh; add the -DBUILD_SHARED_LIBS=OFF
flag to the CMAKE_FLAGS tuple so cmake is unambiguous about building static libs
(update the CMAKE_FLAGS variable in install-sd.sh where it is defined and used,
alongside existing flags such as -DSD_BUILD_EXAMPLES=ON), which prevents
accidental shared .so output and clarifies intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7e7332af-f8ec-4b99-a784-e57f30216bc5
📒 Files selected for processing (4)
.github/workflows/unified-docker.ymldocker/unified/Dockerfiledocker/unified/install-llama.shdocker/unified/install-sd.sh
63e5246 to
5eca715
Compare
No description provided.