Merge pull request #1168 from jsmeix/make_USB_backup_selection_work_i…

…ssue1166

Make USB backup selection work, see
#1166
Now for backup on USB during "rear recover"
there is no longer a backup selection dialog
when USB_SUFFIX is set (i.e. when a fixed
backup directory is used for all backups) and
when USB_SUFFIX is not set but only one single
backup is autodetected (then this one is used)
but if no backup is autodetected it errors out
otherwise (i.e. when more than one backup is detected)
then the backup selection dialog is shown.

usr/share/rear/conf/default.conf

@@ -363,7 +363,7 @@ USB_FILES=()
 # In contrast when USB_SUFFIX is set, backup on USB works in compliance with how backup on NFS works
 # (i.e. BACKUP_URL=usb:... and BACKUP_URL=nfs:... behave compatible when USB_SUFFIX is set)
 # which means a fixed backup directory of the form rear/HOSTNAME/USB_SUFFIX on the USB medium
-# and no automated removal of backups or other stuff (regardless of USB_RETAIN_BACKUP_NR)
+# and no automated removal of backups or other files (regardless of USB_RETAIN_BACKUP_NR)
 # see https://github.com/rear/rear/issues/1164
 # Using multiple backups as described in doc/user-guide/11-multiple-backups.adoc
 # requires a fixed backup directory so that USB_SUFFIX must be set for multiple backups on USB
@@ -374,9 +374,8 @@ USB_FILES=()
 USB_SUFFIX=""
 
 # Number of older rescue environments or backups to retain on USB.
-# The default USB_RETAIN_BACKUP_NR=2 means that there is at most
-# the latest (current) rescue environment or backup plus
-# the second eldest and the third eldest (three all together):
+# What is more than USB_RETAIN_BACKUP_NR gets automatically removed.
+# This setting is ignored when USB_SUFFIX is set (see above).
 USB_RETAIN_BACKUP_NR=2
 
 # Define the default WORKFLOW for the udev handler (empty to disable)

usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh

@@ -36,11 +36,27 @@ local backup_directory=$BUILD_DIR/outputfs/$NETFS_PREFIX
 
 # Normal (i.e. non-incremental/non-differential) backup:
 if ! test "incremental" = "$BACKUP_TYPE" -o "differential" = "$BACKUP_TYPE" ; then
-    backuparchive="$backup_directory/$backup_file_name"
     # In case of normal (i.e. non-incremental) backup there is only one restore archive
     # and its name is the same as the backup archive (usually 'backup.tar.gz'):
-    RESTORE_ARCHIVES=( "$backuparchive" )
-    LogPrint "Using backup archive '$backup_file_name'"
+    backuparchive="$backup_directory/$backup_file_name"
+    LogPrint "Using backup archive '$backuparchive'"
+    # This script is also run during "rear recover/restoreonly" where RESTORE_ARCHIVES must be set.
+    local backup_restore_workflows=( "recover" "restoreonly" )
+    if IsInArray $WORKFLOW ${backup_restore_workflows[@]} ; then
+        # Only set RESTORE_ARCHIVES the backup archive is actually accessible
+        # cf. https://github.com/rear/rear/issues/1166
+        if test -r "$backuparchive" ; then
+            RESTORE_ARCHIVES=( "$backuparchive" )
+        else
+            # In case of USB backup there is the subsequent 540_choose_backup_archive.sh script
+            # that shows a backup selection dialog when RESTORE_ARCHIVES is not already set.
+            if test "usb" = "$scheme" ; then
+                LogPrint "Backup archive '$backuparchive' not readable. Need to select another one."
+            else
+                Error "Backup archive '$backuparchive' not readable."
+            fi
+        fi
+    fi
     return
 fi
 
@@ -137,7 +153,8 @@ local date_time_glob_regex="$date_glob_regex-[0-9][0-9][0-9][0-9]"
 local create_backup_type=""
 # Code regarding creating a backup is useless during "rear recover" and
 # messages about creating a backup are misleading during "rear recover":
