4
\$\begingroup\$

I want to backup a single SQLite database daily up to 30 days back, but I also want to keep at least 2 backups at all times (i.e. if there have been no backups in the last 30 days because the database didn't change, I don't want to delete old backups).

I came up with this simple script that is supposed to run as a daily cronjob:

#!/bin/bash

BACKUP_FILE="/path/to/db.sqlite3"
BACKUP_DIR="$HOME/backups"

today=`date "+%Y-%m-%d"`

# Less than 31 days old, i.e. 30 days or younger
if find "$BACKUP_FILE" -type f -mtime -31 | grep -q .
then
        find "$BACKUP_DIR" -type f -mtime +30 -exec rm {} \;
fi

last_backup=$(ls -t "$BACKUP_DIR" | head -1)
if [ -n "$last_backup" ] && diff "$BACKUP_FILE" "$BACKUP_DIR/$(ls -t "$BACKUP_DIR" | head -1)" >/dev/null
then :
else
        cp "$BACKUP_FILE" "$BACKUP_DIR/$today.sqlite3"
fi

I'm not sure if using a nop in the second then clause makes sense, but it seemed cleaner to me than wrapping the condition in a test and checking $?.

The database is used by <20 people and not changed very often, but I'm not sure if I should lock the database anyways - and I'm also not sure how to lock it from a shell script.

Since this is the first bash script I've ever written for serious use, I'd appreciate any feedback on how it could be improved.

\$\endgroup\$
2
\$\begingroup\$

Don't use the obsolete syntax `...`

Use the modern syntax $(...):

today=$(date "+%Y-%m-%d")

Simplify file deletion

Instead of deleting files with -exec rm {} \;, a simpler way is using -delete:

find "$BACKUP_DIR" -type f -mtime +30 -delete

Don't use the obsolete syntax head -N

head -1 is obsolete syntax. Use head -n 1 instead.

Don't repeat yourself

On these lines, $(ls -t "$BACKUP_DIR" | head -1) is repeated:

last_backup=$(ls -t "$BACKUP_DIR" | head -1)
if [ -n "$last_backup" ] && diff "$BACKUP_FILE" "$BACKUP_DIR/$(ls -t "$BACKUP_DIR" | head -1)" >/dev/null

The second use should be replaced with $last_backup.

Compare files using cmp

Instead of diff, it's more efficient to compare files using cmp:

if [ -n "$last_backup" ] && cmp -s "$BACKUP_FILE" "$BACKUP_DIR/$last_backup"

Avoid empty statements

Instead of the empty then clause of the final condition, it's better to invert it (using DeMorgan's law):

if [ ! "$last_backup" ] || ! cmp -s "$BACKUP_FILE" "$BACKUP_DIR/$last_backup"
then
        cp "$BACKUP_FILE" "$BACKUP_DIR/$today.sqlite3"
fi

Delay operations until really needed

The today variable is only used once at the end, when certain conditions are fulfilled. It would be better to create it right before it's needed.

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