[SOLVED] Bash script critics required

grail · 01-16-2011, 11:32 PM

Howdy all

As part of a package manager I am trying to build I am at the stage of checking if my system has the latest packages installed. To this end I would like some feedback on pieces that could be improved or perhaps alternatives that I can utilise. To the aficionados' of Perl, Python and the like, whilst I know these may be better suited to the task, the package manager is currently completely bash so I am trying to stay within this paradigm for the moment.

I have edited this version slightly as it normally would tie in with the rest of my scripts which are not shown here. If there are any questions please ask and I will answer to the best of my ability:

So here goes:

Code:

#!/bin/bash

x=0
TEMP=$(mktemp)

########################################
#	Additions to work without my system
########################################

PAGE=$1			# Name of program, ie gcc
FILE=$2			# Name of current file that would be used for installation, ie gcc-4.4.3.tar.gz
DFILE=${FILE^}	        # Programs like python have a source file with first letter capitalised
RELEASE=$3              # Current version of application, ie 4.4.3
FUPDATE=$4		# Web address where program source is downloaded from, ie  http://ftp.gnu.org/gnu/gettext/

(( $# == 5 )) && EXTRA=$5	# Used when site includes other items to be ignored, eg xz site
							# includes a file xz-macos-5.0.tar.xz so EXTRA would contain macos
							# rarely used
########################################
#	End of Additions
########################################

#while read -r bio
#do
    unset UPDATE
	REMOVE=latest
    [[ $LEAD ]] && unset LEAD
#    . $bio
    [[ $EXTRA ]] && REMOVE+="|$EXTRA" && unset EXTRA

    wget -q -O- $FUPDATE >/dev/null
    if (( $? == 8 ))
    then
        if [[ ! "$FUPDATE" =~ /$ ]]
        then
           LEAD="(${FUPDATE##*/})?"
           FUPDATE="${FUPDATE%/*}/"
       fi
    fi

    if wget -q -t 3 -O $TEMP $FUPDATE
    then
        until [[ ${UPDATE,,} =~ $PAGE ]]
        do
            UPDATE=$(egrep -io "$PAGE-${RELEASE%%[0-9]*}[0-9][[:alnum:].-]+t?[bglx]z(2|ma)?" $TEMP | egrep -iv "($REMOVE)" | sort -V | tail -n 1)
            if [[ -z $UPDATE ]]
            then
                FUPDATE+=$(egrep -o "$LEAD[0-9]\.[[:alnum:].-]+/" $TEMP | sort -V | tail -n 1)
                wget -q -t 3 -O $TEMP $FUPDATE
            fi
        done

        if [[ ${UPDATE%.*} != ${FILE%.*} && ${UPDATE%.*} != ${DFILE%.*} ]]
        then
            echo $UPDATE >> update
        else
            echo $PAGE - no update required >> noupdate
        fi
    else
        echo "$PAGE - $FUPDATE not replying" >> noreply
    fi

    ((x++))
#done< <(find var/lib/grail/books/stable -path *template -prune -o -iname bio -print | sort)

rm $TEMP

[[ -f update ]] && echo "You have $(grep -c . update) updates available. Would you like to install them?"
[[ -f noupdate ]] && echo "You have $(grep -c . noupdate) with no update required"
[[ -f noreply ]] && echo "You have $(grep -c . noreply) sites not responding"
echo Processed $x files

The only piece that really needs explaining (I think) is the section where LEAD is being set. This is used in the case where the different versions for the source is in its own individual version level of the site.
Hopefully this example will explain better:

gcc normal path to download the source is something like (you may of course get it from elsewhere):

Quote:

ftp://gcc.gnu.org/pub/gcc/releases/gcc-4.5.2/gcc-4.5.2.tar.bz2

Problem is, if you put the following in as the FUPDATE value:

Quote:

ftp://gcc.gnu.org/pub/gcc/releases/gcc-4.5.2/

Then the only version you will get is 4.5.2. If there is a 4.5.3, it will not be in this directory.
So, when you have this scenario, you are required to enter the following:

Quote:

ftp://gcc.gnu.org/pub/gcc/releases/gcc-

This will invoke the LEAD option and the later loop to find the correct version.

NOTE: as per the above, you will notice that the address to set in FUPDATE must end with a forward slash (/) unless invoking the LEAD option

If I have completely lost anyone, as I said before please ask and I will attempt to answer

I look forward to everyone's feedback

Cheers
Grail

Snark1994 · 01-17-2011, 04:58 PM

Quote:

Originally Posted by grail

To the aficionados' of Perl, Python and the like, whilst I know these may be better suited to the task, the package manager is currently completely bash so I am trying to stay within this paradigm for the moment.

... *shuts mouth quickly*

This isn't a criticism, just trying to understand what's going on - I'm not exactly the world's best Bash scripter

How exactly are you invoking the programme? You have:

Code:

PAGE=$1			# Name of program, ie gcc
FILE=$2			# Name of current file that would be used for installation, ie gcc-4.4.3.tar.gz
DFILE=${FILE^}	# Programs like python have a source file with first letter capitalised
FUPDATE=$2		# Web address where program source is downloaded from, ie  http://ftp.gnu.org/gnu/gettext/

(( $# == 3 )) && EXTRA=$3	# Used when site includes other items to be ignored, eg xz site
							# includes a file xz-macos-5.0.tar.xz so EXTRA would contain macos
							# rarely used

Why are you setting both FILE and FUPDATE to $2? Are you expecting them to call "updateyThing gcc gcc-4.4.3.tar.gz" or "updateyThing gcc http://ftp.gnu.org/gnu/gettext/"... Or something I've missed?

Also, you source $bio - what's in this file, and where is this variable set? I don't have it listed in the output of 'env', and any googling with the term "bio" comes up with more biographies than you can shake a stick at. Is it something you've set specifically to work with your package manager?

Thanks a lot

grail · 01-17-2011, 06:25 PM

Hey Snark

It appears your first point about $2 is an issue with not reading the amendments prior to posting

I have fixed the original post for those

Quote:

Also, you source $bio - what's in this file, and where is this variable set?

Actually this line is commented out, but it is being set (in proper script) from the find at the bottom being pumped into while loop.

Let me know if the corrected version works better for you

action_owl · 01-17-2011, 08:06 PM

I'd recommended using getopts for arguments, and adding an option for verbose output (if this scripts gets really big it will be handy)
I also feel that writing variables in all caps should be reserved for constants it just helps readability IMO

call the arguments using getopts like this....

Code:

package-manager -d -p someProgram -v 4.4.3

Code:

#!/bin/bash

debug=false
debug()
{
	if $debug; then
		echo -e $1 >&2
	fi
}

while getopts "dp:f:r:u:" opt; do

	case $opt in

		d)
			debug "Enabling Verbose Output"
			debug=true
			;;

		p)
			debug "Page set to $OPTARG"
			page=$OPTARG
			;;

		f)
			debug "File set to $OPTARG"
			dFile=$OPTARG # do your upper case here (I haven't BASH'd in a while)
			;;

		r)
			debug "Release set to $OPTARG"
			release=$OPTARG
			;;

		u)
			debug "Address set to $OPTARG"
			fUpdate=$OPTARG
			;;

		\?)
			echo "Invalid option: -$OPTARG"
			echo "Usage: package-manager [options]"
			echo "Options:"
			echo -e '\t -d \t\t Show verbose output'
			echo -e '\t -p <opt> \t Name of program, ie gcc'
			echo -e '\t -f <opt> \t File that would be used for installation'
			echo -e '\t -r <opt> \t Current version of application, ie 4.4.3'
			echo -e '\t -u <opt> \t URL where source is downloaded'
			echo -e '\t    \t\t  Example:'
			shift 1
			error=true
			;;
	esac
