[pacman-dev] [PATCH] scripts: replace test builtin [ with shell keywords [[ and ((

[Isaac Good]

On Fri, Nov 06, 2009 at 03:18:23PM +0000, Allan McRae wrote:
> Cedric Staniewski wrote:
> >FS#16623 suggested this change for makepkg; this patch applies it to the
> >remaining files in the scripts directory.
> >
> >Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
> >---
> > scripts/pacman-optimize.sh.in |   22 ++++++------
> > scripts/pkgdelta.sh.in        |   22 ++++++------
> > scripts/rankmirrors.sh.in     |   12 +++---
> > scripts/repo-add.sh.in        |   74 ++++++++++++++++++++--------------------
> > 4 files changed, 65 insertions(+), 65 deletions(-)
> <snip>
> 
> I went through every single change and they all look correct to me.
> I would appreciate someone else also doing a review here are there
> are a lot of changes and I could have overlooked something.
> 
> The only caution I have is this:
> 
> -	[ $CLEAN_LOCK -eq 1 -a -f "$LOCKFILE" ] && rm -f "$LOCKFILE"
> +	(( CLEAN_LOCK )) && [[ -f $LOCKFILE ]] && rm -f "$LOCKFILE"
> 
> The (( CLEAN_LOCK )) test is really equivalent of [ $CLEAN_LOCK -ne
> 0 ] and not [ $CLEAN_LOCK -eq 1 ].  In this case it does not matter
> as $CLEAN_LOCK can only be assigned 0 or 1 and I can not foresee
> that changing.  But it may pay to be careful.
> 
> As an aside. Bash tests are confusing...
> (( 0 )) returns 1 or "false"
> (( 1 )) returns 0 or "true"
> Does that not confuse other people too?
> 
> Allan
> 

In my original patch for makepkg there were a number of instances where I replaced a test for -eq 1 with a non-zero test. If the variable is assigned always to 0 or 1, this shouldn't be a problem, should it? With the notable exception of SOURCEONLY a lot of variables are boolean.

(( 0 )) and (( 1 )) are very much like how C treats integer testing. I just think of (( )) as a C-style test.

 - Isaac