1

I have this simple code that creates .bak files when I clean, . bak files should be removed when I clean again and there is no . bak it should echo "there's no .bak" but it only executes the if condition. Here's the code:

#!/bin/bash
chmod u+x sources/*
if [ "$1" == "clean" ]
then

    if [ -f `sources/*.bak` ]
    then        
       rm sources/*.bak

    else
        echo "there's no .bak files"
    fi

elif ["" == "$1"]
then

echo "This script makes a backup for each .c or .h file in sources directory"

target_files=`echo -n $(ls sources/{*.c,*.h})`
echo
echo "Starting to backup: $target_files"

for i in $target_files 
do 
  cp "$i" "$i".bak
done

echo
echo "The following backup files were created: `echo -n $(ls sources/*.bak)`"
echo
echo "Done"

fi 
Jeff Schaller
  • 67,283
  • 35
  • 116
  • 255

1 Answers1

4

the problem is the line

if [ -f `sources/*.bak` ]

backticks ` have a special meaning in bash: they are used to evaluate a command and return its output.

E.g. If your username is frodl then

if [ -f `whoami` ]

will check if there is a file named frodl.

So if you write sources/*.bak, it will first try to glob the term sources/*.bak, but since there are no files in the sources/ directory with a .bak extension, the term will not be expanded. It will then try to run the literal term sources/*.bak, which is no valid command (actually you can have a file called sources/*.bak (a filename with an asterisk at the beginning; shudder!), but it seems that you don't. However, this file would have a .bak extension, something we ruled out before.)

If you do have a file sources/deletemyharddisk.bak, then the term sources/*.bak will expand to that string, and then try to execute it (potentially deleting your harddisk).

So what you need to do is use ordinary quotes.

backupfiles=sources/*.bak
if [ "x${backupfiles}" = "xsources/*.bak" ]; then
     echo "no .bak files" 1>&2
else
     echo "there are .bak files"...
fi

This will first try to expand the sources/*.bak term into the backupfiles variable. If there are any bak-files, this will become a string of all filenames; if there are none, this will stay the unexpanded string. If you then compare the result with the literal (unexpanded) sources/*.bak you can tell, whether an expansion took place (because there are matching files) or not.

a better approach

however, there is little reason to first check if there are files and then delete them. a simpler approach is to just remove all the files that match a given pattern. find can do that for you:

find sources/ -maxdepth 1 -type f -name "*.bak" -delete

(this will search the first level (-maxdepth 1) of the sources/ directory for files (-type f; rather than directories or symlinks or other things), which end in .bak (-name "*.bak") and then remove them (-delete)

Do not parse ls

Another issue with your code is

target_files=`echo -n $(ls sources/{*.c,*.h})`
for i in $target_files 
...

You should never parse the output of ls.

Instead use something like

 for i in sources/*.c sources/*h
 do
   #...
 done
umläute
  • 6,472
  • 3
    No, double quotes don't allow globbing. And if they did, that is if the glob expanded to several arguments to the [ command, that would break the syntax of the [ command. You can't use globbing with [. – Stéphane Chazelas Jan 12 '17 at 11:22
  • @StéphaneChazelas shame on me; no idea what i was thinking; updated (and fixed) the answer... – umläute Jan 12 '17 at 11:26
  • The glob is not expanded in backupfiles=sources/*.bak either. You'd need an array and to set nullglob. (shopt -s nullglob; backupfiles=(sources/*.bak); if [ "${#backupfiles[@]}" -gt 0 ]...) and then there's a question of whether you want to check if they are regular files (all of them? any of them?) as the OP's [ -f ] suggests. – Stéphane Chazelas Jan 12 '17 at 11:32
  • here (bash-4.4.5) the term backupfiles=sources/*.bak is expanded. – umläute Jan 12 '17 at 11:33
  • 1
    You may also want to point out the other ["" == "$1"] error. – Stéphane Chazelas Jan 12 '17 at 11:34
  • 1
    No, you probably tried echo $backupfiles (split+glob operator; the glob is expanded upon the variable expansion) instead of echo "$backupfiles". – Stéphane Chazelas Jan 12 '17 at 11:35
  • 2
    Note that to have the equivalent of [ -f file ] in find, you'd need -xtype f (GNU specific) instead of -type f, as with -type f, you'd miss the symlinks to regular files. For the equivalent of the *.bak glob, you'd also need to add a ! -name '.*' to exclude hidden files (and fix the locale to C to avoid deleting hidden files with invalid characters). – Stéphane Chazelas Jan 12 '17 at 11:44