9

When I do rm *.old.* on the command line it removes correctly, but when I do it in the following portion of my script, it does not rm all the *.old.* files.

What is wrong in my bash script:

 for i in ./*; do
    if [[ -f $i ]];  then

        if [[ $i  ==  *.old.* ]]; then
                oldfile=$i
                echo "this file is to be removed: $oldfile"
                rm $oldfile
                exec 2>errorfile
            if [ -s $errorfile ]
            then
                echo "rm failed"
            else
                echo "removed $oldfile!"
            fi
        else
            echo "file with old extension  does not exist"
        fi

        orig=$i
        dest=$i.old
        cp $orig $dest
        echo "Copied $i"

    else
        echo "${i} is not a file"
    fi 
done
3

If I understand what you are doing (delete any files with the .old suffix, and make a copy of any existing files with a .old suffix), you could use find instead:

#!/bin/sh

find . -maxdepth 1 -name \*.old -type f -printf "deleting %P\n" -delete
find . -maxdepth 1 -type f -printf "copying %P to %P.old\n" -exec cp '{}' '{}.old' \;

-maxdepth 0 stops the find command looking in subdirectories, -type f looks for regular files only; -printf creates messages (%P is the filename found). The -exec cp calls the copy function and '{}' is the filename

13

There are various possible failure points in your script. First of all, rm *.old* will use globbing to create a list of all matching files, and that can deal with file names containing whitespace. Your script, however, assigns a variable to each result of the glob and does that without quoting. That will break if your file names contain whitespace. For example:

$ ls
'file name with spaces.old.txt'  file.old.txt
$ rm *.old.*   ## works: both files are deleted

$ touch "file.old.txt" "file name with spaces.old.txt"
$ for i in ./*; do oldfile=$i; rm -v $oldfile; done
rm: cannot remove './file': No such file or directory
rm: cannot remove 'name': No such file or directory
rm: cannot remove 'with': No such file or directory
rm: cannot remove 'spaces.old.txt': No such file or directory
removed './file.old.txt'

As you can see, the loop failed for the file with spaces in its name. To do it correctly, you would need to quote the variable:

$ for i in ./*; do oldfile="$i"; rm -v "$oldfile"; done
removed './file name with spaces.old.txt'
removed './file.old.txt'

The same issue applies to pretty much every use of $i in your script. You should always quote your variables.

The next possible issue is that you seem to expect that *.old.* matches files with the extension .old. It doesn't. It matches "0 or more characters" (*), then a ., then "old", then another . and then "0 or more characters again". This means that it will not match something like file.old, but only something like `file.old.foo:

$ ls
file.old  file.old.foo
$ for i in *; do if [[ "$i" == *.old.* ]]; then echo $i; fi; done
file.old.foo     

So, no match foe file.old. In any case, your script is far more complex than needed. Try this one instead:

#!/bin/bash

for i in *; do
    if [[ -f "$i" ]];  then
        if [[ "$i"  ==  *.old ]]; then
            rm -v "$i" || echo "rm failed for $i"
        else
            echo "$i doesn't have an .old extension"
        fi
        cp -v "$i" "$i".old
    else
        echo "$i is not a file"
    fi 
done

Note that I added -v to the rm and cpwhich does the same thing as what you were doing with yourecho` statements.

This isn't perfect since when you find, for example, file.old, that will be removed and, later on, the script will try to copy it and fail since the file no longer exists. However, you haven't explained what you script is actually attempting to do so I can't fix that for you unless you tell us what you are really trying to accomplish.

If what you want is to i) remove all files with the .old extension and ii) add the .old extension to any existing files that don't have it, all you really need is:

#!/bin/bash

for i in *.old; do
    if [[ -f "$i" ]]; then
        rm -v "$i" || echo "rm failed for $i"
    else
        echo "$i is not a file"
    fi 
done
## All the ,old files have been removed at this point
## copy the rest
for i in *; do
    if [[ -f "$i" ]]; then
        ## the -v makes cp report copied files
        cp -v "$i" "$i".old
    fi
done
  • I am trying to backup files to file.old but at the same time rm any files that end in .old.old or .old.old.old or .old.old.old etc. On the commandline I use rm *.old.* which removes these files but not the file.old backup file. I am trying to do that in my script. Thanks – Don Sep 11 '16 at 15:39
  • 1
    @Don please edit your question and explain this in more detail. Include example file names and what you would like to happen to them after your script is run. Ideally, come into chat and ping me there so we can discuss it. – terdon Sep 11 '16 at 15:55
7

The only cases rm $oldfile could fail are when your filename contains any character of IFS (space, tab, newline) or any glob character (*, ?, []).

If any character of IFS is present the shell will perform word splitting and based on the presence of globbing characters pathname expansion on the variable expansion.

So, for example, if the file name is foo bar.old., variable oldfile would contain foo bar.old..

When you do:

rm $oldfile

shell at first splits the expansion of oldfile on space into two words, foo and bar.old.. So the command becomes:

rm foo bar.old.

which would obviously lead to unexpected result. By the way, if you have any globbing operators (*, ?, []) in the expansion, then pathname expansion would be done too.

You need to quote the variables to get desired result:

rm "$oldfile"

Now, no word splitting or pathname expansion would be done, hence you should get desired result i.e. the desired file would be removed. If any filename happens to start with -, then do:

rm -- "$oldfile"

You might ask, why we don't need to quote variables when used inside [[, the reason being [[ is a bash keyword and it handles variable expansion internally keeping the expansion literal.


Now, couple of points:

  • You should redirect STDERR (exec 2>errorfile) before the rm command otherwise [[ -s errorfile ]] test would give false positives

  • You have used [ -s $errorfile ], you are using a variable expansion $errorfile, which would be NUL given errorfile variable is not defined anywhere. Perhaps you meant, just [ -s errorfile ], based on the STDERR redirection

  • If the variable errorfile is defined, while using [ -s $errorfile ], it would again choke on above mentioned cases of IFS and globbing because unlike [[, [ is not handled internally by bash

  • In the later part of the script, you are trying to cp the already removed file (again without quoting the variable), this does not make any sense, you should check that chuck and make necessary corrections based on your target.

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.