laurent22 / rsync-time-backup

Time Machine style backup with rsync.
3.4k stars 448 forks source link

Backup of / is not cron friendly #181

Open prontog opened 4 years ago

prontog commented 4 years ago

Hello and thanks for this very useful tool!

When you backup /, the SRC_FOLDER var is stripped of the last / (line 378), and the df call (line 411) writes "df: '': No such file or directory" to STDERR. The script continues with the backup but it is not cron friendly since it always writes to STDERR and you cannot do:

MAILTO=panos@example.com 0 /1 rsync_tmbackup.sh / /mnt/backup >/dev/null

and only expect emails when something unexpected happens. It will always send an email due to the df error.

Best regards,

Panos

kapitainsky commented 4 years ago

definitely a bug introduced by #170

prontog commented 4 years ago

@kapitainsky, thanks for the quick response!

After adding a check so that it only strips when [[ $SRC_PATH != / ]], I stumbled on another cron unfriendliness :)

rsync_tmbackup.sh panos@example.com:/home/panos /mnt/backup_drive >/dev/null

will always write "Warning: Permanently added 'blah blah blah" (RSA) to the list of known hosts."

This is most likely because of the -o UserKnownHostsFile=/dev/null in lines 549, 551.

Wouldn't it be easier and more flexible to leave the ssh config to the user (with the -e option of rsync)?

The same goes for the necessary user part when parsing the SRC_FOLDERS and DEST_FOLDER in lines 177 and 188. I bet many users will set the user in their ~/.ssh/config file. I think simply splitting on the : is enough.

Should I make a PR? I could also add tests.

Best regards,

Panos

kapitainsky commented 4 years ago

You should definitely create PR and explain all details. I am sure @laurent22 will consider including it.

reactive-firewall commented 4 years ago

FYI this issue is caused by https://github.com/laurent22/rsync-time-backup/blob/88db869fe7a52864e18afc7e16a971499f79e830/rsync_tmbackup.sh#L359 and https://github.com/laurent22/rsync-time-backup/blob/88db869fe7a52864e18afc7e16a971499f79e830/rsync_tmbackup.sh#L378

reactive-firewall commented 4 years ago

184 looks to be a possible fix for the / issue discussed here, but I wonder if fixing the issue when parsing args might be safer and easier to maintain 🤔 ... thoughts?

prontog commented 4 years ago

This sounds like the best approach. Normalize the arguments at the start.