On Thu, Nov 12, 2009 at 07:16:41PM +0000, Cedric Staniewski wrote:
I hope your patch do not get lost in this thread.
Erm. Yeah, I hope so too. How does it get threaded?
@@ -345,26 +345,26 @@ handle_deps() { local R_DEPS_SATISFIED=0 local R_DEPS_MISSING=1
- [ $# -eq 0 ] && return $R_DEPS_SATISFIED + (( $# == 0 )) && return $R_DEPS_SATISFIED
Is there a reason why you do not use (( ! $# )) here?
Some variables are used as a boolean flag in which case testing if ((VAR)) and if (( ! VAR )) reads well. The $# (and EUID) are used as integer values so it struck me as more understandable or readable in this form.
@@ -450,12 +450,12 @@ download_sources() { for netfile in "${source[@]}"; do local file=$(get_filename "$netfile") local url=$(get_url "$netfile") - if [ -f "$startdir/$file" ]; then + if [[ -f "$startdir/$file" ]]; then msg2 "$(gettext "Found %s in build dir")" "$file" rm -f "$srcdir/$file" ln -s "$startdir/$file" "$srcdir/" continue - elif [ -f "$SRCDEST/$file" ]; then + elif [[ -f "$SRCDEST/$file" ]]; then msg2 "$(gettext "Using cached copy of %s")" "$file" rm -f "$srcdir/$file" ln -s "$SRCDEST/$file" "$srcdir/"
We could remove the quotes here, too.
I was lax with some of the quotes when doing concatenation and parameter expansion...
@@ -512,7 +512,7 @@ generate_checksums() {
local i=0; local indent='' - while [ $i -lt $((${#integ}+6)) ]; do + while [[ $i -lt $((${#integ}+6)) ]]; do indent="$indent " i=$(($i+1)) done
What about "while (( $i < $((${#integ}+6)) )); do"?
I was planning a separate patch switching to a for loop. for (( i = 0; i < ${#integ} + 6; i++ ))
@@ -521,8 +521,8 @@ generate_checksums() { for netfile in "${source[@]}"; do local file="$(get_filename "$netfile")"
- if [ ! -f "$file" ] ; then - if [ ! -f "$SRCDEST/$file" ] ; then + if [[ ! -f $file ]] ; then + if [[ ! -f "$SRCDEST/$file" ]] ; then error "$(gettext "Unable to find source file %s to generate checksum.")" "$file" plain "$(gettext "Aborting...")" exit 1
Same as above. Quotes are not necessary.
@@ -533,10 +533,10 @@ generate_checksums() {
local sum="$(openssl dgst -${integ} "$file")" sum=${sum##* } - [ $ct -gt 0 ] && echo -n "$indent" + (( ct )) && echo -n $indent echo -n "'$sum'"
I do not think we want to remove the quotes here.
$ indent=" " $ echo -n $indent; echo asdf asdf $ echo -n "$indent"; echo asdf asdf $
That'd be a careless use of s/"// in a macro on my behalf. Thanks for catching.
@@ -566,8 +566,8 @@ check_checksums() { file="$(get_filename "$file")" echo -n " $file ... " >&2
- if [ ! -f "$file" ] ; then - if [ ! -f "$SRCDEST/$file" ] ; then + if [[ ! -f $file ]] ; then + if [[ ! -f "$SRCDEST/$file" ]] ; then echo "$(gettext "NOT FOUND")" >&2 errors=1 found=0 @@ -622,8 +622,8 @@ extract_sources() { continue fi
- if [ ! -f "$file" ] ; then - if [ ! -f "$SRCDEST/$file" ] ; then + if [[ ! -f $file ]] ; then + if [[ ! -f "$SRCDEST/$file" ]] ; then error "$(gettext "Unable to find source file %s for extraction.")" "$file" plain "$(gettext "Aborting...")" exit 1
Quotes are not necessary.
@@ -662,31 +662,31 @@ extract_sources() { ... fi done
- if [ $EUID -eq 0 ]; then + if (( EUID == 0 )); then # change perms of all source files to root user & root group chown -R 0:0 "$srcdir" fi }
...
(( ! EUID )) ?
As above
@@ -713,12 +713,12 @@ run_function() { ... local i=1 while true; do - if [ -f "$BUILDLOG.$i" ]; then + if [[ -f "$BUILDLOG.$i" ]]; then i=$(($i +1)) else break
Quotes again? There are several other places where quotes or the dollar signs (in e.g. (( $found )) ) are not needed. Maybe you want to remove them, maybe not.
Cedric
Should I implement these changes and resend this patch? /me goes looking for git help Thanks for the feedback! - Isaac