Removed test code

Code was assuming that you'd run backups hourly, eg, so you'd have
many backups within the last 24 hours. Code assumed that by the time
you found a backup more than 24h old, you'd already have a value for
$prev set. In the event that your most recent backup is more than 24
hours old, $prev would not be set and bash would get an error "unary
operator expected" when it tried to compare the day of the newest
backup to the empty $prev value.

I fix this by setting a default value for $prev, one that cannot be
mistaken for an existing backup because it does not represent a valid
date. This ensures that the most recent backup is correctly preserved
regardless of it's age.

README.md

@@ -40,14 +40,10 @@ An optional exclude file can be provided as a third parameter. It should be comp
 
 # TODO
 
-* Check if there's enough space in the destination before doing the backup. Also automatically delete old backups.
+* Minor changes (see TODO comments in the source).
-
-* Manage the backups in a way similar to Time Machine - hourly backups for the past 24 hours; daily backups for the past month; weekly backups for the previous months.
 
 # LICENSE
 
 [MIT](http://opensource.org/licenses/MIT)
 
-
 [![Bitdeli Badge](https://d2weczhvl823v0.cloudfront.net/laurent22/rsync-time-backup/trend.png)](https://bitdeli.com/free "Bitdeli Badge")
-

rsync_tmbackup.sh

@@ -5,15 +5,15 @@
 # -----------------------------------------------------------------------------
 
 fn_log_info() {
-	echo "[RB INFO] $1"
+	echo "rsync_tmbackup: $1"
 }
 
 fn_log_warn() {
-	echo "[RB WARN] $1"
+	echo "rsync_tmbackup: [WARNING] $1"
 }
 
 fn_log_error() {
-	echo "[RB ERROR] $1"
+	echo "rsync_tmbackup: [ERROR] $1"
 }
 
 # -----------------------------------------------------------------------------
@@ -21,12 +21,40 @@ fn_log_error() {
 # -----------------------------------------------------------------------------
 
 fn_terminate_script() {
-	echo "SIGINT caught"
+	echo "rsync_tmbackup: SIGINT caught."
 	exit 1
 }
 
 trap 'fn_terminate_script' SIGINT
 
+# -----------------------------------------------------------------------------
+# Small utility functions for reducing code duplication
+# -----------------------------------------------------------------------------
+
+fn_parse_date() {
+	# Converts YYYY-MM-DD-HHMMSS to YYYY-MM-DD HH:MM:SS and then to Unix Epoch.
+	case "$OSTYPE" in
+		linux*) date -d "${1:0:10} ${1:11:2}:${1:13:2}:${1:15:2}" +%s ;;
+		darwin*) date -j -f "%Y-%m-%d-%H%M%S" "$1" "+%s" ;;
+	esac
+}
+
+fn_find_backups() {
+	find "$DEST_FOLDER" -type d -name "????-??-??-??????" -prune
+}
+
+fn_expire_backup() {
+	# Double-check that we're on a backup destination to be completely
+	# sure we're deleting the right folder
+	if [ -z "$(fn_is_backup_destination "$(dirname -- "$1")")" ]; then
+		fn_log_error "$1 is not on a backup destination - aborting."
+		exit 1
+	fi
+
+	fn_log_info "Expiring $1"
+	rm -rf -- "$1"
+}
+
 # -----------------------------------------------------------------------------
 # Source and destination information
 # -----------------------------------------------------------------------------
@@ -35,6 +63,13 @@ SRC_FOLDER=${1%/}
 DEST_FOLDER=${2%/}
 EXCLUSION_FILE=$3
 
+for arg in "$SRC_FOLDER" "$DEST_FOLDER" "$EXCLUSION_FILE"; do
+	if [[ "$arg" == *"'"* ]]; then
+		fn_log_error 'Arguments may not have any single quote characters.'
+		exit 1
+	fi
+done
+
 # -----------------------------------------------------------------------------
 # Check that the destination drive is a backup drive
 # -----------------------------------------------------------------------------
@@ -46,15 +81,10 @@ fn_backup_marker_path() {
 }
 
 fn_is_backup_destination() {
-	DEST_MARKER_FILE="$(fn_backup_marker_path $1)"
+	find "$(fn_backup_marker_path "$1")" 2>/dev/null
-	if [ -f "$DEST_MARKER_FILE" ]; then
-		echo "1"
-	else
-		echo "0"
-	fi
 }
 
-if [ "$(fn_is_backup_destination $DEST_FOLDER)" != "1" ]; then
+if [ -z "$(fn_is_backup_destination $DEST_FOLDER)" ]; then
 	fn_log_info "Safety check failed - the destination does not appear to be a backup folder or drive (marker file not found)."
 	fn_log_info "If it is indeed a backup folder, you may add the marker file by running the following command:"
 	fn_log_info ""
@@ -67,15 +97,25 @@ fi
 # Setup additional variables
 # -----------------------------------------------------------------------------
 
-BACKUP_FOLDER_PATTERN="\d\d\d\d-\d\d-\d\d-\d\d\d\d\d\d"
+export IFS=$'\n' # Better for handling spaces in filenames.
 NOW=$(date +"%Y-%m-%d-%H%M%S")
 PROFILE_FOLDER="$HOME/.rsync_tmbackup"
 LOG_FILE="$PROFILE_FOLDER/$NOW.log"
 DEST=$DEST_FOLDER/$NOW
-LAST_TIME=$(ls -1 $DEST_FOLDER | grep "$BACKUP_FOLDER_PATTERN" | tail -n 1)
+PREVIOUS_DEST=$(fn_find_backups | sort | tail -n 1)
-PREVIOUS_DEST=$DEST_FOLDER/$LAST_TIME
 INPROGRESS_FILE=$DEST_FOLDER/backup.inprogress
 
+case "$OSTYPE" in
+	linux*)
+		KEEP_ALL_DATE=$(date -d '-1 day' +%s)
+		KEEP_DAILIES_DATE=$(date -d '-1 month' +%s)
+		;;
+	darwin*)
+		KEEP_ALL_DATE=$(date -j -f "%a %b %d %T %Z %Y" "`date -v -1d`" "+%s")
+		KEEP_DAILIES_DATE=$(date -j -f "%a %b %d %T %Z %Y" "`date -v -1m`" "+%s")
+		;;
+esac
+
 # -----------------------------------------------------------------------------
 # Create profile folder if it doesn't exist
 # -----------------------------------------------------------------------------
