HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Back up or restore home directory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
backdirectoryrestorehome

Problem

I am currently trying to learn bash scripting and I've made a script that backs up or restores my home folder. My main question is whether my code is readable or very messy. I would love to learn to write my scripts understandable to others and also optimally. This script isn't made for sharing right now. Any tips on how I can make it better in any way? (Of course, the answer is yes, but I'm looking for specifics.) This is my first real script, so please don't be harsh.

```
#!/bin/bash

# Find where device is mounted and print so the user can check if correct.
BACKUP_DEV_UUID=#ENTER UUID HERE#
BACKUP_DEV_NODE=$(sudo blkid | grep $BACKUP_DEV_UUID | cut -f1 -d':')
if [[ ! $BACKUP_DEV_NODE =~ /dev/* ]]; then
echo "Error, device not connected"
exit 0
fi
MOUNT_POINT=$(mount | grep $BACKUP_DEV_NODE | cut -f3 -d' ')
echo "Mount point is: $MOUNT_POINT"

# Determine if you want to backup, restore of quit. Then fix full path.
echo -e "Input 1 to backup, 2 to restore, anything else to quit"
read -p "Enter choice: " -n1 fc
echo -e ""
case $fc in
1) echo "Backup chosen"
FULL_PATH=$MOUNT_POINT/$(hostname)_backup/home/$USER
if [[ ! -d $FULL_PATH ]]; then
echo "Backup path doesn't exist, creating"
mkdir -p $FULL_PATH
fi
;;

2) echo "Restore chosen"
# Choose hostname and username folders
echo "Select appropriate hostname folder"
select hn in $MOUNT_POINT/*_backup;
do
echo "Picked hostname folder: $HOSTNAME_VAR"
echo "Select appropriate username folder"
select un in $HOSTNAME_VAR/home/*;
do
echo "Picked username folder: $USERNAME_VAR"
FULL_PATH=$USERNAME_VAR
break
done
break
done
;;

*) echo "Exiting";
exit 0
;;
esac

# Print dest/source and ask user if accepted.
case $fc in
1) echo "Doing home folder backup"
echo "Backup to path: $FULL_PATH"
;;

2) echo "Doing home folder re

Solution

This is very nice for a beginner!

Instead of matching against files in the filesystem as a glob like this:

if [[ ! $BACKUP_DEV_NODE =~ /dev/* ]]; then


It would be better to use the -e flag to check if the path exists or not:

if [[ ! -e $BACKUP_DEV_NODE ]]; then


It's better to avoid any of the flags of echo,
because they don't work consistently in all systems.

In these examples you don't need any flags:

echo -e "Input 1 to backup, 2 to restore, anything else to quit"
echo -e ""


You could simplify these as:

echo "Input 1 to backup, 2 to restore, anything else to quit"
echo


If you find yourself needing any of echo's flags,
look into printf instead.

In here:

select hn in $MOUNT_POINT/*_backup;
do
    echo "Picked hostname folder: $HOSTNAME_VAR"
    echo "Select appropriate username folder"
    select un in $HOSTNAME_VAR/home/*;
        do
            echo "Picked username folder: $USERNAME_VAR"
            FULL_PATH=$USERNAME_VAR
            break
        done
    break
done


You're not using hn.
Another thing, the do...done of the inner select is over-indented, which is not consistent with the rest of the code.

The default exit code is that of the last operation.
So unless you want to forcefully exit with 0 after a failed operation,
you shouldn't need exit 0, you can write simply exit.

Instead of this:

echo $(date) >> $FULL_PATH/homefolder_backup.log


You could simply redirect the output of date itself:

date >> $FULL_PATH/homefolder_backup.log


The double-quotes here are unnecessary:

echo "Syncing"
echo "Done"

Code Snippets

if [[ ! $BACKUP_DEV_NODE =~ /dev/* ]]; then
if [[ ! -e $BACKUP_DEV_NODE ]]; then
echo -e "Input 1 to backup, 2 to restore, anything else to quit"
echo -e ""
echo "Input 1 to backup, 2 to restore, anything else to quit"
echo
select hn in $MOUNT_POINT/*_backup;
do
    echo "Picked hostname folder: $HOSTNAME_VAR"
    echo "Select appropriate username folder"
    select un in $HOSTNAME_VAR/home/*;
        do
            echo "Picked username folder: $USERNAME_VAR"
            FULL_PATH=$USERNAME_VAR
            break
        done
    break
done

Context

StackExchange Code Review Q#64090, answer score: 4

Revisions (0)

No revisions yet.