-1

I created a cron using bash to delete files older than 3 days, but when checking the age of the files with mtime +3 &> /dev/null it is always false. here's the script:

now=$(date)

create log file

file_names=('*_takrib_golive.gz' '*_takrib_golive_filestore.tar.gz')
touch /media/nfs/backup/backup_delete.log
echo "Date: $now" >> /media/nfs/backup/backup_delete.log
for filename in "${file_names[@]}";
do
 echo $filename

        if ls /media/nfs/backup/${filename} &> /dev/null
        then
                echo "backup files exist"
                if find /media/nfs/backup -maxdepth 1 -mtime +3 -name ${filename} -ls &> /dev/null
                then
                        echo "The following backup file was deleted" >> /media/nfs/backup/backup_delete.log 
                        find /media/nfs/backup -maxdepth 1 -mtime +3 -name ${filename} -delete
                else
                        echo "There are no ${filename} files older than 3 days in /media/nfs/backup" &>> /media/nfs/backup/backup_delete.log
                fi
        else
                echo "No ${filename} files found in /media/nfs/backup" >> /media/backup/backup_delete.log
        fi
done
exit 0

in if find /media/nfs/backup -maxdepth 1 -mtime +3 -name ${filename} -ls &> /dev/null always goes to else, even though files older than 3 days are in the directory

Admaja
  • 49
  • 6
  • 1
    Not your question, but `if ls /media/nfs/backup/${filename} &> /dev/null` should be `if test -e "/media/nfs/backup/${filename}"` and you should always quote your variables. – knittl Oct 10 '22 at 09:34
  • Do you get any errors from `find`? Get rid of the output redirection and see if you have errors (if it always goes to false, it hints at having a non-1 exit code) – knittl Oct 10 '22 at 09:35
  • `&>` is a syntax error in `sh`. See also [Difference between `sh` and `bash`](https://stackoverflow.com/questions/5725296/difference-between-sh-and-bash). How exactly are you running this script? – tripleee Oct 10 '22 at 09:43
  • no error occurs when I run find outside the shell script that I created, when I run it like this `find /media/nfs/backup -maxdepth 1 -mtime +3 -name ${filename} -ls` can output results but when I run this script `find /media/nfs/backup -maxdepth 1 -mtime +3 -name ${filename} -ls &> /dev/null` nothing appears – Admaja Oct 10 '22 at 09:44
  • @Admaja please remove the error redirection in your script and see if you get any errors in the script. You can also try running it with `-x` enabled – knittl Oct 10 '22 at 09:50

1 Answers1

1

You are not quoting the -name attribute so it expands to the name of the file which already exists.

I would refactor this rather extensively anyway. Don't parse ls output and perhaps simplify this by making it more obvious when to quote and when not to.

Untested, but hopefully vaguely useful still:

#!/bin/bash

backup=/media/nfs/backup
backuplog=$backup/backup_delete.log
# no need to touch if you write to the file anyway
date +"Date: %C" >> "$backuplog"

# Avoid using a variable, just loop over the stems
for stem in takrib_golive takrib_golive_filestore.tar
do
    # echo $filename
    # Avoid parsing ls; instead, loop over matches
    for filename in "$backup"/*_"$stem".gz; do
        pattern="*_$stem.gz"
        if [ -e "$filename" ]; then
            echo "backup files exist"
            if files=$(find "$backup" -maxdepth 1 -mtime +3 -false -o -name "$pattern" -print -delete)
            then
                echo "The following backup file was deleted" >> "$backuplog"
                echo "$files" >> "$backuplog"
            else
                echo "There are no $pattern files older than 3 days in $backup" >> "$backuplog"
            fi
        else
            echo "No $pattern files found in $backup" >> "$backuplog"
        fi
        # Either way, we can break the loop after one iteration
        break
    done
done
# no need to explicitly exit 0

The for + if [ -e ... ] arrangement is slightly clumsy, but that's how you check if a wildcard matched any files. If the wildcard did not match, if [ -e will check for a file whose name is literally the wildcard expression itself, and fail.

tripleee
  • 175,061
  • 34
  • 275
  • 318
  • Wouldn't it then be cleaner to turn on _nullglob_? If nothing matches the pattern, the loop is simply not executed. The message for no files being found can be generated by a flag, which is turned on once the first file has been found. – user1934428 Oct 10 '22 at 10:23
  • thanks, but there is still an error at the end of the line, it says `line 25: syntax error: unexpected end of file` – Admaja Oct 10 '22 at 10:24
  • Sorry, added the missing `done`. Thanks for the pings. – tripleee Oct 10 '22 at 10:55
  • @user1934428 `setopt -s nullglob` would work instead, absolutely. I tried to stick to reasonably POSIX syntax in case that would simplify things for the OP. – tripleee Oct 10 '22 at 10:55
  • almost, but there is still an error, namely the `name` error message appears `find: paths must precede expression: `name'` , then I tried to change it to `-name` it worked. but all files were deleted, so I removed `-false` and `-o`. and it works as i want. thanks again you saved my day. – Admaja Oct 10 '22 at 14:18
  • The point of `-false -o` is to produce an error if no files are found, but I guess I will have to refactor it to work correctly. Sorry again for the typos; I added the missing dash in `-name` now. – tripleee Oct 10 '22 at 14:38