fix: add install script with platform detection #63
Conversation
WalkthroughAdds a platform-aware Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant install.sh as Installer
participant FS as Filesystem
participant Shell as Shell/PATH
participant Git as git-gtr
Note over User,Installer: user runs ./install.sh
User->>Installer: execute script
Installer->>Installer: detect platform & select target dir
Installer->>FS: check script-local git-gtr binary
alt binary missing
Installer->>User: error & exit
else binary present
Installer->>FS: ensure target dir exists (mkdir / sudo if needed)
Installer->>FS: check existing target path
alt existing symlink -> same target
Installer->>User: report already installed
else existing symlink -> different
Installer->>FS: replace symlink (warn)
else non-symlink file
Installer->>User: error & exit
end
Installer->>FS: create symlink (may invoke sudo)
Installer->>Shell: verify PATH contains target dir
Installer->>Git: run `git gtr version` to validate
alt success
Installer->>User: success message + usage hints
else failure
Installer->>User: warning + PATH remediation steps
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
install.sh (2)
60-82: Remove unusedplatformparameter.The
platformparameter is declared but never used in the function body. The installation directory selection logic doesn't vary by platform, so this parameter is unnecessary.🔎 Proposed fix
# Find the best installation directory find_install_dir() { - local platform="$1" - # Option 1: Homebrew bin (if Homebrew is installed) if command -v brew >/dev/null 2>&1; thenAlso update the caller at line 109:
- install_dir="$(find_install_dir "$platform")" + install_dir="$(find_install_dir)"
156-165: Consider detecting the user's shell for targeted guidance.The script assumes zsh or bash by suggesting
~/.zshrcor~/.bashrc. While these cover the majority of users, you could detect the current shell via$SHELLand provide shell-specific instructions (e.g.,~/.config/fish/config.fishfor Fish users).🔎 Example enhancement
if ! in_path "$install_dir"; then log_warn "$install_dir is not in your PATH" echo local shell_config case "$SHELL" in */zsh) shell_config="~/.zshrc" ;; */bash) shell_config="~/.bashrc" ;; */fish) shell_config="~/.config/fish/config.fish" echo "Add this to $shell_config:" echo " set -gx PATH \"$install_dir\" \$PATH" return ;; *) shell_config="your shell configuration file (~/.zshrc or ~/.bashrc)" ;; esac echo "Add this to $shell_config:" echo echo " export PATH=\"$install_dir:\$PATH\"" echo echo "Then restart your terminal or run: source $shell_config" echo fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.mdinstall.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for all shell scripts
Use snake_case naming for all functions and local variables in Bash scripts
Use UPPER_CASE naming for constants and environment variables in Bash scripts
Use 2-space indentation (no tabs) in all Bash scripts
Always quote variables and paths in Bash scripts (e.g., use"$var"not$var)
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Maintain POSIX-compatible Bash patterns targeting Bash 3.2+ compatibility
Git worktree operations must work cross-platform on macOS, Linux, and Windows Git Bash
All code must only use git and basic POSIX shell utilities - no external dependencies
Useset -efor error handling in Bash scripts where appropriate
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (7)
install.sh (4)
1-15: LGTM! Clean setup following shell script guidelines.The shebang, error handling with
set -e, and UPPER_CASE constants all follow the coding guidelines correctly.
85-88: LGTM! Clean permission check.The function correctly uses POSIX-compatible
[ ]tests and proper variable quoting.
115-135: LGTM! Robust symlink handling.The code correctly handles existing symlinks, uses
|| trueto prevent premature exit withset -e, and appropriately uses sudo when needed. The distinction between symlinks and regular files is properly managed.
170-197: LGTM! Thorough installation verification with helpful guidance.The function properly checks both PATH availability and functional operation of
git gtr, provides clear success/warning messages, and offers actionable next steps. Variable usage and quoting follow guidelines correctly.README.md (3)
24-57: LGTM! Clear installation instructions with proper shell practices.The documentation correctly references the new
install.shscript and provides platform-specific manual installation options. All command examples use proper quoting ("$(pwd)/bin/git-gtr") and the PATH export correctly escapes$PATH. The collapsible manual options section is a nice UX touch.
500-504: LGTM! Clear hook timing documentation.The updated table clearly explains when each hook executes and typical use cases. The formatting is clean and easy to read.
723-747: LGTM! Comprehensive troubleshooting guidance with correct shell commands.The troubleshooting section provides practical verification steps that follow shell best practices:
- Proper use of
2>/dev/nullfor silent fallbacks- Clean PATH inspection with
tr ':' '\n'- PATH export guidance matches the install.sh script
The debugging workflow is logical and should help users quickly identify installation issues.
…platform parameter
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
install.sh (2)
36-39: Consider usinggrep -Ffor literal path matching.The current
grep -qxtreats the path as a regex pattern. If a directory path contains special regex characters, it could cause false matches or failures. Usegrep -Fqxfor fixed/literal string matching.🔎 Proposed fix
in_path() { - echo "$PATH" | tr ':' '\n' | grep -qx "$1" + echo "$PATH" | tr ':' '\n' | grep -Fqx "$1" }
89-111: LGTM! Clean initial setup logic.The validation, platform detection, and directory selection logic are sound. All variables use proper scoping and quoting.
Optional: Consider checking executability.
Line 94 could use
-xin addition to-fto verify thatgit-gtris executable, catching configuration issues earlier.🔎 Optional improvement
- if [ ! -f "$GIT_GTR_PATH" ]; then + if [ ! -f "$GIT_GTR_PATH" ] || [ ! -x "$GIT_GTR_PATH" ]; then log_error "Could not find git-gtr at: $GIT_GTR_PATH" + log_error "Make sure the file exists and is executable." log_error "Please run this script from the git-worktree-runner directory." exit 1 fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
install.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for all shell scripts
Use snake_case naming for all functions and local variables in Bash scripts
Use UPPER_CASE naming for constants and environment variables in Bash scripts
Use 2-space indentation (no tabs) in all Bash scripts
Always quote variables and paths in Bash scripts (e.g., use"$var"not$var)
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Maintain POSIX-compatible Bash patterns targeting Bash 3.2+ compatibility
Git worktree operations must work cross-platform on macOS, Linux, and Windows Git Bash
All code must only use git and basic POSIX shell utilities - no external dependencies
Useset -efor error handling in Bash scripts where appropriate
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (9)
install.sh (9)
1-18: LGTM! Excellent setup.The shebang, error handling, color constants, and path detection follow all coding guidelines correctly. Variables are properly quoted, and
${BASH_SOURCE[0]}is the right approach for locating the script directory.
20-34: LGTM! Clean logging abstraction.The logging functions follow snake_case naming conventions and properly quote all variables. The color-coded output will improve user experience.
41-57: LGTM! Comprehensive platform detection.The platform detection correctly handles macOS, Linux, and Windows (Git Bash environments: MINGW/MSYS/CYGWIN), with a sensible fallback for unknown platforms.
59-80: LGTM! Well-designed installation directory selection.The priority order (Homebrew →
/usr/local/bin→~/bin) is sensible, and the logic properly validates directory existence and permissions before selection. The fallback to~/binensures installation can proceed without elevated privileges in most cases.
82-86: LGTM! Correct sudo detection logic.The function correctly identifies when elevated privileges are needed by checking if the directory exists but is not writable. Proper use of local variables and quoting.
112-133: LGTM! Robust handling of existing installations.The logic correctly handles three scenarios: symlink to correct target (already installed), symlink to wrong target (replace), and non-symlink file (error with clear guidance). Proper use of
|| trueforreadlinkerror handling withset -eactive, and conditionalsudousage.
152-166: LGTM! Clear PATH configuration guidance.The PATH check and user instructions are well-designed. The
\$PATHis properly escaped in the echo statement so users can copy-paste the export command directly. Good UX to callverify_installationat the end.
168-195: LGTM! Comprehensive installation verification.The verification logic properly checks both
git-gtravailability andgit gtrcommand execution, with helpful success messages and troubleshooting guidance. Good use ofcommand -vfor POSIX compatibility.
197-197: LGTM! Proper script execution.Correctly invokes the main function with properly quoted arguments.
| # Create directory if needed | ||
| if [ ! -d "$install_dir" ]; then | ||
| log_info "Creating directory: $install_dir" | ||
| mkdir -p "$install_dir" | ||
| fi | ||
|
|
||
| # Create symlink | ||
| if needs_sudo "$install_dir"; then | ||
| log_info "Creating symlink (requires sudo)..." | ||
| sudo ln -s "$GIT_GTR_PATH" "$symlink_path" | ||
| else | ||
| log_info "Creating symlink..." | ||
| ln -s "$GIT_GTR_PATH" "$symlink_path" | ||
| fi | ||
|
|
||
| log_success "Symlink created: $symlink_path -> $GIT_GTR_PATH" | ||
| echo |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's run the test script to check platform capabilities
#!/bin/bash
echo "=== Platform Information ==="
uname -s
echo
echo "=== Checking sudo availability ==="
if command -v sudo >/dev/null 2>&1; then
echo "sudo: available"
sudo -V | head -n1
else
echo "sudo: NOT available"
fi
echo
echo "=== Checking symlink support ==="
test_dir=$(mktemp -d)
test_file="$test_dir/test_target"
test_link="$test_dir/test_link"
echo "test" > "$test_file"
if ln -s "$test_file" "$test_link" 2>/dev/null; then
echo "Symlink creation: SUCCESS"
if [ -L "$test_link" ]; then
echo "Symlink verification: SUCCESS"
target=$(readlink "$test_link" 2>/dev/null || echo "readlink failed")
echo "readlink result: $target"
else
echo "Symlink verification: FAILED"
fi
else
echo "Symlink creation: FAILED"
fi
rm -rf "$test_dir"Repository: coderabbitai/git-worktree-runner
Length of output: 305
🏁 Script executed:
# Locate and examine install.sh
fd -t f "install.sh" | head -5Repository: coderabbitai/git-worktree-runner
Length of output: 86
🏁 Script executed:
# Find the needs_sudo function in install.sh
rg -n "needs_sudo" --type sh -A 5 -B 1Repository: coderabbitai/git-worktree-runner
Length of output: 942
🏁 Script executed:
# Get context around lines 135-151 in install.sh
fd -t f "install.sh" -x wc -l {} \; 2>/dev/null | head -1Repository: coderabbitai/git-worktree-runner
Length of output: 92
🏁 Script executed:
# Read the install.sh file to examine the needs_sudo function and context
fd -t f "install.sh" -x cat -n {} \; 2>/dev/null | sed -n '110,160p'Repository: coderabbitai/git-worktree-runner
Length of output: 1995
🏁 Script executed:
# Check find_install_dir function to understand how install_dir is determined
rg -n "find_install_dir" --type sh -A 20 -B 2Repository: coderabbitai/git-worktree-runner
Length of output: 2074
🏁 Script executed:
# Look for Windows-specific handling or documentation
rg -i "windows|msys|git.bash|cygwin" --type sh --type mdRepository: coderabbitai/git-worktree-runner
Length of output: 1534
🏁 Script executed:
# Check if there's a README or docs mentioning platform support
fd -t f "README|CONTRIBUTING|INSTALL|docs" -i | head -10Repository: coderabbitai/git-worktree-runner
Length of output: 112
🏁 Script executed:
# Examine what GIT_GTR_PATH is and how install_dir is constructed
rg -n "GIT_GTR_PATH|symlink_path|install_dir" --type sh -B 1 | head -40Repository: coderabbitai/git-worktree-runner
Length of output: 1757
Critical Windows Git Bash compatibility issue: symlink creation and sudo handling will fail.
The install script lacks Windows-specific handling for ln -s and sudo, despite Windows Git Bash being explicitly listed as a supported platform in README.md.
Specific problems:
sudocommand does not exist in Windows Git Bash; whenneeds_sudoreturns true,sudo ln -swill fail with "command not found"ln -son Windows Git Bash requires Developer Mode or admin privileges; without them, it fails silentlymkdir -pat line 138 attempts directory creation without sudo, which will fail if installing to a system directory without admin rights- No error handling or platform detection to guide users on Windows
The code should either detect Windows and provide platform-specific installation instructions, skip symlink creation on Windows with alternative PATH setup guidance, or add explicit error handling for symlink creation failures with helpful messages for Windows users.
🤖 Prompt for AI Agents
In install.sh around lines 135-151, the symlink and sudo logic is incompatible
with Windows Git Bash and lacks error handling; update the script to detect
Windows/MSYS/CYGWIN (via uname or $MSYSTEM), branch: on Windows avoid using sudo
and skip ln -s (or attempt ln -s but check for failure), on non-Windows use
needs_sudo to run mkdir/symlink under sudo when required; ensure mkdir uses sudo
when installing to protected dirs, wrap ln -s and mkdir calls with immediate
exit-on-error checks and user-friendly messages that explain Windows
alternatives (add PATH update instructions or suggest running with
Administrator/Developer Mode or copying the binary), and log actionable guidance
when symlink creation fails on Windows instead of letting the commands silently
fail.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.