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.

asked Jan 29, 2017 at 17:57
\$\endgroup\$

1 Answer 1

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.

answered Jan 29, 2017 at 20:25
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Maybe also point out mywiki.wooledge.org/ParsingLs though there isn't a really elegant fix for that. \$\endgroup\$ Commented Feb 9, 2017 at 13:06

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.