fix: error when install path does not exist#2982
fix: error when install path does not exist#2982waveywaves wants to merge 1 commit intochainloop-dev:mainfrom
Conversation
Add install.sh to the main repository (previously only in the now-archived chainloop-dev/docs repo) with the following improvements: 1. Add early validation after argument parsing to verify the --path installation directory exists before downloading anything. Provides a clear error message with a suggested fix (mkdir -p) instead of failing silently or showing confusing sudo errors. 2. Improve the install step to provide proper error output when the binary installation fails, instead of silently redirecting stderr to /dev/null. Fixes chainloop-dev#2977 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
7709d56 to
47e3f9b
Compare
There was a problem hiding this comment.
3 issues found across 1 file
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="install.sh">
<violation number="1" location="install.sh:58">
P2: `validate_checksums_file` changes CWD with `cd "$1"` and never restores it. The script later runs `rm -r ${TMP_DIR}` while still inside TMP_DIR, which fails and causes the script to exit under `set -e`, leaving the temp directory behind.</violation>
<violation number="2" location="install.sh:85">
P2: `--help` prints usage but does not exit, so the installer proceeds with downloads and installation instead of stopping.</violation>
<violation number="3" location="install.sh:92">
P2: `--path` does not validate a following argument, so with `set -u` the script exits on an unbound `$1` when `--path` is the final CLI argument.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| # checksums.txt file validation | ||
| # example: check_sha256 "${TMP_DIR}" checksums.txt | ||
| validate_checksums_file() { | ||
| cd "$1" |
There was a problem hiding this comment.
P2: validate_checksums_file changes CWD with cd "$1" and never restores it. The script later runs rm -r ${TMP_DIR} while still inside TMP_DIR, which fails and causes the script to exit under set -e, leaving the temp directory behind.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 58:
<comment>`validate_checksums_file` changes CWD with `cd "$1"` and never restores it. The script later runs `rm -r ${TMP_DIR}` while still inside TMP_DIR, which fails and causes the script to exit under `set -e`, leaving the temp directory behind.</comment>
<file context>
@@ -0,0 +1,225 @@
+# checksums.txt file validation
+# example: check_sha256 "${TMP_DIR}" checksums.txt
+validate_checksums_file() {
+ cd "$1"
+ if is_command sha256sum; then
+ sha256sum --ignore-missing --quiet --check "$2"
</file context>
| ;; | ||
| '--path') | ||
| shift | ||
| INSTALL_PATH=$1 |
There was a problem hiding this comment.
P2: --path does not validate a following argument, so with set -u the script exits on an unbound $1 when --path is the final CLI argument.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 92:
<comment>`--path` does not validate a following argument, so with `set -u` the script exits on an unbound `$1` when `--path` is the final CLI argument.</comment>
<file context>
@@ -0,0 +1,225 @@
+ ;;
+ '--path')
+ shift
+ INSTALL_PATH=$1
+ ;;
+ *)
</file context>
| ;; | ||
| '--help' | -h) | ||
| shift | ||
| print_help |
There was a problem hiding this comment.
P2: --help prints usage but does not exit, so the installer proceeds with downloads and installation instead of stopping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 85:
<comment>`--help` prints usage but does not exit, so the installer proceeds with downloads and installation instead of stopping.</comment>
<file context>
@@ -0,0 +1,225 @@
+ ;;
+ '--help' | -h)
+ shift
+ print_help
+ ;;
+ '--force-verification')
</file context>
Summary
install.shto the main repository (migrated from the now-archivedchainloop-dev/docsrepo atstatic/install.sh) with two bug fixes:--pathinstallation directory exists before downloading anything, with a clear error message and actionable suggestion (mkdir -p)2>/dev/nullstderr suppression in the install step with proper error handling and user-facing messagesNote: The install script was previously hosted only in the archived
chainloop-dev/docsrepo and served viadl.chainloop.dev. The CDN/deployment pipeline will need to be updated to serve this script from the new location.Fixes #2977
Test plan
bash install.sh --path /nonexistent/pathand verify it exits early with:The installation path '/nonexistent/path' does not exist. Please create it first with: mkdir -p /nonexistent/pathbash install.sh --path /tmp/test-install(aftermkdir -p /tmp/test-install) and verify normal installation succeedsbash install.shwith default path (/usr/local/bin) and verify existing behavior is preserved🤖 Generated with Claude Code