feat: add setup-apple command for Apple Silicon Macs#285
Open
chhotu2601 wants to merge 3 commits intoNVIDIA:mainfrom
Open
feat: add setup-apple command for Apple Silicon Macs#285chhotu2601 wants to merge 3 commits intoNVIDIA:mainfrom
chhotu2601 wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Adds a 'nemoclaw setup-apple' command that validates and configures macOS Apple Silicon environments: - Validates macOS + arm64 architecture - Checks Docker Desktop socket (Desktop, Colima, Podman) - Installs and configures Ollama for local inference - Reports GPU cores and unified memory - Validates OpenShell CLI availability - Prints clear next steps and known limitations Closes part of NVIDIA#260 (macOS support tracking) Co-authored-by: Yajat Singh <yajatns@gmail.com>
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new "setup-apple" CLI command and a corresponding Bash script to detect and configure Apple Silicon macOS environments (checks macOS/arm64, Docker availability, Ollama installation/startup, OpenShell presence, and prints a final status and next steps). Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as "nemoclaw CLI"
participant Handler as "setupApple Handler"
participant Script as "scripts/setup-apple.sh"
participant System as "System Components\n(macOS, Docker, Ollama, OpenShell)"
User->>CLI: run `nemoclaw setup-apple`
CLI->>Handler: dispatch setupApple()
Handler->>Script: execute `scripts/setup-apple.sh`
Script->>System: check macOS version & architecture
System-->>Script: macOS/arch info
Script->>System: locate/verify Docker daemon & socket
System-->>Script: Docker reachable / version
Script->>System: install/start Ollama (brew or serve)
System-->>Script: Ollama API health / models list
Script->>System: check OpenShell CLI presence
System-->>Script: OpenShell status
Script-->>User: display formatted status and next steps
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup-apple.sh`:
- Around line 43-45: The script currently only warns when the ARCH variable is
not "arm64" or "aarch64" (the if block checking ARCH and calling warn), but this
setup is Apple-Silicon-specific and should fail fast; update that conditional to
stop execution on non-Apple-Silicon hosts by replacing the warn-only behavior
with a fatal exit (e.g., call the existing fatal/err handler or run exit 1) so
the script does not continue running Intel-specific systems — locate the if [
"$ARCH" != "arm64" ] && [ "$ARCH" != "aarch64" ] check and change it to
terminate with a non-zero status and a clear error message.
- Line 53: The GPU_CORES assignment pipeline can cause the script to exit under
set -euo pipefail if grep returns no match; update the command that sets
GPU_CORES so the pipeline cannot trigger pipefail (e.g., append a safe fallback
like "|| true" to the pipeline or wrap the pipeline in a construct that prevents
non-zero exit), then normalize/validate GPU_CORES afterwards (set a sensible
default like 0 or detect an alternate key) to ensure GPU_CORES is always
defined; target the GPU_CORES assignment line to implement these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06d1e9e8-fa3f-41cc-a7c4-3600cfae13e1
📒 Files selected for processing (2)
bin/nemoclaw.jsscripts/setup-apple.sh
…a cleanup - Intel Macs now fail with clear message instead of warning - Replaced python3 JSON parsing with grep (macOS 12.3+ has no Python) - Ollama startup prefers brew services (managed, survives reboots) - Added cleanup trap for background Ollama process - No orphaned processes on script failure/Ctrl-C
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/setup-apple.sh (1)
52-55:⚠️ Potential issue | 🟠 MajorMake GPU metadata collection non-fatal.
Line 53 still runs
grepinside a pipeline underset -euo pipefail, so localized or variantsystem_profileroutput can abort the entire setup instead of just leavingGPU_CORESempty.Suggested fix
- GPU_CORES=$(system_profiler SPDisplaysDataType 2>/dev/null | grep "Total Number of Cores" | awk -F': ' '{print $2}' | head -1) + GPU_CORES=$(system_profiler SPDisplaysDataType 2>/dev/null | awk -F': ' '/Total Number of Cores/{print $2; exit}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/setup-apple.sh` around lines 52 - 55, The GPU metadata collection can abort the script under set -euo pipefail because the pipeline (system_profiler | grep | awk) can fail; change the GPU/ memory detection to be non-fatal by running system_profiler into a temporary variable or file and then safely grepping/awk’ing with fallbacks (e.g., capture output from `system_profiler SPDisplaysDataType` into a var, then set GPU_CORES from that var using grep/awk with `|| true` or conditional checks) and similarly compute UNIFIED_MEM using `sysctl -n hw.memsize` but default to empty or zero if the command fails; update the code paths that set GPU_CORES and UNIFIED_MEM (references: GPU_CORES, UNIFIED_MEM, system_profiler, grep, awk, sysctl) so failures do not cause script exit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup-apple.sh`:
- Around line 129-139: The EXIT trap is killing the fallback Ollama background
process on normal script completion; update the trap logic so the background PID
stored in OLLAMA_BG_PID is only killed on interrupt signals, not on normal
exit—either remove EXIT from the trap and register it for INT and TERM only, or
introduce a boolean flag (e.g., OLLAMA_PRESERVE_ON_EXIT) set after confirming
Ollama started and change the trap handler to check that flag before killing
OLLAMA_BG_PID; adjust the trap declaration and use of OLLAMA_BG_PID and
OLLAMA_RUNNING accordingly so the process remains running after successful
script completion but is still cleaned up on interrupts.
- Around line 145-151: The MODEL_COUNT command substitution can produce
duplicate "0" lines because grep -co returns nonzero exit status and the || echo
fallback appends another "0", so update the MODEL_COUNT assignment to reliably
produce a single integer; for example, replace the current command that sets
MODEL_COUNT with a pipeline that counts matches robustly (e.g., use curl ... |
grep -o '"name"' | wc -l or use jq to count items) so the subsequent comparison
[ "$MODEL_COUNT" = "0" ] in the setup logic correctly detects an empty model
list; modify the assignment that defines MODEL_COUNT and keep the rest of the
conditional (info "No models found..." / info "Ollama has ${MODEL_COUNT}
model(s) available") unchanged.
---
Duplicate comments:
In `@scripts/setup-apple.sh`:
- Around line 52-55: The GPU metadata collection can abort the script under set
-euo pipefail because the pipeline (system_profiler | grep | awk) can fail;
change the GPU/ memory detection to be non-fatal by running system_profiler into
a temporary variable or file and then safely grepping/awk’ing with fallbacks
(e.g., capture output from `system_profiler SPDisplaysDataType` into a var, then
set GPU_CORES from that var using grep/awk with `|| true` or conditional checks)
and similarly compute UNIFIED_MEM using `sysctl -n hw.memsize` but default to
empty or zero if the command fails; update the code paths that set GPU_CORES and
UNIFIED_MEM (references: GPU_CORES, UNIFIED_MEM, system_profiler, grep, awk,
sysctl) so failures do not cause script exit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aac52eed-d44c-4572-b3fa-281c6bf221fd
📒 Files selected for processing (1)
scripts/setup-apple.sh
…fallback - Comment 2: Add || true to GPU core detection pipeline to prevent pipefail abort on VMs/headless Macs - Comment 3: Remove EXIT from trap — only kill Ollama on INT/TERM, not normal script completion - Comment 4: Disable pipefail for MODEL_COUNT grep to handle empty JSON gracefully
Contributor
|
Thanks for this — a Could you rebase this branch against the latest main and push an update? If you run into questions while rebasing, feel free to ask here. If we don't hear back within 7 days, we'll post a reminder; items with no response at 14 days are closed to keep the queue healthy. |
wscurran
added
the
status: rebase
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
3 participants
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
Adds a
nemoclaw setup-applecommand (analogous tosetup-sparkfor DGX Spark) that validates and configures Apple Silicon Mac environments for NemoClaw.What it does
Why
macOS/Apple Silicon is a major developer platform, but NemoClaw only has
setup-sparkfor DGX. This fills the gap for Mac users who need guidance on Docker Desktop + Ollama configuration.References tracking issue #260.
Test plan
Screenshot
N/A (CLI output only)
Co-authored-by: Yajat Singh yajatns@gmail.com
Addresses part of #260
Summary by CodeRabbit