5
\$\begingroup\$

One year ago, I asked for a review of Simple Linux upgrade script in Bash.

Conditions are slightly changed, and/or more accurate:

  1. Code readability for the code user, making it simple to adjust the script's behavior

  2. Output readability for the script user, supposing it will run always in interactive form (this is sort of contradicting the original post)

  3. Colored headings describing what exactly is the script doing

  4. Attempt to correct things; well this one is hard to make, but let's suppose the two initial commands will do the trick

  5. Clean-up old packages after successful update

I hereby add:

  1. It must be a portable POSIX shell script

  2. It must have a clear typed-in header for users to distinguish between different computers, if running this script on several machines. It could be a hostname, but I decided otherwise for personal purposes (I myself run it on 3 machines in sequence with SSH public key authentication)

  3. It must, in the end, check if the Linux it is running on, is Mint. If so, it needs to offer a kernel upgrade (I chose the safest way through MintUpdate) - ask if the user wants to run it

  4. I want it to pass ShellCheck.net without any warning


#!/bin/sh

red="\\033[1;31m"
green="\\033[1;32m"
yellow="\\033[1;33m"
blue="\\033[1;34m"
nocolor="\\033[0m"

print_newline()
{
    printf "\\n"
}

show_success()
{
    echo "${green}Success.${nocolor}"
    print_newline
}

show_error_and_exit()
{
    echo "${red}An error occured.${nocolor}"
    print_newline
    exit "$1"
}

error_handler()
{
    if [ "$1" -ne 0 ]
    then
        show_error_and_exit "$2"
    else
        show_success
    fi
}

ask_for_sudo_password()
{
    sudo sh -c ":"
}

is_it_linux_mint()
{
    return $(grep -i mint /etc/lsb-release > /dev/null 2>&1)
}

print_newline

echo "${blue}--------------------${nocolor}"
echo "${blue}Vlasta - Laptop Mint${nocolor}"
echo "${blue}--------------------${nocolor}"

print_newline

echo "${green}Step 0: enter password${nocolor}"

# in order to print information AFTER sudo password has been provided
ask_for_sudo_password

error_handler "$?" 1

echo "${green}Step 1: configure packages${nocolor}"
echo "${yellow}dpkg --configure -a${nocolor}"

sudo dpkg --configure -a

error_handler "$?" 2


echo "${green}Step 2: fix broken dependencies${nocolor}"
echo "${yellow}apt-get install --fix-broken${nocolor}"

sudo apt-get install --fix-broken

error_handler "$?" 3


echo "${green}Step 3: update cache${nocolor}"
echo "${yellow}apt-get update${nocolor}"

sudo apt-get update

error_handler "$?" 4


echo "${green}Step 4: upgrade packages${nocolor}"
echo "${yellow}apt-get upgrade${nocolor}"

sudo apt-get upgrade

error_handler "$?" 5


echo "${green}Step 5: upgrade packages with possible removals${nocolor}"
echo "${yellow}apt-get dist-upgrade${nocolor}"

sudo apt-get dist-upgrade

error_handler "$?" 6


echo "${green}Step 6: remove unused packages${nocolor}"
echo "${yellow}apt-get --purge autoremove${nocolor}"

sudo apt-get --purge autoremove

error_handler "$?" 7


echo "${green}Step 7: clean up${nocolor}"
echo "${yellow}apt-get autoclean${nocolor}"

sudo apt-get autoclean

error_handler "$?" 8

if is_it_linux_mint
then

    while true
    do
        echo "Do you want to check for kernel upgrade via MintUpdate?"
        read -r answer
        case "$answer" in
            [Yy]) :   ; break;;
            [Nn]) exit; break;;
               *) echo "Please answer Y or N.";;
        esac
    done

    echo "${green}Step 8: Linux Mint kernel upgrade${nocolor}"
    echo "${yellow}mintupdate${nocolor}"

    sudo /usr/bin/python3 /usr/lib/linuxmint/mintUpdate/mintUpdate.py show

fi
\$\endgroup\$
6
\$\begingroup\$

2018/04/24 edit

I missed an obvious bug: is_linux_mint is not implemented correctly. The results of a return on an empty output of a command substitution, with or without quotes, is implementation-defined:

The value of the special parameter '?' shall be set to n, an unsigned decimal integer, or to the exit status of the last command executed if n is not specified. If n is not an unsigned decimal integer, or is greater than 255, the results are unspecified.

The fix is to remove the return and dollar-parentheses:

is_linux_mint()
{
    grep -i mint /etc/lsb-release >/dev/null 2>&1
}

#!/bin/sh

printc()
{
    fd=1
    color=0 # Black
    case "$1" in
        -info)
            color=4 # Blue
            ;;
        -ok)
            color=2 # Green
            ;;
        -warning)
            fd=2
            color=3 # Yellow
            ;;
        -error)
            fd=2
            color=1 # Red
            ;;
        *)
            ;;
    esac
    [ "$color" -ne 0 ] && shift
    printf '%s\n' "$(tput bold)$(tput setaf "$color")$@$(tput sgr0)" >&"$fd"
}