done

if $error; then
	exit 1
fi

just my 2 cents...

grail · 01-17-2011, 08:47 PM

Hi action

Thanks for the 2 cents, but the page, file, etc variables are only being entered by hand here as the script would normally plug into the
rest of my system that already populates these (most come from being sourced in the $bio that has been commented out)

I will look into the debug option for another part of the program that deals with the parameters, although getopts is not an option as I have
both short and long versions

Cheers

grail · 01-19-2011, 04:45 AM

So I have come up with a question of my own, has anyone got anything quicker / better for checking if a page is active / correct than:

Code:

wget -q -O- $FUPDATE >/dev/null

crts · 01-19-2011, 11:47 AM

Hi grail,

finally had some time to test your script. I also tested it with "wrong" parameters. The idea behind that was to test the behavior in the rare case that a repository would change its directory structure. E.g., suppose that all gcc versions were available under the address
ftp://gcc.gnu.org/pub/gcc/releases/

At some point the developers would decide to change that and create a subdirectory for each version of gcc, like it actually is today
ftp://gcc.gnu.org/pub/gcc/releases/gcc-4.4.3/
ftp://gcc.gnu.org/pub/gcc/releases/gcc-4.5.2/
...

If the script were unaware of this (overnight) change it would still use the "slashed" address. LEAD would not be set. I don't know if the overall framework takes care that the address to download is always correct when the script is invoked. However, if this is not the case then the script goes into infinite loop.

