fix(services): disable macOS ollama agent and fix local-binaries linker - #1279
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses two distinct but critical macOS-specific issues impacting development environment stability and build processes. It resolves a port conflict with the Ollama service by disabling a redundant Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Mesa DescriptionTL;DRDisabled the Ollama What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the macOS launchd agent for the Ollama service to prevent port conflicts with the native application, ensuring it's only enabled on Linux via systemd. Additionally, it introduces logic to scripts/update-local-binaries.sh to set the LIBRARY_PATH on macOS, aiming to help the linker find clang_rt libraries for Rust builds. A review comment highlights robustness issues in this new macOS logic, specifically regarding a hardcoded Xcode path and unsafe glob expansion, and provides a more resilient implementation suggestion.
| CLANG_LIB="$(echo /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin)" | ||
| if [ -d "$CLANG_LIB" ]; then | ||
| export LIBRARY_PATH="${CLANG_LIB}${LIBRARY_PATH:+:$LIBRARY_PATH}" | ||
| fi |
There was a problem hiding this comment.
This implementation has a couple of robustness issues:
- Hardcoded Xcode Path: The path
/Applications/Xcode.appis hardcoded. It's better to usexcode-select -pto find the developer directory dynamically, as Xcode can be installed in other locations. - Unsafe Glob Expansion: The command
echo ...*...is not safe. If the*glob expands to multiple directories (e.g., multiple clang versions installed),$CLANG_LIBwill contain a space-separated string of paths, and theif [ -d "$CLANG_LIB" ]test will fail with a "too many arguments" error.
Here is a more robust implementation that addresses both points by using xcode-select and handling glob expansion with an array.
| CLANG_LIB="$(echo /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin)" | |
| if [ -d "$CLANG_LIB" ]; then | |
| export LIBRARY_PATH="${CLANG_LIB}${LIBRARY_PATH:+:$LIBRARY_PATH}" | |
| fi | |
| XCODE_DEV_PATH="$(xcode-select -p 2>/dev/null)" | |
| if [ -n "$XCODE_DEV_PATH" ]; then | |
| # Use an array to safely handle glob expansion and take the first result. | |
| CLANG_LIB_PATHS=("$XCODE_DEV_PATH"/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin) | |
| if [[ ${#CLANG_LIB_PATHS[@]} -gt 0 && -d "${CLANG_LIB_PATHS[0]}" ]]; then | |
| export LIBRARY_PATH="${CLANG_LIB_PATHS[0]}${LIBRARY_PATH:+:$LIBRARY_PATH}" | |
| fi | |
| fi |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/update-local-binaries.sh">
<violation number="1" location="scripts/update-local-binaries.sh:10">
P2: Avoid hard-coding `/Applications/Xcode.app`; resolve the active developer directory dynamically so the linker path fix works across macOS setups.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Export LIBRARY_PATH in update-local-binaries.sh to point to the actual Xcode clang runtime dir, fixing Rust crate builds that hard-code an older clang lib search path (e.g. clang 17 vs installed clang 21). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 06e8f9690147
4cd8f53 to
089814e
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses two macOS-related developer-environment issues in the repo’s Home Manager services and local build tooling: avoiding a conflicting Ollama launchd agent on macOS, and improving Rust/macOS linking reliability by pointing the linker at Xcode’s clang_rt runtime directory.
Changes:
- Add a macOS-only
LIBRARY_PATHexport inscripts/update-local-binaries.shto help the linker findclang_rt. - Remove the macOS
launchdOllama agent and keep the Ollama service enabled only on Linux viasystemd.user.services.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/update-local-binaries.sh | Exports LIBRARY_PATH on Darwin to include Xcode clang_rt runtime directory for linker compatibility. |
| home-manager/services/ollama/default.nix | Drops macOS launchd agent configuration; retains Linux-only systemd user service. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CLANG_LIB="$(echo /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin)" | ||
| if [ -d "$CLANG_LIB" ]; then |
There was a problem hiding this comment.
CLANG_LIB is derived via echo .../clang/*/lib/darwin, which can expand to multiple paths (space-separated) if more than one clang version directory exists; in that case [ -d "$CLANG_LIB" ] will fail and LIBRARY_PATH won’t be set. Consider resolving a single directory deterministically (e.g., using xcode-select -p plus ls -d ... | sort -V | tail -n1, or a small find/array-glob) and avoiding echo for glob expansion.
| CLANG_LIB="$(echo /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin)" | |
| if [ -d "$CLANG_LIB" ]; then | |
| # Use the active Xcode toolchain and pick the highest clang version's darwin lib dir. | |
| XCODE_PATH="$(xcode-select -p 2>/dev/null || true)" | |
| if [ -n "$XCODE_PATH" ]; then | |
| CLANG_LIB="$(ls -d "$XCODE_PATH"/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/*/lib/darwin 2>/dev/null | sort -V | tail -n 1 || true)" | |
| else | |
| CLANG_LIB="" | |
| fi | |
| if [ -n "${CLANG_LIB:-}" ] && [ -d "$CLANG_LIB" ]; then |
Summary
:11434, causing perpetual "address already in use" errors in/tmp/ollama.error.logLIBRARY_PATHinupdate-local-binaries.shto point to the actual Xcode clang runtime dir, fixing Rust crate builds that hard-code an older clang lib search path (e.g. clang 17 vs installed clang 21)Test plan
make shell-test— 253 tests, 892 examples, 0 failurescoding_agent_session_searchbuilds successfully with theLIBRARY_PATHfix:11434after removing the home-manager agent🤖 Generated with Claude Code
Summary by cubic
Disable the Ollama
launchdagent on macOS to prevent :11434 conflicts with Ollama.app. Export the Xcode clang runtime path inupdate-local-binaries.shso Rust crates linkclang_rtcorrectly on macOS.launchdagent; keep the Linuxsystemduser service.update-local-binaries.sh, exportLIBRARY_PATHto the Xcode clanglib/darwindir when present, fixingclang_rtlinking errors.Written for commit 089814e. Summary will update on new commits.