• 1

• 87.5k
• 14
• 104
• 323

Don't use non-portable terminal escape codes

The tput program exists for exactly this purpose:

red=$(tput setaf 1)
green=$(tput setaf 2)
yellow=$(tput setaf 3)
nocolor=$(tput sgr0)

(I've downcased your variable names, to avoid surprising conflicts with environment variables)

#Inline the single use functions show_success and show_error_and_exit are laudable, and do help readability, but they are only used once each, and may be clearer if written directly into error_handler. Either way, their output really ought to go to the error stream: >&2.

#Reduce duplication Look at this pattern:

echo -e "\n${GREEN}Step 1: configure packages${NOCOLOR}"
echo -e "${YELLOW}dpkg --configure -a${NOCOLOR}"
sudo dpkg --configure -a
error_handler $? 1

This structure is repeated throughout the program, but it can be refactored. Let's write the variable parts using variables:

echo "${green}Step ${step}: ${description}${nocolor}"
echo "${yellow}${command}${nocolor}"
$command
error_handler $? $step

There's three things we need: a step number, description, and the command to run. The command needs to be an array; that's easy to arrange by passing it as all the arguments after description. We can also inline the error_handler:

do_action()
{
 step=1ドル; shift
 description=1ドル; shift
 printf "${green}Step %s: %s\n" "$step" "$description"
 printf "${yellow}"
 printf '%q ' "$@"
 printf "${nocolor}\n"
 if "$@"
 then
 printf "${green}Step %s: SUCCESS${nocolor}\n" "$step"
 else
 err=$?
 printf "${red}Step %s: FAILED${nocolor}\n" "$step" >&2
 exit $err
 fi
}

This function is then used as:

i=0
do_action $((++i)) "configure packages" dpkg --configure -a
do_action $((++i)) "fix broken dependencies" apt-get install --fix-broken
do_action $((++i)) "update cache" apt-get update
do_action $((++i)) "upgrade packages" apt-get upgrade
do_action $((++i)) "upgrade distribution" apt-get dist-upgrade
do_action $((++i)) "remove unused packages" apt-get --purge autoremove
do_action $((++i)) "clean up" apt-get autoclean

You might choose to make the step number a global variable and increment it inside the function; I haven't done that, because global variables tend to proliferate and make the program hard to reason about.

#Don't sudo in a script Many calls to sudo is a sign that the entire script needs superuser capabilities, so it's better if the user runs the script as root. You could automate this (though I would personally just error out):

test $EUID = 0 || exec sudo 0ドル "$@"

#Full program #!/bin/bash set -e -u

red=$(tput setaf 1)
green=$(tput setaf 2)
yellow=$(tput setaf 3)
nocolor=$(tput sgr0)
do_action()
{
 step=1ドル; shift
 description=1ドル; shift
 printf "${green}Step %s: %s\n" "$step" "$description"
 printf "${yellow}"
 printf '%q ' "$@"
 printf "${nocolor}\n"
 if "$@"
 then
 printf "${green}Step %s: SUCCESS${nocolor}\n" "$step"
 else
 err=$?
 printf "${red}Step %s: FAILED${nocolor}\n" "$step" >&2
 exit $err
 fi
}
i=0
do_action $((++i)) "configure packages" dpkg --configure -a
do_action $((++i)) "fix broken dependencies" apt-get install --fix-broken
do_action $((++i)) "update cache" apt-get update
do_action $((++i)) "upgrade packages" apt-get upgrade
do_action $((++i)) "upgrade distribution" apt-get dist-upgrade
do_action $((++i)) "remove unused packages" apt-get --purge autoremove
do_action $((++i)) "clean up" apt-get autoclean