fix(hailo): build Hailo-Ollama from hailo_model_zoo_genai via cmake (real repo)#1850
Conversation
…istent repo The installer defaulted HAILO_OLLAMA_REPO to hailo-ai/hailo-ollama, which does not exist (git ls-remote: Repository not found), so every Hailo-10H install failed at clone. The Hailo-Ollama server is not a standalone repo: it ships inside hailo-ai/hailo_model_zoo_genai and is built from source there (its README: an Ollama-compatible API written in C++ on top of HailoRT). - Point the repo at hailo_model_zoo_genai, pinned to a real commit. - Replace the Python venv/pip build with the repo's actual cmake flow (configure -> build -> install), adding the C++ toolchain + OpenSSL deps. cmake --install lands the hailo-ollama binary in /usr/local/bin and manifests under /usr/local/share/hailo-ollama. - Resolve the binary from PATH (system install), not a venv. - Start the server bare (it has no serve subcommand or --port flag) and set the listen port via OLLAMA_HOST (bind 127.0.0.1 on the managed port 7836, off the banned default 8000), per the repo's docs/USAGE.rst. Not hardware-tested locally (no Hailo-10H here); validated by bash -n and the upstream build docs. doc62fr (#1771) tests live on real hardware and reports logs.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe Hailo installer now clones a pinned ChangesHailo Ollama installation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| cmake build-essential libssl-dev >/dev/null | ||
|
|
||
| log "configuring hailo-ollama build (cmake, Release)" | ||
| run_as_user sh -c "cd '$HAILO_OLLAMA_DIR' && cmake -B build -DCMAKE_BUILD_TYPE=Release" |
There was a problem hiding this comment.
WARNING: Configuring from the repo top-level may not build the hailo-ollama server.
hailo_model_zoo_genai is a large meta-repo; its top-level CMakeLists.txt may build the entire GenAI zoo (or nothing relevant) rather than the C++ hailo-ollama server, or the server may live in a subdirectory (e.g. hailo-ollama/) that needs its own configure flag or that is disabled by default. A top-level cmake -B build could either fail (if it pulls in Python/GPU deps) or succeed while producing no hailo-ollama binary, and resolve_bin would then die. Since this path was not hardware-tested, please confirm against the tester's live install (#1771) that this exact top-level configure produces hailo-ollama in build/bin (or adjust the working directory / flags to the server's own CMake project).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| log "compiling hailo-ollama (C++; can take several minutes on the Pi)" | ||
| run_as_user sh -c "cd '$HAILO_OLLAMA_DIR' && cmake --build build --config Release" | ||
| log "installing hailo-ollama system-wide (cmake --install, needs sudo)" | ||
| sudo cmake --install "$HAILO_OLLAMA_DIR/build" |
There was a problem hiding this comment.
SUGGESTION: cmake --install has no prefix or target selection.
Running cmake --install without -DCMAKE_INSTALL_PREFIX=... and without a target (--component/--target) installs whatever install rules the whole repo defines into /usr/local. For a meta-repo like hailo_model_zoo_genai that may install far more than the hailo-ollama binary (headers, manifests, other tools), polluting the system and widening the attack/footprint surface. Consider pinning the prefix (e.g. CMAKE_INSTALL_PREFIX=/usr/local explicitly) and, if the CMake project exposes a component/target for just the server, installing only that. Also note this step requires root and writes to /usr/local — the binary ends up root-owned, which is acceptable but worth being deliberate about.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # Match the installed binary path, not any command line merely containing | ||
| # "hailo-ollama" (which would kill an editor or tail open on these files). | ||
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN serve --port $HAILO_OLLAMA_PORT" | ||
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN" |
There was a problem hiding this comment.
SUGGESTION: pkill -9 -f "$HAILO_OLLAMA_BIN" now matches only the bare binary path.
Previously the pattern included serve --port $HAILO_OLLAMA_PORT, so it targeted the specific daemon invocation. Now it matches any process whose full command line contains the binary path (e.g. hailo-ollama). This is more robust for reaping orphans, but be aware it will also kill the cmake --install-launched process or any manual hailo-ollama run if one is live during a restart. The - prefix tolerates "no process found", so a no-match won't fail the unit. Low risk, just confirming intent and that no unrelated process legitimately runs this exact path under the target user.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 files)
Fix these issues in Kilo Cloud Reviewed by hy3:free · Input: 70K · Output: 3.8K · Cached: 76.1K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/install-hailo.sh`:
- Around line 376-382: Update the ExecStartPre pkill command to use exact
process-name matching with -x and pass the basename of HAILO_OLLAMA_BIN, rather
than matching the full command line or path. Preserve the existing pre-start
cleanup and non-failing behavior.
🪄 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 Plus
Run ID: 76bb0d9a-05ef-4307-9db3-44521cdeb85f
📒 Files selected for processing (1)
scripts/install-hailo.sh
| Environment=OLLAMA_HOST=127.0.0.1:$HAILO_OLLAMA_PORT | ||
| # hailo-ollama can leave a bare orphan process listening on the port if it | ||
| # crashes during model load (the adopt-an-orphan lesson from PR #1755 for | ||
| # rkllama). Reap any such process before (re)start so the bind cannot fail. | ||
| # Match the exact server invocation, not any command line merely containing | ||
| # Match the installed binary path, not any command line merely containing | ||
| # "hailo-ollama" (which would kill an editor or tail open on these files). | ||
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN serve --port $HAILO_OLLAMA_PORT" | ||
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use exact process name matching (-x) to avoid killing editors.
The -f flag tells pkill to match the pattern against the full command line (i.e., argv), not just the executable path. Consequently, pkill -f "/usr/local/bin/hailo-ollama" will terminate commands like nano /usr/local/bin/hailo-ollama or tail -f /usr/local/bin/hailo-ollama because the string is present in their arguments, violating the explicit intent described in the comment.
To safely match the exact process and ignore arguments, use the exact process name matcher (-x) with the binary's basename. Process names in Linux are derived from the executable's basename.
🐛 Proposed fix
Environment=OLLAMA_HOST=127.0.0.1:$HAILO_OLLAMA_PORT
# hailo-ollama can leave a bare orphan process listening on the port if it
# crashes during model load (the adopt-an-orphan lesson from PR `#1755` for
# rkllama). Reap any such process before (re)start so the bind cannot fail.
-# Match the installed binary path, not any command line merely containing
-# "hailo-ollama" (which would kill an editor or tail open on these files).
-ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN"
+# Match the exact process name, not any command line merely containing the path
+# (which -f does, and would kill an editor or tail open on these files).
+ExecStartPre=-/usr/bin/pkill -9 -x "$(basename "$HAILO_OLLAMA_BIN")"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Environment=OLLAMA_HOST=127.0.0.1:$HAILO_OLLAMA_PORT | |
| # hailo-ollama can leave a bare orphan process listening on the port if it | |
| # crashes during model load (the adopt-an-orphan lesson from PR #1755 for | |
| # rkllama). Reap any such process before (re)start so the bind cannot fail. | |
| # Match the exact server invocation, not any command line merely containing | |
| # Match the installed binary path, not any command line merely containing | |
| # "hailo-ollama" (which would kill an editor or tail open on these files). | |
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN serve --port $HAILO_OLLAMA_PORT" | |
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN" | |
| Environment=OLLAMA_HOST=127.0.0.1:$HAILO_OLLAMA_PORT | |
| # hailo-ollama can leave a bare orphan process listening on the port if it | |
| # crashes during model load (the adopt-an-orphan lesson from PR `#1755` for | |
| # rkllama). Reap any such process before (re)start so the bind cannot fail. | |
| # Match the exact process name, not any command line merely containing the path | |
| # (which -f does, and would kill an editor or tail open on these files). | |
| ExecStartPre=-/usr/bin/pkill -9 -x "$(basename "$HAILO_OLLAMA_BIN")" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install-hailo.sh` around lines 376 - 382, Update the ExecStartPre
pkill command to use exact process-name matching with -x and pass the basename
of HAILO_OLLAMA_BIN, rather than matching the full command line or path.
Preserve the existing pre-start cleanup and non-failing behavior.
1 participant
The installer defaulted
HAILO_OLLAMA_REPOtohailo-ai/hailo-ollama, which does not exist (git ls-remote: Repository not found), so every Hailo-10H install failed at clone. Hailo-Ollama is not a standalone repo: it ships insidehailo-ai/hailo_model_zoo_genaiand is built from source there (C++ on HailoRT, per its README).hailo_model_zoo_genai, pinned to1a3ba6be.cmake --installlands thehailo-ollamabinary in /usr/local/bin.serve --port); port viaOLLAMA_HOST=127.0.0.1:7836(off the banned default 8000), per the repo's docs/USAGE.rst.Not hardware-tested locally (no Hailo-10H here); validated by
bash -n+ the upstream build docs. doc62fr (#1771) validates live and reports logs.Summary by CodeRabbit