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

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

I've found your script to be very useful in the time that I've used it (since shortly after you originally posted this question). I have made a few modifications though. Mainly, my version does not copy the original tarball to /opt (that's unnecessary I/O, especially since it's deleted immediately after it's extracted); instead, it uses tar's --one-top-level argument to extract the tarball from its original location to /opt without copying it or changing the working directory. Also, mine doesn't actually delete existing installs and symlinks; it just overwrites them where necessary.

#!/bin/bash

# firefox-upgrade - program to upgrade firefox quantum

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

# parsing path and filename
if [ "$#" -ne 1 ]; then
    error_exit "$0: usage: $0 firefox_quantum_path"
fi
firefox_path="$1"

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

# unpacking the tar
if tar --overwrite --one-top-level=/opt -jxvf "$firefox_path"; then
    # if unpack was successful create symlinks if they don't
    # already exist or don't point to the right target
    if [ "$(readlink /usr/bin/firefox 2> /dev/null)" != /opt/firefox/firefox ]; then
        ln -fs /opt/firefox/firefox /usr/bin/firefox
    fi
    if [ "$(readlink /usr/bin/firefox-quantum 2> /dev/null)" != /opt/firefox/firefox ]; then
        ln -fs /opt/firefox/firefox /usr/bin/firefox-quantum
    fi
else
    error_exit "Could not extract file! Aborting."
fi
exit 0

In addition, I've adapted the script for use with Thunderbird as well:

#!/bin/bash

# thunderbird-upgrade - program to upgrade thunderbird

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

# parsing path and filename
if [ "$#" -ne 1 ]; then
    error_exit "$0: usage: $0 thunderbird_path"
fi
thunderbird_path="$1"

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

# unpacking the tar
if tar --overwrite --one-top-level=/opt -jxvf "$thunderbird_path"; then
    # if unpack was successful create symlinks if they don't
    # already exist or don't point to the right target
    if [ "$(readlink /usr/bin/thunderbird 2> /dev/null)" != /opt/thunderbird/thunderbird ]; then
        ln -fs /opt/thunderbird/thunderbird /usr/bin/thunderbird
    fi
else
    error_exit "Could not extract file! Aborting."
fi
exit 0
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Hey, happy this script helped you and thanks for reporting back your improvements! \$\endgroup\$ – iuvbio Apr 16 at 9:52

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.