-if ! test "recover" = "$WORKFLOW" ; then
+local recovery_workflows=( "recover" "layoutonly" "restoreonly" )
+if ! IsInArray $WORKFLOW ${recovery_workflows[@]} ; then
     # When today is a specified full backup day, do a full backup in any case
     # (regardless if there is already a full backup of this day):
     if IsInArray "$current_weekday" "${FULLBACKUPDAY[@]}" ; then
@@ -161,7 +178,7 @@ if test "$latest_full_backup" ; then
     local full_or_incremental_backup_glob_regex="$date_time_glob_regex-[$full_backup_marker$incremental_backup_marker]$backup_file_suffix"
     # Code regarding creating a backup is useless during "rear recover" and
     # messages about creating a backup are misleading during "rear recover":
-    if ! test "recover" = "$WORKFLOW" ; then
+    if ! IsInArray $WORKFLOW ${recovery_workflows[@]} ; then
         # There is nothing to do here if it is already decided that
         # a full backup must be created (see "full backup day" above"):
         if ! test "full" = "$create_backup_type" ; then
@@ -215,7 +232,7 @@ if test "$latest_full_backup" ; then
 else
     # Code regarding creating a backup is useless during "rear recover" and
     # messages about creating a backup are misleading during "rear recover":
-    if ! test "recover" = "$WORKFLOW" ; then
+    if ! IsInArray $WORKFLOW ${recovery_workflows[@]} ; then
         # If no latest full backup is found create one during "rear mkbackup":
         create_backup_type="full"
         LogPrint "No full backup found (YYYY-MM-DD-HHMM-F.tar.gz) triggers full backup"
@@ -235,7 +252,7 @@ else
 fi
 # Code regarding creating a backup is useless during "rear recover" and
 # messages about creating a backup are misleading during "rear recover":
-if ! test "recover" = "$WORKFLOW" ; then
+if ! IsInArray $WORKFLOW ${recovery_workflows[@]} ; then
     # Set the right variables for creating a backup (but do not actually do anything at this point):
     case "$create_backup_type" in
         (full)

usr/share/rear/verify/USB/NETFS/default/540_choose_backup_archive.sh

@@ -1,5 +1,7 @@
 
-# In case of backup on USB let the user choose which backup will be restored.
+# In case of backup restore from USB let the user choose which backup will be restored.
+# This script is only run during a backup restore workflow (recover/restoreoly)
+# so that RESTORE_ARCHIVES is set in this script.
 
 scheme=$( url_scheme "$BACKUP_URL" )
 # Skip if not backup on USB:
@@ -9,44 +11,85 @@ test "usb" = "$scheme" || return
 # backup on USB works in compliance with backup on NFS which means
 # a fixed backup directory where the user cannot choose the backup
 # because what there is in the fixed backup directory will be restored
-# via RESTORE_ARCHIVES in usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh
+# via RESTORE_ARCHIVES set by usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh
 # Use plain $USB_SUFFIX and not "$USB_SUFFIX" because when USB_SUFFIX contains only blanks
 # test "$USB_SUFFIX" would result true because test " " results true:
 test $USB_SUFFIX && return
 
+# When backup on USB works in its default mode with several timestamp backup directories
+# let the user choose the backup regardless whether or not RESTORE_ARCHIVES
+# is already set by usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh
+# because when the user has created a backup plus recovery system via "rear mkbackup"
+# and afterwards a newer backup without recovery system via "rear mkbackuponly"
+# then the recovery system directory contains the (older) backup and that one gets
+# set in RESTORE_ARCHIVES by usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh
+# but probably the user wants to choose the newer backup to be actually restored.
+
 # Detect all backups on the USB device.
-# FIXME: This fails when the backup archive name is not
+# TODO: This fails when the backup archive name is not
 # ${BACKUP_PROG_ARCHIVE}${BACKUP_PROG_SUFFIX}${BACKUP_PROG_COMPRESS_SUFFIX}
 # so that in particular it fails for incremental/differential backups
 # but incremental/differential backups usually require several backup archives
 # to be restored (one full backup plus one differential or several incremental backups)
 # cf. RESTORE_ARCHIVES in usr/share/rear/prep/NETFS/default/070_set_backup_archive.sh
-# and the code below only works for one single backup archive:
+# and the backup selection code below only works to select one single backup archive:
 backups=()
 backup_times=()
