Monitor Downloads folder and sort files as they are downloaded

Question

I have written my first shell script that uses inotify-tools to monitor my Downloads directory. What I intend to do with the script is that, as soon as a file is downloaded, it moves that file to a folder named after its extension. For example: if it is test.pdf it will be moved to a folder named pdf.

The script is here:

#!/bin/bash

cd ~/
basepath=Downloads/InternetDownloads/
while true ; do
filename=$(inotifywait -e create --format "%f" Downloads/InternetDownloads)
arr=(${filename//./ })
if [ ! -d ${arr[1]} ];
    then mkdir ${basepath}${arr[1]}
fi


separator='/'

newpath=${arr[1]}
mv $basepath${filename} $basepath$newpath${separator}
done    

The program is working correctly, but I am having trouble ending it elegantly. I have obtained a solution for working here. I want to know if my code is correct since the normal procedure to end scripts of this sort is just to fine the process if and using kill pid

Answer 1

There's a bunch of small items to consider, and then some larger items too. Starting small....

Sweating the small stuff....

• cd ~/ can be shortened to just cd. Not a big deal one way, or the other, but still....

• Try to quote string values, even when they do not have spaces. Your basepath variable may end up having a space in it at some point ("My Downloads" at some point?), and the quotes make it clear. Again, this is a minor item...

• Having declared the basepath variable, why are you not using it? Here you have:

  filename=$(inotifywait -e create --format "%f" Downloads/InternetDownloads)
  

  which should be:

  filename=$(inotifywait -e create --format "%f" "$basepath")
  

  ... note the quotes around the variable in case it ends up with spaces again.

File Extensions

You pull the extension off the file with: arr=(${filename//./ }). This replaces the dots in the filename, with spaces, then loads that space-separated list in to an array (which considers the spaces to be item-separators). Then, you pull the extension from the array with ${arr[1]}....

Ouch. This works only if there's only one . in the file name, and the file has no spaces in the name.

There's a much simpler pattern-match you can do to alleviate this problem:

extension=${filename##*.}

From the documentation:

the result of the expansion is the expanded value of parameter with the shortest matching pattern (the # case) or the longest matching pattern (the ## case) deleted.

With that, there's no arrays, and the extension is the last part (or the whole filename if there are no '.' values in the file.... you need to check for that case too. There's also the possibility that a file may end with a '.', leaving an empty-value extension.... you need to check that too.

inotify

Currently you have a number of problems with inotify:

1. you are letting it dump messages to STDERR:

   Setting up watches.
   Watches established.
   

   This is repeated for each file. It's unnecessary, and can be removed with the -q option to inotifywait.

2. You are using the create event to track. This is triggered when the file is created... but is that what you want? A download may take a while to complete, so moving the file part way through a download may not make sense. Many programs will continue based only on the filehandle, which does not depend on the file location, but not all programs will behave well if the file is moved part way through.

   You should consider tracking the close_write event instead. This will only trigger when a file handle is clsed after a write to the file handle. I expect this will track the 'end of download' rather than the 'start of download'

3. inotifywait (the way you have it set up) will only notify when the first file is created. This leaves you open to race conditions where a download may happen when you are copying the file to a new location. This new download will not be triggered on the next inotifywait cycle because it started before the wait. As a result, you are at risk of losing events.

I believe the best strategy for your job would be to declare a function for the jobs you want to do with each file.... and then pipe a 'monitor' stream in to it. I have put together the following:

#!/bin/bash

function relocate {
    local filename=$1
    local extension=${filename##*.}
    # need -d to text for directories we ourselves create
    if [[ -d "$filename" || "$extension" = "" || "$extension" = "$filename" ]]
    then
        # Directory, No extension, or empty extension.
        echo "Skipping: $filename"
        return
    fi

    if [ ! -d "$extension" ]
    then
        echo "Creating directory: $extension"
        mkdir "$outdir/$extension"
    fi

    echo "Relocate: $filename -> $outdir/$extension"
    mv "$filename" "$outdir/$extension/"

}

path=${1:-.}
outdir=${2:-.}

cd $path
echo "Working in $PWD with relocate to $outdir"


inotifywait --quiet --monitor --event close_write --format "%f" .  | while read line
do
    relocate "$line"
done