8
\$\begingroup\$

I wrote a little script to automate upgrading Firefox to a new version under Debian, since I don't want to install it from unstable and the snap version has rendering issues.

Any comments are welcome, is there anything I have missed or any potential problems?

#!/bin/bash

# firefox_upgrade - program to upgrade firefox quantum

error_exit()
{
    echo "$1" 1>&2
    exit 1
}

firefox_path=""
firefox_file=""

# parsing path and filename
if [ $# -ne 1 ]; then
    error_exit "usage: $0 firefox_quantum_path"
else
    firefox_path="$1"
    firefox_file="${firefox_path##*/}"
fi

# checking if input is a valid file
if [ ! -f "$firefox_path" ]; then
    error_exit "Invalid file! Aborting."
fi

# removing previous install, if existent
firefox_bin="/opt/firefox"
if [ -e "$firefox_bin" ]; then
    rm -rf $firefox_bin
else
    echo "$firefox_bin doesn't exist."
fi

# removing previous symlink, if existent
firefox_link="/usr/bin/firefox-quantum"
if [ -f "$firefox_link" ]; then
    rm $firefox_link
else
    echo "$firefox_link doesn't exist."
fi

# copying the tar to /opt
rsync -ah --progress $firefox_path /opt/$firefox_file

# unpacking the tar if successfully changed directory
if cd /opt; then
    tar -jxvf $firefox_file
else
    error_exit "Could not change directory! Aborting."
fi

# if unpack was successful, set permissions, create symlink, and remove tar
if [ "$?" = "0" ]; then
    chmod 755 /opt/firefox
    ln -s /opt/firefox/firefox /usr/bin/firefox-quantum
    rm $firefox_file
else
    error_exit "Could not extract file! Aborting."
fi
exit 0
\$\endgroup\$
8
\$\begingroup\$

Pretty nice script, I have mostly minor suggestions.

Always double-quote variables used in command arguments

The parameters in these command should be double-quoted to protect from word splitting and globbing:

rsync -ah --progress $firefox_path /opt/$firefox_file
...
rm $firefox_link
...
tar -jxvf $firefox_file
...
rm $firefox_file

Avoid changing the working directory in scripts

Changing working directories is usually error-prone and confusing, look for ways to do what you need without changing the working directory.

if cd /opt; then
    tar -jxvf $firefox_file
else
    error_exit "Could not change directory! Aborting."
fi

Another way to write this without changing the directory (and correct quoting):

tar -jxvf "/opt/$firefox_file" -C /opt

It's very good that you check the exit codes of commands, and this writing style will actually help you do that, by simplifying near the end:

if ! tar -jxvf "/opt/$firefox_file" -C /opt; then
    error_exit "Could not extract file! Aborting."
fi

chmod 755 /opt/firefox
ln -s /opt/firefox/firefox /usr/bin/firefox-quantum
rm "/opt/$firefox_file"

Declare constants at the top of the file

There are some constant values that are very important for the program's behavior, but they are buried within the code. It would be better to move these to somewhere near the top of the file, where they are easy to see and to change.

firefox_bin="/opt/firefox"
firefox_link="/usr/bin/firefox-quantum"

There are some other values that are not in variables, and it would be good to define variables for them, for example the ones in this command:

ln -s /opt/firefox/firefox /usr/bin/firefox-quantum

Technique

Here, the else branch is not necessary, because if the condition is true, the script will exit anyway. I suggest to just drop the else, simply move its body after the conditional (just like you did in other places).

if [ $# -ne 1 ]; then
    error_exit "usage: $0 firefox_quantum_path"
else
    firefox_path="$1"
    firefox_file="${firefox_path##*/}"
fi

To set a variable empty, you can simply write like this:

firefox_path=
firefox_file=

It's usually not a good idea to exit 0 at the end of a script. This forces exit code 0 (success). The exit code of a script is the exit code of the last command. By forcing it to 0, the failure of the last command will get incorrectly covered up.

\$\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.