@@ -90,52 +130,88 @@ fi
 # -----------------------------------------------------------------------------
 
 if [ -f "$INPROGRESS_FILE" ]; then
-	if [ "$LAST_TIME" != "" ]; then
+	if [ "$PREVIOUS_DEST" != "" ]; then
 		# - Last backup is moved to current backup folder so that it can be resumed.
 		# - 2nd to last backup becomes last backup.
 		fn_log_info "$INPROGRESS_FILE already exists - the previous backup failed or was interrupted. Backup will resume from there."
-		LINE_COUNT=$(ls -1 $DEST_FOLDER | grep "$BACKUP_FOLDER_PATTERN" | tail -n 2 | wc -l)
+		LINE_COUNT=$(fn_find_backups | sort | tail -n 2 | wc -l)
-		mv $PREVIOUS_DEST $DEST
+		mv -- "$PREVIOUS_DEST" "$DEST"
 		if [ "$LINE_COUNT" -gt 1 ]; then
-			SECOND_LAST_TIME=$(ls -1 $DEST_FOLDER | grep "$BACKUP_FOLDER_PATTERN" | tail -n 2 | head -n 1)
+			PREVIOUS_PREVIOUS_DEST=$(fn_find_backups | sort | tail -n 2 | head -n 1)
-			LAST_TIME=$SECOND_LAST_TIME
+			PREVIOUS_DEST=$PREVIOUS_PREVIOUS_DEST
 		else
-			LAST_TIME=""
+			PREVIOUS_DEST=""
 		fi
-		PREVIOUS_DEST=$DEST_FOLDER/$LAST_TIME
 	fi
 fi
 
-# -----------------------------------------------------------------------------
+# Run in a loop to handle the "No space left on device" logic.
-# Check if we are doing an incremental backup (if previous backup exists) or not
+while [ "1" ]; do
-# -----------------------------------------------------------------------------
 
-LINK_DEST_OPTION=""
+	# -----------------------------------------------------------------------------
-if [ "$LAST_TIME" == "" ]; then
+	# Check if we are doing an incremental backup (if previous backup exists) or not
+	# -----------------------------------------------------------------------------
+
+	LINK_DEST_OPTION=""
+	if [ "$PREVIOUS_DEST" == "" ]; then
 		fn_log_info "No previous backup - creating new one."
-else
+	else
 		# If the path is relative, it needs to be relative to the destination. To keep
 		# it simple, just use an absolute path. See http://serverfault.com/a/210058/118679
-	PREVIOUS_DEST=`cd \`dirname "$PREVIOUS_DEST"\`; pwd`"/"`basename "$PREVIOUS_DEST"`
+		PREVIOUS_DEST=`cd \`dirname -- "$PREVIOUS_DEST"\`; pwd`"/"`basename -- "$PREVIOUS_DEST"`
 		fn_log_info "Previous backup found - doing incremental backup from $PREVIOUS_DEST"
 		LINK_DEST_OPTION="--link-dest=$PREVIOUS_DEST"
-fi
+	fi
 
-# -----------------------------------------------------------------------------
+	# -----------------------------------------------------------------------------
-# Create destination folder if it doesn't already exists
+	# Create destination folder if it doesn't already exists
-# -----------------------------------------------------------------------------
+	# -----------------------------------------------------------------------------
 
-if [ ! -d "$DEST" ]; then
+	if [ ! -d "$DEST" ]; then
 		fn_log_info "Creating destination $DEST"
-	mkdir -p $DEST
+		mkdir -p -- "$DEST"
-fi
+	fi
 
-# -----------------------------------------------------------------------------
+	# -----------------------------------------------------------------------------
-# Start backup
+	# Purge certain old backups before beginning new backup.
-# -----------------------------------------------------------------------------
+	# -----------------------------------------------------------------------------
+
+	# Default value for $prev ensures that the most recent backup is never deleted.
+	prev="0000-00-00-000000"
+	for fname in $(fn_find_backups | sort -r); do
+		date=$(basename "$fname")
+		stamp=$(fn_parse_date $date)
+
+		# Skip if failed to parse date...
+		# TODO: display warning
+		[ -n "$stamp" ] || continue
+
+		if   [ $stamp -ge $KEEP_ALL_DATE ]; then
+			
+			true
+
+		elif [ $stamp -ge $KEEP_DAILIES_DATE ]; then
+			
+			# Delete all but the most recent of each day.
+			if [ ${date:0:7} == ${prev:0:7} ]; then
+				[ ${date:8:2} -eq ${prev:8:2} ] && fn_expire_backup "$fname"
+			fi
+
+		else
+			
+			# Delete all but the most recent of each month.
+			if [ ${date:0:4} == ${prev:0:4} ]; then
+				[ ${date:5:2} -eq ${prev:5:2} ] && fn_expire_backup "$fname"
+			fi
+		fi
+
+		prev=$date
+	done
+
+	# -----------------------------------------------------------------------------
+	# Start backup
+	# -----------------------------------------------------------------------------
 
-# Run in a loop to handle the "No space left on device" logic
-while [ "1" ]; do
 	LOG_FILE="$PROFILE_FOLDER/$(date +"%Y-%m-%d-%H%M%S").log"
 
 	fn_log_info "Starting backup..."