-for rear_run in $BUILD_DIR/outputfs/rear/$HOSTNAME/* ;do
-    Debug "Relax-and-Recover run $rear_run detected."
+for rear_run in $BUILD_DIR/outputfs/rear/$HOSTNAME/* ; do
+    Debug "Relax-and-Recover run '$rear_run' detected."
     backup_name=$rear_run/${BACKUP_PROG_ARCHIVE}${BACKUP_PROG_SUFFIX}${BACKUP_PROG_COMPRESS_SUFFIX}
-    if [ -e $backup_name ] ; then
-        Debug "Relax-and-Recover backup $backup_name detected."
-        backups=( "${backups[@]}" "$backup_name")
-        backup_times=( "${backup_times[@]}" "${rear_run##*/}")
+    if test -r "$backup_name" ; then
+        LogPrint "Backup archive $backup_name detected."
+        backups=( "${backups[@]}" "$backup_name" )
+        backup_times=( "${backup_times[@]}" "${rear_run##*/}" )
     fi
 done
 
-# The user has to choose the backup
+# When there is no backup archive detected error out because in this case
+# it does not make sense to show a basically empty backup selection dialog
+# (strictly speaking a backup selection dialog with the only choice 'Abort').
+# For the 'test' one must have all array members as a single word i.e. "${name[*]}"
+# because it should succeed when there is any non-empty array member, not necessarily the first one:
+test "${backups[*]}" || Error "No '${BACKUP_PROG_ARCHIVE}${BACKUP_PROG_SUFFIX}${BACKUP_PROG_COMPRESS_SUFFIX}' detected in '$BUILD_DIR/outputfs/rear/$HOSTNAME/*'"
+
+# When there is only one backup archive detected use that and do not disrupt
+# "rear recover" or "rear restoreonly" with a backup selection dialog
+# because what else could the user chose except that one backup
+# (it is questionable why there is an 'Abort' choice below):
+if test "1" = "${#backups[@]}" ; then
+    backuparchive=${backups[0]}
+    RESTORE_ARCHIVES=( "$backuparchive" )
+    LogPrint "Using backup archive '$backuparchive'."
+    return
+fi
+
+# Let the user choose the backup that should be restored:
 LogPrint "Select a backup archive."
-select choice in "${backup_times[@]}" "Abort"; do
-    [ "$choice" != "Abort" ]
-    StopIfError "User chose to abort recovery."
-    n=( $REPLY ) # trim blanks from reply
-    let n-- # because bash arrays count from 0
+# Disable printing commands and their arguments as they are executed on stderr
+# which could have been enabled when running e.g. "rear -d -D recover"
+# to not disturb the select output which also happens on stderr.
+# When 'set -x' is set even calling 'set +x 2>/dev/null' would output '+ set +x' but
+# http://stackoverflow.com/questions/13195655/bash-set-x-without-it-being-printed
+# shows that when 'set -x' is set calling '{ set +x ; } 2>/dev/null' runs silently:
+{ set +x ; } 2>/dev/null
+select choice in "${backup_times[@]}" "Abort" ; do
+    test "Abort" = "$choice" && Error "User chose to abort recovery."
+    # trim blanks from reply
+    n=( $REPLY )
+    # bash arrays count from 0
+    let n--
     if [ "$n" -lt 0 ] || [ "$n" -ge "${#backup_times[@]}" ] ; then
-        LogPrint "Invalid choice $REPLY, please try again or abort."
+        # direct output to stdout which is fd7 (see lib/_input-output-functions.sh)
+        # and not using a Print function to always print to the original stdout
+        # i.e. to the terminal wherefrom the user has started "rear recover":
+        echo "Invalid choice $REPLY, try again or abort." >&7
         continue
     fi
-    LogPrint "Backup archive ${backups[$n]} chosen."
     backuparchive=${backups[$n]}
     break
-done 2>&1
+done 2>&7
+# Go back from "set +x" to the defaults:
+apply_bash_flags_and_options_commands "$DEFAULT_BASH_FLAGS_AND_OPTIONS_COMMANDS"
+RESTORE_ARCHIVES=( "$backuparchive" )
+LogPrint "Using backup archive '$backuparchive'."