[SOLVED] bash while loop fail

rbees · 10-02-2014, 06:23 PM

A friend wrote a script for me and it seams to work on his server but it does not on mine.

The script is suppose to process a data file and pull a single line out and make a separate file out of it. Each line in the file begins with a number followed by a double pipe as in " 2||Some text." It is suppose to pull the lines one at a time up to the limit set in a different place in the script. And there are a few variables inside the data file that need to be filled correctly. That is about the extent of what this does. It is part of a larger project.

So far I have not been able to get it to do anything but "not generate any files" or "generate the files but all with the same content." Grrr.

At one point it did work briefly when I changed the grep to egrep. I think something may have gotten deleted when I was copying parts of it out to enable other functionalities that I wanted. So I may have broked it myself.

Code:

#! /bin/bash

STORDIR="${HOME}/BDay/Dev"
BINDIR="${HOME}/BDay"
TMPDIR="${HOME}/BDay/Dev"
BIN="/bin"


# Temporary Variables
Name="John"
AGE="54"
Delay="1440"
Domain="altell"
PNumber="123456789" 
COUNT="54"

        #Number of times to send blessing, based on age
        Count="56"

#set -x
#        Delay=$((60*$(($(($TotalDayMinutes))/$(($AGE))))))

        #Processing loop
        #
        #Processing loop to generate blessings
        #
        I=0
        COUNT=0
        while [ ${I} -lt ${Count} ]; do
                #for now, assumes all blessings are generic.  No M/F, Age, or relational component branches
                I=$(($I+1))
                #
                #Get Blessing from Blessings file
                #
                
                BlText=`egrep -e "^${I}|" ${STORDIR}/Blessings.txt`
                if [ "${BlText}" == "" ]; then
                  BlText="999||You have lived so long that you have already received every blessing humanly possible.  It is time to share that blessing and your wisdom with the world."
                  COUNT=${I}
                  I=999
                fi
                #
                #Convert variables in the blessings to values
                #
                Blessing=`echo ${BlText//@AGE@/$AGE}`
                Blessing=`echo ${Blessing//@NAME@/$Name}`
                Blessing=`echo ${Blessing//@SEX@/$Sex}`

                #  Column 1 is the plessing ID, Column 2 is for any flags for decision making, Column 3 is the acutal blessing.  
                Blessing=`echo ${Blessing} | awk -F"|" '{printf("%s",$3)}'`

                #
                #Store blessing to be sent in temporary file
                #
                echo "${Blessing}" > $TMPDIR/$Name.blessing.$I.txt
        done
        if [ "${COUNT}" == "0" ]; then
           COUNT=${AGE}
        fi
        echo "${BIN}/bash ${BINDIR}/send_blessing.sh ${Name} ${AGE} ${Delay} ${Domain} ${PNumber} ${COUNT} &" >> ${TMPDIR}/myjobs.sh
        chmod +x ${BINDIR}/send_blessing.sh

exit

I have not had any success trying to debug this. Some lines make sense to be but others don't. Inserting a "set -x" results in processing the data file however many times count is set. Looking through the scroll black does not yeild anything that I can say "yup, that's an issue".

Because of the debug fail I have no idea what to even search for to see if I can fix it myself.

Thanks

jlinkels · 10-02-2014, 06:48 PM

For one thing,

Code:

while [ ${I} -lt ${Count} ]; do

But throughout the script you use $COUNT. Case is significant!

jlinkels

rbees · 10-02-2014, 07:03 PM

Thanks

Count and COUNT actually are two separate variables that serve separate purposes in other parts of the larger project. One and I forget which remains constant and the other is incremented so the loop is processed the correct number of times.

rbees · 10-02-2014, 07:36 PM

I think I may have tracked down the issue, or one of them anyway.

This line does not seem to be doing what I think it should

I=$(($I+1))

I have tried I=$I(($I+1)) which errors

jlinkels · 10-02-2014, 09:16 PM

Quote:

Originally Posted by rbees

This line does not seem to be doing what I think it should
I=$(($I+1))

That is the correct expression for i=i+1

jlinkels

grail · 10-02-2014, 10:29 PM

Well I see a few issues but not sure which points to your problem:

1. Space between shebang and interpreter ... probably won't error but not the accepted norm

