fix(install): update SCRIPT_DIR after clone and guard usage-notice#1387
Closed
latenighthackathon wants to merge 1 commit intoNVIDIA:mainfrom
Closed
fix(install): update SCRIPT_DIR after clone and guard usage-notice#1387latenighthackathon wants to merge 1 commit intoNVIDIA:mainfrom
latenighthackathon wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
When piped via `curl | bash`, BASH_SOURCE[0] is empty and $0 is `bash`, so SCRIPT_DIR resolves to the user's cwd instead of the NemoClaw source directory. After cloning, SCRIPT_DIR was never updated, causing show_usage_notice() to look for usage-notice.js in the wrong location. Update SCRIPT_DIR to the clone/source directory after the link step in both the local-source and GitHub-clone install paths. Also guard show_usage_notice() against the file not existing, since the `latest` release tag may predate the usage-notice feature. Closes NVIDIA#1373 Closes NVIDIA#1381 Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.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)
📝 WalkthroughWalkthroughThe installer script now correctly resolves paths to bundled resources. Added existence checks for the usage notice file and sets Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
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
1 participant
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
SCRIPT_DIRto the cloned source directory after the link step in both local-source and GitHub-clone install pathsshow_usage_notice()againstusage-notice.jsnot existing in the cloned sourceRoot Cause
When piped via
curl | bash,BASH_SOURCE[0]is empty and$0isbash, so:SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)"resolves to the user's cwd (e.g.
/home/u), not the NemoClaw source. After cloning to~/.nemoclaw/source,SCRIPT_DIRwas never updated, soshow_usage_notice()looked forusage-notice.jsin the wrong directory.Additionally, the
latesttag currently points tob999c0ewhich predates theusage-notice.jsaddition in #1290. Even with the correctSCRIPT_DIR, the file won't exist until the tag is moved forward.Test plan
Tested in Docker (Ubuntu 24.04, clean install, piped via
cat install.sh | bash):Cannot find module '/home/testuser/bin/lib/usage-notice.js'(wrong path)latesttagnemoclaw onboardsuccessfullyCloses #1373
Closes #1381
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit