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

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 1 Answer 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"

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.