[pacman-dev] [PATCH] scripts: replace test builtin [ with shell keywords [[ and ((
allan at archlinux.org
Fri Nov 6 11:19:21 EST 2009
Isaac Good wrote:
> 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(-)
>> 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?
> 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.
Well, SOURCEONLY used to only have values 0 and 1 but then got extended
to have 2 as well. This was more flagging that we will need to be
careful if we ever expand other variables.
More information about the pacman-dev