So the question is if bio(?I guess?) is always updated and/or verified by the other scripts before the actual updater is called.

I hope I expressed the issue in an understandable manner.

grail · 01-20-2011, 01:51 AM

Hey crts

Thanks for having a poke around

I have made a change to capture the fact that I cannot get to an address. A perfect example is util-linux-ng which as of the next version (2.19) they have reverted back
to util-linux (without the -ng). Hence my script was running forever as it could never find the file I was looking for even though it correctly descended
into the v2.19 directory. It is a simple counter that lets the inner loop go for 5 rounds and then bails. So in your example the same would happen.
This is then echoed to the user on completion to say that the address needs to be checked (manually).

As I cannot hope to forsee all web maintainers ideas I figure this is a fair enough trade on the few instances when it happens.
This list will also capture those pages that were unreachable, so no edit may be required, just that the site is not available.

I will also include a BIO below so everyone can see the sort of information included.

template BIO (this lists all the fields that are currently available to a user should they wish to write their own (not all are mandatory)):

Code:

		PAGE=
	     RELEASE=
	EXTRA_GROUPS=
		BOOK=
	     CHAPTER=
	      SOURCE=${PAGE}-${RELEASE}
             ESOURCE=
		FILE=${SOURCE}.tar.bz2
               EFILE=
	SOURCE_PATCH=${SOURCE}-XXXXX.patch
	  SOURCE_DIR=${BUILD_DIR}/${SOURCE}
	   FDOWNLOAD=
	   EDOWNLOAD=
	   PDOWNLOAD=
             FUPDATE=$FDOWNLOAD
             PUPDATE=$PDOWNLOAD
               EXTRA=
	   SIGNATURE=
	    HOMEPAGE=
	     WRITTEN=
	       SHORT=
		LONG=

This is the current BIO for gcc:

Code:

		PAGE=gcc
	     RELEASE=4.5.2
		BOOK=stable
	     CHAPTER=gnu
	      SOURCE=$PAGE-$RELEASE
		FILE=${SOURCE}.tar.gz
        SOURCE_PATCH=${SOURCE}-pure64-2.patch
	  SOURCE_DIR=${BUILD_DIR}/${SOURCE}
       TMP_BUILD_DIR=$PAGE-build
	   FDOWNLOAD=ftp://gcc.gnu.org/pub/$PAGE/releases/$SOURCE/
	   PDOWNLOAD=http://patches.cross-lfs.org/dev/
             FUPDATE=${FDOWNLOAD%$RELEASE*}
             PUPDATE=$PDOWNLOAD
	   SIGNATURE=
	    HOMEPAGE=http://gcc.gnu.org
	     WRITTEN=20100618
	       SHORT="The GCC package contains the GNU compiler collection."
		LONG=

Here is the change to handle possible infinite loops or unreachable sites:

Code:

if (( TRIES < 5 )) && [[ $UPDATE ]]
then
	if [[ ${UPDATE%.*} != ${FILE%.*} && ${UPDATE%.*} != ${DFILE%.*} ]]
	then
		NEW_EXT=${UPDATE##*.}
		[[ $UPDATE =~ $PAGE ]] && TMP=${UPDATE#$PAGE-} || TMP=${UPDATE#${PAGE^}-}
		NEW_RELEASE=${TMP%.tar.*}
		[[ $NEW_RELEASE == $TMP ]] && NEW_RELEASE=${TMP%.$NEW_EXT}

# this sed is used to update the BIO
#		sed -i -e "/FILE=/s/[^.]*$/$NEW_EXT/"    \
#			   -e "/RELEASE=/s/[^=]*$/$NEW_RELEASE/" $PAGE_BIO
                echo $UPDATE >> update
#		read -r $PAGE_ARRAY_NAME <<< "${!PAGE_ARRAY_NAME} $PAGE"
#		(( COUNTER++ ))
	fi
else
#	CHECK_ADDRESS+=( "$PAGE|$FUPDATE" )
        echo "$PAGE - $FUPDATE not replying" >> noreply
fi

The CHECK_ADDRESS array is echoed at the end if it contains anything. This is in place of the files currently being populated.
I have commented out those features that won't work without the BIOs.