2. Whilst case sensitivity is used for bash variables, you can already see how naming 2 or more variables the same can cause issues when viewed by other parties

3. Prior to the while loop you have the following which will cancel each other (and not sure why one is a string and the other a number):

Code:

COUNT="54"
COUNT=0

4. Use (()) for arithmetic operations AND tests

Code:

while [ ${I} -lt ${Count} ]; do

while (( I < Count )); do

5. You can use standard increment options inside (())

Code:

I=$(($I+1))

(( I++ ))

6. Using $() is much clearer and more extensible than ``

Code:

BlText=`egrep -e "^${I}|" ${STORDIR}/Blessings.txt`

BlText="$(egrep -e "^${I}|" ${STORDIR}/Blessings.txt)"

7. The -e is superfluous in its current use (but i see no issue with leaving it in), however the change to egrep (which is deprecated and should be grep -E) has cause side effects:

Code:

grep -e "^${I}|" ${STORDIR}/Blessings.txt) - - This looks for a digit at the start of the line and followed by a pipe

egrep -e "^${I}|" ${STORDIR}/Blessings.txt) - - This looks for a digit at the start of the line OR ... as there is nothing after the pipe, my grep -E interpreted this to mean anything and hence spewed back the entire file

8. [[]] over [] (see here for some info) and use built in tests when looking at variable values

Code:

if [ "${BlText}" == "" ]; then

if [[ -z "${BlText}" ]]; then

9. As you have identified the column information, the below can be simplified to exclude the call to awk:

Code:

Blessing=`echo ${Blessing} | awk -F"|" '{printf("%s",$3)}'`

Blessing="${Blessing##*|}"

10. Obviously we have a cut down listing of the full script, but the chmod at the end makes little sense unless this file is being deleted elsewhere and also populated somewhere else as it is not done in this part.

rbees · 10-03-2014, 01:27 PM

Thanks grail and jlinkels

To take your list in order

1. Fixed

2. Understood. Although in the larger project that confusion is not as great. Said project can be seen at https://github.com/rbees/Birthday-Blessings

3. COUNT="54" is a temporary just for testing as that variable is set in a different part of the larger project and serves other purposes. I have no idea why in this section it is then zeroed out. Also looking through I see that it need not be set prior to this section being run. So I removed it from the temporary variable list.

4. That is a lot easier for me to understand what it does.

5. Trying

6. I was somewhat aware of that. But the original author did not use it, so for consistencies sake I will not change it unless that proves to be the fix, at which time I will probably break it again in the process of making the script uniform.

7. Trying grep -E

8. Same as 6

9. Took me a while to figure out what that was doing. It had me all confused before your insight.

10. This one is all me. If you look through the whole project you will see that the first script that is run generates a csv data file which is then read by the second script (which this peace is part of), and generates a third script ( one of the ones generated here). That third script is then run at a specific time and calls a 4th script to process out the batch job to completion.

The original second script did not make the file executable so it had to be done by hand, kind of a pain, so I added that line. I have been working on adding the functionality of automating the batch job but there are issue with hdate converting a Hebrew dates during leap years back into Gregorian dates. I think it is actually a bug in hdate because it is not processing the leap-months, which are numbered 13 and 14, correctly when trying to convert backwards as the man page says it should. I have posted to the mailing list but not heard back.

So now for the test. One more note first. I am using a pristine Debian Wheezy vm now.

And a fail

rbees · 10-03-2014, 02:16 PM

it's current form

Code:

#!/bin/bash

STORDIR="${HOME}/BDay"
BINDIR="${HOME}/BDay"
TMPDIR="${HOME}/BDay"
BIN="/bin"


# Temporary Variables
Name="John"
AGE="54"
Delay="1440"
Domain="altell"
PNumber="123456789" 

        #Number of times to send blessing, based on age
        Count="3"

