1

I am trying to add a way to remove backups older than 3 days ago in my script.

See below a stripped down simplified version, which just leaves the removal part.

It was my understanding that ${BACKUP_DEST[$i]}/$OLDBACKUP should be outputting something like '/Users/christopherdavies/Desktop/rsync/test1.co.uk/07-06-18'.

However, each time this runs, it deletes the folder above, of 'test1.co.uk', and regardless if it 3 days old or not.

Is anyone able to point me in the right direction? I haven't been able to make any headway... fresh pair of eyes?

#!/bin/sh

WEBSITE_HOST=(
    "test1@shell.gridhost.co.uk"
    "test2@shell.gridhost.co.uk"  
)

BACKUP_DEST=(
    "/Users/christopherdavies/Desktop/rsync/test1.co.uk"
    "/Users/christopherdavies/Desktop/rsync/test2.co.uk"
)

OLDBACKUP=`date -d "3 days ago" +"%d%m%Y"`

SITE_COUNT=${#WEBSITE_HOST[@]}

for (( i = 0; i < SITE_COUNT; i++ )); do

    # Remove old backups    
    if [ -d "${BACKUP_DEST[$i]}/$OLDBACKUP" ]; then rm -Rf ${BACKUP_DEST[$i]}/$OLDBACKUP; fi

done
exit 0
  • Use tmpreaper – Ipor Sircer Jun 9 '18 at 7:25
  • Is there any reason why you cannot rely on the file modification date? – Hermann Jun 9 '18 at 7:25
3

The date utility on macOS (which I presume you're using judging from the location of the user home directories) is not GNU date and therefore does not support all its features.

This means that you OLD_BACKUP variable will be empty (and you ought to see an error message when date is run), which in turn means that it's the ${BACKUP_DEST[$i]} directory that gets deleted.

Instead, you may use date -v -3d +'%d-%m-%Y'.

#!/bin/bash

set -e

backup_dest=(
    '/Users/christopherdavies/Desktop/rsync/test1.co.uk'
    '/Users/christopherdavies/Desktop/rsync/test2.co.uk'
)

when=$( date -v -3d +'%d-%m-%Y' )

for backup in "${backup_dest[@]}"; do
    [ -d "$backup/$when" ] && rm -Rf "$backup/$when"
done

Alternatively, install GNU coreutils and use GNU date.

Other changes made to your code:

  • Removed exit 0 from the end and added set -e. If the script fails, you will want to be able to notice it.

  • The website array was not used.

  • Loop over the items in the array rather than using indexes, it's easier to read.

  • If you want a single line simple if statement, you may use short circuit syntax instead (which is shorter).

  • Use lower-case variable names.

  • Always quote variable expansions.

  • It's a bash script, so it should reference bash on the #!-line.

  • Thank you! That was the issue. Also, thank you for your advice on my script. I am new to bash and still learning (as I am sure my code shows clearly). I have added my full script into my question, as I am unsure if some of your points would be correct (for example, removal of some variables etc). If you could take a look and let me know, that would be fantastic. I am still working on a way to loop the database connection details... – ccdavies Jun 9 '18 at 7:53
  • @ccdavies I'm happy to have helped. Rather than continuing to review your code in this answer, I'd suggest that you ask a new question if you have specific issues with the code. We don't usually do pure code reviews (there's a StackExchange site dedicated to that). – Kusalananda Jun 9 '18 at 8:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.