fix(install): support Jetson BSP R39+ in host setup script - #1910
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-jetson.sh`:
- Around line 29-37: The info() messages inside the Jetson detection logic are
being written to stdout and polluting the captured output of
get_jetpack_version, so update the three info() invocations in the Jetson checks
to write to stderr instead (e.g., append >&2 or call a stderr-capable logger) —
specifically change the info call inside the “could not parse L4T release”
branch, the info call inside the “L4T R39+ does not require host setup” branch,
and the info call used during JetPack version detection so that
get_jetpack_version returns empty when appropriate; keep the surrounding
conditionals and return statements (the release checks and the
jetpack_version="$(get_jetpack_version)" usage) unchanged.
🪄 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: 022462f1-ddff-4890-a7b8-d7838b23c3a6
📒 Files selected for processing (1)
scripts/setup-jetson.sh
|
PR description:
Lines 73-78 still run modprobe, sysctl, and persistent config writes for every recognized version. I suspect the PR description should be updated to be more clear about what is updated or not, or those commands should not be run in the r38 case? |
R39 and later BSP versions do not require any host customization. Detect the L4T release number arithmetically so future versions are handled automatically without needing script changes. Signed-off-by: Paritosh Dixit <paritoshd@nvidia.com>
9af20fc to
784451f
Compare
Hi @brandonpelfrey , |
ericksoa
left a comment
There was a problem hiding this comment.
Regression analysis looks clean — low risk across all four changes. The stderr redirects actually fix a latent bug where unrecognized versions would crash the script instead of gracefully skipping. No external references to the renamed jp7 identifier. All CI green. LGTM.
## Summary - Add "Ollama network exposure warning during onboard" troubleshooting entry (from #1877) - Document snapshot restore liveness preflight and clean restore behavior (from #1901) - Update Jetson troubleshooting for BSP R39+ support (from #1910) - Document `--from` Dockerfile permission error handling (from #1931) - Bump doc version switcher through 0.0.17 - Regenerate agent skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit hooks pass - [ ] Verify rendered pages in docs build output 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified onboarding error when build context contains unreadable files. * Updated snapshot restore: sandbox must be running; restore cleanly replaces state directories and removes files added after the snapshot. * Added Jetson BSP R39 automatic configuration guidance. * Added Ollama network-exposure security guidance for local provider selection during onboarding. * **Documentation (versions)** * Added docs entry for version 0.0.17 and updated project docs version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bring feat/jetson-orin-nano-support up to date with origin/main after 217 commits of upstream work. Conflict resolutions: - scripts/setup-jetson.sh: adopt main's L4T-version-aware structure (NVIDIA#1910, NVIDIA#1913, NVIDIA#2419) — JP6 / JP7-R38 / JP7-R39 case dispatch with idempotent br_netfilter persistence. Drops the PR's preflight wrapper in favor of the simpler main rewrite that already covers Orin Nano via the JP6 path. - src/lib/onboard.ts: keep main's verifyWebSearchInsideSandbox; preserve the Jetson GPU-detection branch on top of main's refined NVIDIA detection format; restore the gpu parameter on startGatewayWithOptions so the patchGatewayImageForJetson() call still type-checks. - src/lib/onboard-providers.ts: re-add the ollama-local / vllm-local branches in getSandboxInferenceConfig that override inferenceBaseUrl via getLocalProviderBaseUrl. - docs/get-started/quickstart.md and the autogenerated skills commands.md: take main (Jetson now lives in ci/platform-matrix.json). Port setup-jetson into main's oclif command-registry architecture: - Add SetupJetsonCliCommand thin oclif adapter and runSetupJetsonAction that shells out to scripts/setup-jetson.sh under sudo on Linux. - Register setup-jetson in command-registry.ts (Getting Started), legacy-oclif-dispatch.ts, and oclif-commands.ts. Switch patchGatewayImageForJetson to docker.dockerInspectFormat / docker.dockerRun so it stays inside main's docker-abstraction guard, and update the existing jetson tests to match the argv-based runner contract. Fix the Jetson note in ci/platform-matrix.json — the relative ../reference/commands.md#nemoclaw-setup-jetson link broke the markdown-links CI check when the platform-matrix generator copied it into root README.md. Replaced with the docs.nvidia.com absolute URL. Allowlist two pre-existing slack-app-token false positives surfaced by the local pre-commit gitleaks pass. Verified: npm run build:cli, npm run lint, full vitest suite (3160 pass, 13 skip), and check-docs.sh markdown-link check all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
R39 and later BSP versions do not require any host customization. Detect the L4T release number arithmetically so future versions are handled automatically without needing script changes.
Summary
Related Issue
Changes
Type of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
nemoclaw-contributor-update-docsagent skill to draft changes while complying with the style guide. For example, prompt your agent with "/nemoclaw-contributor-update-docscatch up the docs for the new changes I made in this PR."Signed-off-by: Your Name your-email@example.com
Summary by CodeRabbit