fix(install): improve error handling and output control
- Add set -E flag to propagate ERR traps through subshells - Fix cleanup script execution to avoid inheriting parent shell flags - Remove invalid --noconfirm flag from pacstrap (auto-enabled by default) - Fix spinner output leaking by removing tee redirection - Add --noconfirm to all pacman -Sy commands for consistency - Add || true to pgrep in cleanup to handle no-process-found case
- commit
- 04812b147d1fbf9676b1de685fcc66359e97f44c
- author
- bnema · · 2025-10-21 16:16 UTC
- committer
- bnema · · 2025-10-21 21:14 UTC
- parent
- 740acb2
- signature
- unsigned
8 files · +16 -10
M CHANGELOG.md +3 -0
| @@ -18,6 +18,9 @@ All notable changes to this project will be documented in this file. | |||
| 18 | 18 | ||
| 19 | 19 | ### Fixed | |
| 20 | 20 | - **Help flag**: Moved --help check before any side effects for proper argument parsing | |
| 21 | + | - **Error handling**: Fixed ERR trap propagation with set -E and proper cleanup execution | |
| 22 | + | - **Spinner output**: Fixed package installation output leaking through spinner display | |
| 23 | + | - **Cleanup flag**: Fixed --cleanup flag triggering false errors from pgrep exit codes | |
| 21 | 24 | ||
| 22 | 25 | ## [0.2.0] - 2025-10-21 | |
| 23 | 26 | ||
| @@ -18,6 +18,9 @@ All notable changes to this project will be documented in this file. | |||||
| 18 | 18 | ||||
| 19 | ### Fixed | 19 | ### Fixed | ||
| 20 | - **Help flag**: Moved --help check before any side effects for proper argument parsing | 20 | - **Help flag**: Moved --help check before any side effects for proper argument parsing | ||
| 21 | + | - **Error handling**: Fixed ERR trap propagation with set -E and proper cleanup execution | |||
| 22 | + | - **Spinner output**: Fixed package installation output leaking through spinner display | |||
| 23 | + | - **Cleanup flag**: Fixed --cleanup flag triggering false errors from pgrep exit codes | |||
| 21 | 24 | ||||
| 22 | ## [0.2.0] - 2025-10-21 | 25 | ## [0.2.0] - 2025-10-21 | ||
| 23 | 26 | ||||
M install.sh +3 -2
| @@ -2,7 +2,7 @@ | |||
| 2 | 2 | # ArchUp - Fast, minimal Arch Linux auto-installer | |
| 3 | 3 | # Main installation orchestrator | |
| 4 | 4 | ||
| 5 | - | set -eo pipefail | |
| 5 | + | set -Eeo pipefail | |
| 6 | 6 | ||
| 7 | 7 | # Handle help flag FIRST - before any side effects | |
| 8 | 8 | if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then | |
| @@ -70,7 +70,8 @@ source "$ARCHUP_INSTALL/helpers/all.sh" | |||
| 70 | 70 | # Handle cleanup flag (after helpers are available) | |
| 71 | 71 | if [ "$1" = "--cleanup" ]; then | |
| 72 | 72 | echo "Running cleanup before installation..." | |
| 73 | - | source "$ARCHUP_INSTALL/helpers/cleanup.sh" default | |
| 73 | + | # Run cleanup in subshell to avoid inheriting set -Eeo pipefail | |
| 74 | + | bash "$ARCHUP_INSTALL/helpers/cleanup.sh" default | |
| 74 | 75 | fi | |
| 75 | 76 | ||
| 76 | 77 | # Display logo | |
| @@ -2,7 +2,7 @@ | |||||
| 2 | # ArchUp - Fast, minimal Arch Linux auto-installer | 2 | # ArchUp - Fast, minimal Arch Linux auto-installer | ||
| 3 | # Main installation orchestrator | 3 | # Main installation orchestrator | ||
| 4 | 4 | ||||
| 5 | - | set -eo pipefail | 5 | + | set -Eeo pipefail |
| 6 | 6 | ||||
| 7 | # Handle help flag FIRST - before any side effects | 7 | # Handle help flag FIRST - before any side effects | ||
| 8 | if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then | 8 | if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then | ||
| @@ -70,7 +70,8 @@ source "$ARCHUP_INSTALL/helpers/all.sh" | |||||
| 70 | # Handle cleanup flag (after helpers are available) | 70 | # Handle cleanup flag (after helpers are available) | ||
| 71 | if [ "$1" = "--cleanup" ]; then | 71 | if [ "$1" = "--cleanup" ]; then | ||
| 72 | echo "Running cleanup before installation..." | 72 | echo "Running cleanup before installation..." | ||
| 73 | - | source "$ARCHUP_INSTALL/helpers/cleanup.sh" default | 73 | + | # Run cleanup in subshell to avoid inheriting set -Eeo pipefail |
| 74 | + | bash "$ARCHUP_INSTALL/helpers/cleanup.sh" default | |||
| 74 | fi | 75 | fi | ||
| 75 | 76 | ||||
| 76 | # Display logo | 77 | # Display logo | ||
M install/base/enable-multilib.sh +1 -1
| @@ -7,7 +7,7 @@ gum style --foreground 6 --padding "1 0 0 $PADDING_LEFT" "Enabling multilib repo | |||
| 7 | 7 | enable_multilib "/etc/pacman.conf" | |
| 8 | 8 | ||
| 9 | 9 | # Update package database | |
| 10 | - | if ! run_with_spinner "Updating package database..." "pacman -Sy"; then | |
| 10 | + | if ! run_with_spinner "Updating package database..." "pacman -Sy --noconfirm"; then | |
| 11 | 11 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Failed to update package database" | |
| 12 | 12 | exit 1 | |
| 13 | 13 | fi | |
| @@ -7,7 +7,7 @@ gum style --foreground 6 --padding "1 0 0 $PADDING_LEFT" "Enabling multilib repo | |||||
| 7 | enable_multilib "/etc/pacman.conf" | 7 | enable_multilib "/etc/pacman.conf" | ||
| 8 | 8 | ||||
| 9 | # Update package database | 9 | # Update package database | ||
| 10 | - | if ! run_with_spinner "Updating package database..." "pacman -Sy"; then | 10 | + | if ! run_with_spinner "Updating package database..." "pacman -Sy --noconfirm"; then |
| 11 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Failed to update package database" | 11 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Failed to update package database" | ||
| 12 | exit 1 | 12 | exit 1 | ||
| 13 | fi | 13 | fi | ||
M install/base/pacstrap.sh +1 -0
| @@ -23,6 +23,7 @@ if [ "$ARCHUP_ENCRYPTION" = "enabled" ]; then | |||
| 23 | 23 | fi | |
| 24 | 24 | ||
| 25 | 25 | # Install base system with spinner | |
| 26 | + | # Note: pacstrap automatically runs with --noconfirm unless -i flag is used | |
| 26 | 27 | echo "Installing ${#packages[@]} packages: ${packages[*]}" >> "$ARCHUP_INSTALL_LOG_FILE" | |
| 27 | 28 | if ! run_with_spinner "Installing base system (${#packages[@]} packages)..." "pacstrap /mnt ${packages[*]}"; then | |
| 28 | 29 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Base system installation failed" | |
| @@ -23,6 +23,7 @@ if [ "$ARCHUP_ENCRYPTION" = "enabled" ]; then | |||||
| 23 | fi | 23 | fi | ||
| 24 | 24 | ||||
| 25 | # Install base system with spinner | 25 | # Install base system with spinner | ||
| 26 | + | # Note: pacstrap automatically runs with --noconfirm unless -i flag is used | |||
| 26 | echo "Installing ${#packages[@]} packages: ${packages[*]}" >> "$ARCHUP_INSTALL_LOG_FILE" | 27 | echo "Installing ${#packages[@]} packages: ${packages[*]}" >> "$ARCHUP_INSTALL_LOG_FILE" | ||
| 27 | if ! run_with_spinner "Installing base system (${#packages[@]} packages)..." "pacstrap /mnt ${packages[*]}"; then | 28 | if ! run_with_spinner "Installing base system (${#packages[@]} packages)..." "pacstrap /mnt ${packages[*]}"; then | ||
| 28 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Base system installation failed" | 29 | gum style --foreground 1 --padding "1 0 1 $PADDING_LEFT" "[ERROR] Base system installation failed" | ||
M install/helpers/cleanup.sh +2 -1
| @@ -140,7 +140,8 @@ cleanup_processes() { | |||
| 140 | 140 | log_cleanup "Checking for stuck pacstrap/chroot processes..." | |
| 141 | 141 | ||
| 142 | 142 | # Kill any stuck pacstrap or arch-chroot processes | |
| 143 | - | local stuck_pids=$(pgrep -f "pacstrap|arch-chroot" 2>/dev/null) | |
| 143 | + | # Note: pgrep returns 1 if no processes found, which is expected/normal | |
| 144 | + | local stuck_pids=$(pgrep -f "pacstrap|arch-chroot" 2>/dev/null || true) | |
| 144 | 145 | if [ -n "$stuck_pids" ]; then | |
| 145 | 146 | log_cleanup "Killing stuck processes: $stuck_pids" | |
| 146 | 147 | kill $stuck_pids 2>/dev/null || true | |
| @@ -140,7 +140,8 @@ cleanup_processes() { | |||||
| 140 | log_cleanup "Checking for stuck pacstrap/chroot processes..." | 140 | log_cleanup "Checking for stuck pacstrap/chroot processes..." | ||
| 141 | 141 | ||||
| 142 | # Kill any stuck pacstrap or arch-chroot processes | 142 | # Kill any stuck pacstrap or arch-chroot processes | ||
| 143 | - | local stuck_pids=$(pgrep -f "pacstrap|arch-chroot" 2>/dev/null) | 143 | + | # Note: pgrep returns 1 if no processes found, which is expected/normal |
| 144 | + | local stuck_pids=$(pgrep -f "pacstrap|arch-chroot" 2>/dev/null || true) | |||
| 144 | if [ -n "$stuck_pids" ]; then | 145 | if [ -n "$stuck_pids" ]; then | ||
| 145 | log_cleanup "Killing stuck processes: $stuck_pids" | 146 | log_cleanup "Killing stuck processes: $stuck_pids" | ||
| 146 | kill $stuck_pids 2>/dev/null || true | 147 | kill $stuck_pids 2>/dev/null || true | ||
M install/helpers/logging.sh +1 -2
| @@ -52,9 +52,8 @@ run_logged() { | |||
| 52 | 52 | ||
| 53 | 53 | echo "[$(date '+%Y-%m-%d %H:%M:%S')] Starting: $script" >> "$ARCHUP_INSTALL_LOG_FILE" | |
| 54 | 54 | ||
| 55 | - | # Don't start/stop log monitor here - it runs continuously once started | |
| 56 | 55 | # Source helpers in subshell before running the script so functions are available | |
| 57 | - | # Redirect output to log but allow gum to access terminal for display | |
| 56 | + | # Redirect output to log file only | |
| 58 | 57 | bash -c "source '$ARCHUP_INSTALL/helpers/all.sh' && source '$script'" >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | |
| 59 | 58 | ||
| 60 | 59 | local exit_code=$? | |
| @@ -52,9 +52,8 @@ run_logged() { | |||||
| 52 | 52 | ||||
| 53 | echo "[$(date '+%Y-%m-%d %H:%M:%S')] Starting: $script" >> "$ARCHUP_INSTALL_LOG_FILE" | 53 | echo "[$(date '+%Y-%m-%d %H:%M:%S')] Starting: $script" >> "$ARCHUP_INSTALL_LOG_FILE" | ||
| 54 | 54 | ||||
| 55 | - | # Don't start/stop log monitor here - it runs continuously once started | |||
| 56 | # Source helpers in subshell before running the script so functions are available | 55 | # Source helpers in subshell before running the script so functions are available | ||
| 57 | - | # Redirect output to log but allow gum to access terminal for display | 56 | + | # Redirect output to log file only |
| 58 | bash -c "source '$ARCHUP_INSTALL/helpers/all.sh' && source '$script'" >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | 57 | bash -c "source '$ARCHUP_INSTALL/helpers/all.sh' && source '$script'" >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | ||
| 59 | 58 | ||||
| 60 | local exit_code=$? | 59 | local exit_code=$? | ||
M install/helpers/presentation.sh +4 -3
| @@ -81,9 +81,10 @@ run_with_spinner() { | |||
| 81 | 81 | local title="$1" | |
| 82 | 82 | local command="$2" | |
| 83 | 83 | ||
| 84 | - | # Redirect only stderr to terminal for gum display | |
| 85 | - | # Command output goes to log file, spinner animation displays on terminal | |
| 86 | - | if gum spin --spinner dot --title "$title" -- bash -c "$command >> \"$ARCHUP_INSTALL_LOG_FILE\" 2>&1" 2>/dev/tty; then | |
| 84 | + | # Run command with spinner, output goes only to log file | |
| 85 | + | # If command fails, error will be caught by ERR trap and shown in error handler | |
| 86 | + | if gum spin --spinner dot --title "$title" -- \ | |
| 87 | + | bash -c "$command >> \"$ARCHUP_INSTALL_LOG_FILE\" 2>&1"; then | |
| 87 | 88 | return 0 | |
| 88 | 89 | else | |
| 89 | 90 | return 1 | |
| @@ -81,9 +81,10 @@ run_with_spinner() { | |||||
| 81 | local title="$1" | 81 | local title="$1" | ||
| 82 | local command="$2" | 82 | local command="$2" | ||
| 83 | 83 | ||||
| 84 | - | # Redirect only stderr to terminal for gum display | 84 | + | # Run command with spinner, output goes only to log file |
| 85 | - | # Command output goes to log file, spinner animation displays on terminal | 85 | + | # If command fails, error will be caught by ERR trap and shown in error handler |
| 86 | - | if gum spin --spinner dot --title "$title" -- bash -c "$command >> \"$ARCHUP_INSTALL_LOG_FILE\" 2>&1" 2>/dev/tty; then | 86 | + | if gum spin --spinner dot --title "$title" -- \ |
| 87 | + | bash -c "$command >> \"$ARCHUP_INSTALL_LOG_FILE\" 2>&1"; then | |||
| 87 | return 0 | 88 | return 0 | ||
| 88 | else | 89 | else | ||
| 89 | return 1 | 90 | return 1 | ||
M install/repos/chaotic.sh +1 -1
| @@ -27,7 +27,7 @@ EOF | |||
| 27 | 27 | fi | |
| 28 | 28 | ||
| 29 | 29 | # Update package databases | |
| 30 | - | arch-chroot /mnt pacman -Sy >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | |
| 30 | + | arch-chroot /mnt pacman -Sy --noconfirm >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | |
| 31 | 31 | ||
| 32 | 32 | # Verify Chaotic-AUR is working | |
| 33 | 33 | if arch-chroot /mnt pacman -Sl chaotic-aur >/dev/null 2>&1; then | |
| @@ -27,7 +27,7 @@ EOF | |||||
| 27 | fi | 27 | fi | ||
| 28 | 28 | ||||
| 29 | # Update package databases | 29 | # Update package databases | ||
| 30 | - | arch-chroot /mnt pacman -Sy >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 | 30 | + | arch-chroot /mnt pacman -Sy --noconfirm >> "$ARCHUP_INSTALL_LOG_FILE" 2>&1 |
| 31 | 31 | ||||
| 32 | # Verify Chaotic-AUR is working | 32 | # Verify Chaotic-AUR is working | ||
| 33 | if arch-chroot /mnt pacman -Sl chaotic-aur >/dev/null 2>&1; then | 33 | if arch-chroot /mnt pacman -Sl chaotic-aur >/dev/null 2>&1; then | ||