Upgrade Firefox Quantum with logging and version check
To sudo or not to sudo


  I'm also wondering if it's better to only run the commands that need it with sudo or if I should rerun the whole script as sudo after prompting for the password like this:

if [[ "$EUID" != 0 ]]; then
    warning "Please run as root"
    sudo "$0" "$@"
fi



Can the script do its job without root permissions? This script cannot, because it wants to modify files that are normally only editable by root, such as /opt and /usr/bin. Therefore, the script requires root permissions. A good way to handle this is to check if the user has root permission, and exit if that's not the case, for example:

if [[ "$EUID" != 0 ]]; then
    warning "Please run as root"
    exit 1
fi


There's really no need to include the complexity of getting root permission in the script. With the above simple check, the user can either switch to the root user, or rerun the script with sudo. The rest of the script can assume the privilege (and responsibility) of root.

About logging


  I first wanted to save the logs to a log file at "$HOME/.logs/ffupgrade.log", but I'm not sure whether that is good practice or whether I should log to syslog.


I don't really know. But I can reason about it.

Does this really belong to syslog? The script upgrades Firefox. Let's look at something similar, for example, what happens when you upgrade software using the standard package manager of a Linux system. I just upgraded something in a Debian system I have at hand, and it didn't log anything about it in syslog. It may be relevant whether the upgrade is happening on explicit user action or automatically. I would conclude that when a user upgrades a package manually, it's not noteworthy enough to add to syslog.

On the other hand, what happens if you keep logging to ~/.logs? It accumulates forever without cleanup. You might want to log somewhere else that is subject to log a rotation policy in your systems by default, or document how to set it up in the script. This could be a valid argument, except that I doubt your Firefox upgrade logs will ever accumulate to anything significant. So... I would not bother about rotation. If the script is only going to run as root, then I would log somewhere more visible than ~/.logs, let's say /var/log.


  When logging to a file, I would add another variable logfile at the beginning and adapt the functions like so (similarly for success and warning)


I would add a logger function to encapsulate that logic, overriding the logger program, and then you won't need to touch the other functions.

Use local for local variables

It's good to use local for local variables in functions to avoid setting values in the global scope, which can be a source of nasty errors.

Double quote variables used on the command line

$fflink here should be enclosed in double-quotes:


ffinstalled_version="$($fflink --version | sed 's/[^0-9]*//g')"



Comparing versions

I have a doubt about comparing versions.
The script removes all non-numeric characters, and then compares values lexicographically.
Are you sure that will work reliably?
It would have been good to include some example versions in comments, to help assure readers.

Also, to remove non-numeric characters, I would use tr -cd 0-9 instead of sed 's/[^0-9]*//g'.

Verify exit codes

The script does some things, and tells some things to the user that might not be true, for example here:


sudo chmod 755 "$ffbin"
success "permissions set"
sudo ln -s "$ffbin/firefox" "$fflink"
success "symlink created"
sudo rm "/opt/$fffile"
success "tarball removed from /opt"
success "Firefox was upgraded"



Even if those commands fail, the script will print success messages, and happily carry on.

Use better variable names

What is vind?

Use verbose mode more

I would add -v to all interesting commands that support it, for example rm, mv.

Replace 1&gt;&amp;2 with &gt;&amp;2

The default file descriptor redirected is 1, no need to spell it out explicitly.

Simple is better than complex

[ $# != 1 ] is probably more natural and therefore easier to understand than [ $# -ne 1 ].