patternbashMinor
Back up or restore home directory
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
```
#!/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:
It would be better to use the
It's better to avoid any of the flags of
because they don't work consistently in all systems.
In these examples you don't need any flags:
You could simplify these as:
If you find yourself needing any of
look into
In here:
You're not using
Another thing, the
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
Instead of this:
You could simply redirect the output of
The double-quotes here are unnecessary:
Instead of matching against files in the filesystem as a glob like this:
if [[ ! $BACKUP_DEV_NODE =~ /dev/* ]]; thenIt would be better to use the
-e flag to check if the path exists or not:if [[ ! -e $BACKUP_DEV_NODE ]]; thenIt'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"
echoIf 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
doneYou'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.logYou could simply redirect the output of
date itself:date >> $FULL_PATH/homefolder_backup.logThe double-quotes here are unnecessary:
echo "Syncing"
echo "Done"Code Snippets
if [[ ! $BACKUP_DEV_NODE =~ /dev/* ]]; thenif [[ ! -e $BACKUP_DEV_NODE ]]; thenecho -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"
echoselect 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
doneContext
StackExchange Code Review Q#64090, answer score: 4
Revisions (0)
No revisions yet.