#set -x
#        Delay=$((60*$(($(($TotalDayMinutes))/$(($AGE))))))

        #Processing loop
        #
        #Processing loop to generate blessings
        #
        I=0
        COUNT=0
        while (( I < Count )); do
                #for now, assumes all blessings are generic.  No M/F, Age, or relational component branches
               
                (( I++ ))

                #
                #Get Blessing from Blessings file
                #
                
                BlText=`grep -E "^${I}|" ${STORDIR}/Blessings.txt`
                if [ "${BlText}" == "" ]; then
                  BlText="999||You have lived so long that you have already received every blessing humanly possible.  It is time to share that blessing and your wisdom with the world."
                  COUNT=${I}
                  I=999
                fi
                
                echo " # 1  ${BlText}"
                #
                #Convert variables in the blessings to values
                #
                Blessing=`echo ${BlText//@AGE@/$AGE}`
                Blessing=`echo ${Blessing//@NAME@/$Name}`
                Blessing=`echo ${Blessing//@SEX@/$Sex}`

                #  Column 1 is the blessing ID, Column 2 is for any flags for decision making, Column 3 is the acutal blessing.  
#                Blessing=`echo ${Blessing} | awk -F"|" '{printf("%s",$3)}'`
                Blessing="${Blessing##*|}"
                #
                #Store blessing to be sent in temporary file
                #
                echo "${Blessing}" > $TMPDIR/$Name.blessing.$I.txt
        done
        if [ "${COUNT}" == "0" ]; then
           COUNT=${AGE}
        fi
        echo "${BIN}/bash ${BINDIR}/send_blessing.sh ${Name} ${AGE} ${Delay} ${Domain} ${PNumber} ${COUNT} &" >> ${TMPDIR}/myjobs.sh
        chmod +x ${BINDIR}/send_blessing.sh

exit

rbees · 10-03-2014, 02:45 PM

And all I get is "epic fail".

I have come to the conclusion that this line is the issue

Code:

 # BlText=`egrep "^${I}|" ${STORDIR}/Blessings.txt`
                BlText="$(egrep -e "^${I}||" ${STORDIR}/Blessings.txt)"

I have tried about every combination of -e -E egrep gerp that I can think of. It either dumps the whole data file into BlText or the "999 error text" I even tried adding an extra | with no joy.

Could it be the | (pipe)? Maybe there should be a different separator like a ,(comma) in the data file to search for. There must be another way to pull the proper data out of the file and dump it into BlText for further processing.

ntubski · 10-03-2014, 04:10 PM

Quote:

Originally Posted by rbees

Could it be the | (pipe)?

Yes, the pipe is a regex character so you would need to escape it:

Code:

BlText="$(egrep -e "^${I}\|\|" ${STORDIR}/Blessings.txt)"

Although if you use grep (rather than egrep) then | is not considered a regex character:

Code:

BlText="$(grep -e "^${I}||" ${STORDIR}/Blessings.txt)"

Quote:

I have tried about every combination of -e -E egrep gerp

-e just says the next argument is a pattern, so it only really matters if you pass more than 1 pattern or your pattern starts with a "-". grep -E is the same as egrep.

rbees · 10-03-2014, 04:27 PM

no joy on my laptop, no time to fire up the test-vm, Yom Kippur and all

grail · 10-04-2014, 04:24 AM

AS ntubski has said and myself in point 7 of my previous post, grep is all you really need here to have the desired affect as the pipe will be treated as just another character.

My suggestion, like when doing any bash scripting, would be test that command in isolation on the command line. Remembering that the script is just a way to put a whole lot of
commands in the same place, so if it work on the command line there should be no real reason not to work in a script (generally)

ntubski · 10-04-2014, 10:52 AM

Looking at https://github.com/rbees/Birthday-Bl.../Blessings.txt, I see the format is a bit redundant: the first column is the same as the line number. We can exploit this to avoid the need for a loop entirely:

Code:

cut '-d|' -f3  Blessings.txt                         | # take only 3rd column
head -$AGE                                           | # lines 1 to $AGE
sed "s/@AGE@/$AGE/g;s/@NAME@/$Name/g;s/@SEX@/$Sex/g" | # replace @placeholders@
split --lines=1 --additional-suffix=.txt --numeric-suffixes=1 - "$Name.blessing."
# split blessings into separate files.

rbees · 10-05-2014, 05:14 PM

Thanks ntubski

That made it process correctly. I really don't know why I could not get grep/egrep to work.

My only concern with the above is when the data file is broken up into smaller more relevant to specifics that it will still work. But I will cross that bridge when I get to it.