@@ -151,21 +227,21 @@ while [ "1" ]; do
 	CMD="$CMD --delete-excluded"
 	CMD="$CMD --one-file-system"
 	CMD="$CMD --archive"
-	CMD="$CMD --progress"
 	CMD="$CMD --itemize-changes"
 	CMD="$CMD --verbose"
 	CMD="$CMD --log-file '$LOG_FILE'"
 	if [ "$EXCLUSION_FILE" != "" ]; then
-		CMD="$CMD --exclude-from \"$EXCLUSION_FILE\""
+		# We've already checked that $EXCLUSION_FILE doesn't contain a single quote
+		CMD="$CMD --exclude-from '$EXCLUSION_FILE'"
 	fi
 	CMD="$CMD $LINK_DEST_OPTION"
-	CMD="$CMD $SRC_FOLDER/ $DEST/"
+	CMD="$CMD -- '$SRC_FOLDER/' '$DEST/'"
 	CMD="$CMD | grep -E '^deleting|[^/]$'"
 
 	fn_log_info "Running command:"
 	fn_log_info "$CMD"
 
-	touch $INPROGRESS_FILE
+	touch -- "$INPROGRESS_FILE"
 	eval $CMD
 	RSYNC_EXIT_CODE=$?
 
@@ -185,6 +261,7 @@ while [ "1" ]; do
 	rm -- "$LOG_FILE"
 
 	if [ "$NO_SPACE_LEFT" == "0" ]; then
+		# TODO: -y flag
 		read -p "It looks like there is no space left on the destination. Delete old backup? (Y/n) " yn
 		case $yn in
 			[Nn]* ) exit 0;;
@@ -192,31 +269,19 @@ while [ "1" ]; do
 
 		fn_log_warn "No space left on device - removing oldest backup and resuming."
 
-		BACKUP_FOLDER_COUNT=$(ls -1 $DEST_FOLDER | grep "$BACKUP_FOLDER_PATTERN" | wc -l)
+		BACKUP_FOLDER_COUNT=$(fn_find_backups | wc -l)
 		if [ "$BACKUP_FOLDER_COUNT" -lt "2" ]; then
 			fn_log_error "No space left on device, and no old backup to delete."
 			exit 1
 		fi
 
-		# TODO: handle case where only two backup folders are left. In which case, need to remove --link-dest option
+		OLD_BACKUP_PATH=$(fn_find_backups | head -n 1)
-		
+		if [ "$OLD_BACKUP_PATH" == "" ]; then
-		OLDEST_BACKUP=$(ls -1 $DEST_FOLDER | grep "$BACKUP_FOLDER_PATTERN" | head -n 1)
-		if [ "$OLDEST_BACKUP" == "" ]; then
 			fn_log_error "No space left on device, and cannot get path to oldest backup to delete."
 			exit 1
 		fi
 
-		OLD_BACKUP_PATH="$DEST_FOLDER/$OLDEST_BACKUP"
+		fn_expire_backup "$OLD_BACKUP_PATH"
-		
-		# Double-check that we're on a backup destination to be completely sure we're deleting the right folder
-		OLD_BACKUP_PARENT_PATH=$(dirname -- "$OLD_BACKUP_PATH")
-		if [ "$(fn_is_backup_destination $OLD_BACKUP_PARENT_PATH)" != "1" ]; then
-			fn_log_error "'$OLD_BACKUP_PATH' is not on a backup destination - aborting."
-			exit 1
-		fi
-		
-		fn_log_info "Deleting '$OLD_BACKUP_PATH'..."
-		rm -rf -- "$OLD_BACKUP_PATH"
 
 		# Resume backup
 		continue
@@ -227,7 +292,8 @@ while [ "1" ]; do
 		exit $RSYNC_EXIT_CODE
 	fi
 
-	rm $INPROGRESS_FILE
+	rm -- "$INPROGRESS_FILE"
+	# TODO: grep for "^rsync error:.*$" in log
 	fn_log_info "Backup completed without errors."
 	exit 0
 done