patternbashMinor
Yet another bash backup script, using rsync --link-dest
Viewed 0 times
scriptyetdestanotherusinglinkbashrsyncbackup
Problem
I’ve written this bash backup script. It uses the
It’s mostly based on this very nice guide by Mike Rubel and various other contributors, as well as a few answers from Unix SE and other web reference.
The script is meant to be run at regular (typically, hourly) intervals with
Of course, I want to minimize the size of the backups. I also want to delete older backups before newer ones. To do so, I build
As always with bash scripts, I’m especially afraid of quoting issues, but any remark is welcome.
I in particular quite dislike how I use
Most of the “standard” commands, such as
```
#!/bin/bash
# Check if we are root (no one else should run this)
# ==================================================
if (( $(id -u) != 0 )); then
echo "/ ! \ Only root can run this script. Backup cancelled." >&2
exit 3
fi
# Functions
# =========
# Given a date (or a placeholder), returns the corresponding hourly backup name
function scheme {
local token="$@"
echo "hourly ${token}"
}
# Parameters
# ==========
# password_file=/etc/backup/passwd # Network yet untested
backup_directory=/path/to/bac
--link-dest option of rsync; that way, the user have access to the backed data at any time stamped with relatively affordable data overhead. Any duplicated data should be hard linked; the overhead mostly come from the directory structure.It’s mostly based on this very nice guide by Mike Rubel and various other contributors, as well as a few answers from Unix SE and other web reference.
The script is meant to be run at regular (typically, hourly) intervals with
cron and other scripts are in charge of safely keeping daily/weekly backups.Of course, I want to minimize the size of the backups. I also want to delete older backups before newer ones. To do so, I build
${backups}, an array of every backup1 sorted by modification date from newest to oldest. Thus ${backups[0]} (if it exists) is the latest complete backup and ${backups[@:$n]} for some integer $n lists every backup but the $n newest (from 0 to $n - 1).As always with bash scripts, I’m especially afraid of quoting issues, but any remark is welcome.
I in particular quite dislike how I use
find with both -mindepth and maxdepth, but couldn’t find any way around it.Most of the “standard” commands, such as
cut, sort or grep, are provided by BusyBox 1.16.1 and may not have every option available on most recent Linux distribution. cut, in particular, does not understand the -d option, hence the ugly tr trick.```
#!/bin/bash
# Check if we are root (no one else should run this)
# ==================================================
if (( $(id -u) != 0 )); then
echo "/ ! \ Only root can run this script. Backup cancelled." >&2
exit 3
fi
# Functions
# =========
# Given a date (or a placeholder), returns the corresponding hourly backup name
function scheme {
local token="$@"
echo "hourly ${token}"
}
# Parameters
# ==========
# password_file=/etc/backup/passwd # Network yet untested
backup_directory=/path/to/bac
Solution
The script is nicely written. I only have minor suggestions that are barely more than nitpicks.
Function declaration style
Instead of this:
The generally preferred style for declaring functions is this:
Redundant local variable
The local variable
You could simplify to:
Simplify condition
This condition can be simplified:
To just this:
Instead of this:
You can omit the
Don't repeat yourself
The
It would be good to create a helper function for this purpose:
Function declaration style
Instead of this:
function scheme {The generally preferred style for declaring functions is this:
scheme() {Redundant local variable
The local variable
token is redundant here:function scheme {
local token="$@"
echo "hourly ${token}"
}You could simplify to:
echo "hourly $@"Simplify condition
This condition can be simplified:
if (( ${#backups[@]} > 0 )); then
latest_backup="${backups[0]}"
else
latest_backup=""
fiTo just this:
latest_backup="${backups[0]}"Instead of this:
if [[ "${password_file}" != "" ]]; thenYou can omit the
!= "":if [[ "${password_file}" ]]; thenDon't repeat yourself
The
echo statement is duplicated for the sake of underlining:echo "Backing up ${source_directory}"
echo "Backing up ${source_directory}" | sed "s/./=/g"It would be good to create a helper function for this purpose:
print_heading() {
echo "$@"
echo "$@" | sed "s/./=/g"
}Code Snippets
function scheme {function scheme {
local token="$@"
echo "hourly ${token}"
}echo "hourly $@"if (( ${#backups[@]} > 0 )); then
latest_backup="${backups[0]}"
else
latest_backup=""
filatest_backup="${backups[0]}"Context
StackExchange Code Review Q#126697, answer score: 2
Revisions (0)
No revisions yet.