Bash Script Loop Question

SoulShaker · 06-17-2009, 11:30 AM

I don't have a lot of experience scripting in Bash, but am working on a script right now. What the script is doing is comparing identically named files in two different directories, then reporting the differences between them with some information parsed out of the file and filename.

Here is the script:

Code:

#!/bin/bash

###Variables###
#-Do not include trailing forward slashes at the end of these directory locations!
wf_twist_dir=/path/to/dir1
wf_nas_dir=/path/to/dir2

#Use if changes made
emailBody1="Changes made.\n\n"	

#Use if no changes made
emailBody2="Changes not made.\n\n"	

changesExist=0		#If still zero at end of script, no changes were made.
###############


for file in `ls $wf_twist_dir | grep 'params.[0-9][0-9]*'`;do
	cmp -s $wf_twist_dir/$file $wf_nas_dir/$file > /dev/null ; changeFlag=$?
	echo Comparing $file "--" $changeFlag
	wfName=`echo $wf_twist_dir | sed 's/.*\/\(.*$\)/\1/'`
	toolNum=`echo $file | sed 's/.params.\([[:digit:]]\)*/\1/'`
	toolName=`cat $wf_twist_dir/$wfName.wfl | grep \($toolNum,NAME\) | sed 's/.*NAME)\(.*\)/\1/'`		
	if [$changeFlag] ; then
		$changesExist=1
		$emailBody1=$emailBody1"Workflow Name: "$wfName"\nTool Name: "$toolName"\nChanges:\n"	
		$emailBody1=$emailBody1`diff -y $wf_twist_dir/$file $wf_nas_dir/$file --suppress-common-lines`"\n\n"
	else
		echo nothing > /dev/null
	fi
done

echo $changesExist

if [$changesExist] ; then
	echo $emailBody1
else
	echo $emailBody2
fi

The script is running to completion, but is throwing the error:

./wfCompare.sh: 31: [0]: not found

Line 31 is the 'done' closing the for loop. It throws it on every iteration. It also gets thrown once at the end on line 39, which is the 'fi' closing the final if statement.

Any idea what this means?

colucix · 06-17-2009, 11:53 AM

Code:

if [$changeFlag] ; then

This is the wrong statement. First you have to put spaces between the square brackets and the expression. Second, the shell does not make difference when evaluating 0 or 1 as the expression: it will return always true! You have to put it explicitly:

Code:

if [ $changeFlag -eq 0 ] ; then

David the H. · 06-17-2009, 12:31 PM

As colucix pointed out, your main problem is in your test statement. You have to put spaces around the test brackets. This is because '[' is actually a command name. It's an alias for 'test'. The ending bracket is not strictly necessary, but all modern bash versions require it.

But let's give you a few other formatting tips also.

1) The double quotes (") are considered the "weak" quoting mechanism, because variable expansion and some other translations will still work inside them. This means you can do, for example:

Code:

echo "Comparing $file -- $changeFlag"

instead of

echo Comparing $file "--" $changeFlag

(You should always quote all of your echo outputs in any case.)

Single quotes, OTOH, (') are "hard" quotes, because everything inside them will be taken literally.

2) The backtick (`) form of command substitution has been superseded by the '$()' form. Not only is it easier to read, but they can more easily be nested.

Code:

wfName=`echo $wf_twist_dir | sed 's/.*\/\(.*$\)/\1/'`

should more properly be

wfName="$(echo $wf_twist_dir | sed 's/.*\/\(.*$\)/\1/')"

3) Speaking of the above string, there are better ways to get the base name of a file than using sed. One is to use the 'basename' command. Another is to use parameter substitution.

Finally, even the sed command can be cleaned up if you use some other character as the separator instead of '/', and add the -r (extended regex) flag, so you don't have to escape everything.

Code:

wfName="$(basename $wf_twist_dir)"

or

wfName="${wf_twist_dir##*/}"

or

wfName="$(echo $wf_twist_dir | sed -r 's|.*/(.*$)|\1|')"

4) '$variable' is only used when you want to output the value of a variable. Use the variable name without the dollar sign when you want to set it. You made this mistake inside the if statement. Also, '${variable}' is the extended form of '$variable', which will let you more safely combine them inside of character strings, as well as performing various parameter substitutions.

You should generally quote the variable values when you set them (variable="value"), except perhaps when it's a simple integer or such.

Code:

changesExist=1

emailBody1="${emailBody1}Workflow Name: $wfName\nTool Name: $toolName\nChanges:\n"

emailBody1="${emailBody1}$(diff -y $wf_twist_dir/$file $wf_nas_dir/$file --suppress-common-lines)\n\n"

Edit: one more:

5)

Your final echo statement will not be formatted correctly. You need to use 'echo -e' for it to translate the newline characters in the variable output. Also the variable should be quoted so that the contents are seen as a single unit.

Code:

echo -e "$emailBody1"

David the H. · 06-17-2009, 12:49 PM

Quote:

Originally Posted by colucix

Second, the shell does not make difference when evaluating 0 or 1 as the expression: it will return always true!

To be pedantic, the test '[ $variable ]' will return true if the variable is set to something, anything. It will return false if the variable is unset or set to a null string.

colucix · 06-17-2009, 01:00 PM

Quote:

Originally Posted by David the H.

To be pedantic, the test '[ $variable ]' will return true if the variable is set to something, anything. It will return false if the variable is unset or set to a null string.

Yes, of course. I wanted just to clarify that the shell does not evaluates 0 or 1 as true or false, as some other scripting language do. Thank you for pointing it out.

SoulShaker · 06-17-2009, 01:44 PM

Thanks for the tips, guys. Much appreciated.