show_error_and_exit()
{
    if [ "$?" -ne 0 ]
    then
        printc -error 'An error has occurred.'
        echo
        exit "$step"
    fi
}

ask_for_sudo_password()
{
    sudo sh -c ':'
}

is_linux_mint()
{
    return "$(grep -i mint /etc/lsb-release >/dev/null 2>&1)"
}

mintupdate()
{
    if is_linux_mint
    then
        while true
        do
            echo 'Do you want to check for kernel upgrade via MintUpdate?'
            read -r answer </dev/tty
            case "$answer" in
                [Yy])
                    break
                    ;;
                [Nn])
                    exit
                    ;;
                *)
                    echo 'Please answer Y or N.'
                    ;;
            esac
        done
        sudo /usr/bin/python3 /usr/lib/linuxmint/mintUpdate/mintUpdate.py show
    fi
}

echo
printc -info '--------------------'
printc -info 'Vlasta - Laptop Mint'
printc -info '--------------------'
echo
step=0
trap show_error_and_exit EXIT
while IFS='#' read -r command description
do
    printc -ok "Step $step:$description"
    printc -warning "$command"
    step=$(( step + 1 ))
    set -e
    eval "$command"
    set +e
    printc -ok 'Success.'
    echo
done <<'EOF'
ask_for_sudo_password # enter password
sudo dpkg --configure -a # configure packages
sudo apt-get install --fix-broken # fix broken dependencies
sudo apt-get update # update cache
sudo apt-get upgrade # upgrade packages
sudo apt-get dist-upgrade # upgrade packages with possible removals
sudo apt-get --purge autoremove # remove unused packages
sudo apt-get autoclean # clean up
mintupdate # Linux Mint kernel upgrade
EOF

ShellCheck output:

$ shellcheck myscript

Line 26:
    printf '%s\n' "$(tput bold)$(tput setaf "$color")$@$(tput sgr0)" >&"$fd"
                                                     ^-- SC2145: Argument mixes string and array. Use * or separate argument.
                                                                     ^-- SC2039: In POSIX sh, >& is undefined.

Line 31:
    if [ "$?" -ne 0 ]
         ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
  • SC2181 is a minor, irrelevant warning: the exit code can't be checked directly in this case.
  • SC2039 is a false positive for it couldn't detect the value of fd; just make sure that fd is always assigned a valid number here, which I have.
  • SC2145 is what I disagree with. See What is the difference between $* and $@?, from which I quote:

    "$*" expands to a single word "$1c$2c...". Usually c is a space, but it's actually the first character of IFS, so it can be anything you choose.

    "$@" expands to separate words: "$1" "$2" ...

    Of course this depends on how you want to use printc: if you're only giving it strings to print, it's OK to use "$*"; if you're also passing it the format string, or don't want IFS to affect the output presentation, use "$@". Feel free to replace $@ with $* if you feel so inclined to clear this warning.

You forgot to enable interpretation of backslash escapes in echo with the -e option, so I didn't get colorful outputs on my openSUSE where the default for echo is to disable interpretation of backslash escapes. But it doesn't matter since options for echo aren't specified by POSIX. Also, the ANSI escape code is probably less portable, and definitely less readable, than the tput alternative, as a SO user noted.

I grabbed the printc function from my personal arsenal (with minor modifications) and put it here. Originally it didn't have the format string hard-coded in, but you always use it like echo in your code and I'm not about to define another function or add another option to get this behavior, so I sacrificed generality for convenience.

echo without any argument prints a newline, so you don't have to define a function to do just that.

The time frame between each prompt for a password by sudo is dependent on the sudoers policy, specifically the timestamp_timeout option, which can be set to 0 to always prompt for a password. Therefore, I'm not sure of the utility of your ask_for_sudo_password function when one can already type in the password on-demand, unless it is to test if sudo is available as a command, in which case it's better to write it in the same way as your is_it_linux_mint function and rename it to has_sudo or test_sudo.

Compare if is_linux_mint with if is_it_linux_mint, I think is_linux_mint is a better name.

There's no easy way to identify which Linux distribution is running on a user's system. /etc/os-release is a new standard, but for those without it, the most reliable way to find out, in general, is probably by parsing lsb_release -i, provided it is installed on the system.

I see some superfluous code in your case statement:

            [Yy]) :   ; break;;
            [Nn]) exit; break;;

It's probably done for stylistic reasons, but still I want to point out that : and the break after exit are not needed. On a side note, I prefer the shell equivalent of the Allman coding style, so I don't cramp up my case constructs into one line per pattern. But however you write your code, consistency is key.

Last but not least, you're repeating the same patterns throughout each step of the maintenance tasks, so I wrapped them in a while read loop reading from a here document. So now your while read loop, which I grouped inside a new function mintupdate, has to have its input redirected back to the controlling terminal, and rest assured that /dev/tty is required to exist by the POSIX specifications.

After the rewrite, it has become quite easy to adjust the script's behavior: you just add the commands you want to run, potentially wrapped inside functions, in sequential order in the here document. The descriptions are optional, and you can omit them by not adding #-style comments after the commands that they annotate. The # delimiter is chosen so one can easily copy the lines and run them on their terminal.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.