[arch-general] [PATCH 01/48] Bashification of initscripts

[Victor Lowther]

On Wed, 2010-07-07 at 14:03 +1000, Allan McRae wrote:
> Here is a quick review on all these patches.   I recommend that the lvm 
> and crypttab changes get a decent amount of testing before these go live 
> as they are the biggest changes being done.
> 
> 
> Tighten up the console size finding code a bit.
>    Add some white space in test construct:
>      if ((STAT_COL==0)); then
>      if (( STAT_COL == 0 )); then
>    and throughout these patches

Maybe when all the other bugs are fixed. :)

> Simplify the code that clears USECOLOR.
>    The following condition is removed with no commit message to explain why
>      if [ $? = 3 ]; then
>        TERM_COLOURS=8

An exitval of 3 means tput has no idea what terminal type we are running
on, and I figured that it is after to default to not using colorized
output in that case.

> Replace trivial use of grep with bash regex conditional.
>    - if [ -n "$CONSOLEMAP" ] && echo "$LOCALE" | /bin/grep -qi utf ; then
>    + [[ $CONSOLEMAP && $LOCALE =~ UTF|utf ]] && CONSOLEMAP=""
> 
>    Use ... && ${LOCALE,,} == utf ]] to accurately replicate the grep

Hmmm... I had not seen that parameter expansion before.  New to bash
4.1?

> Replace slightly too long echo staement with a here document.
>                                    ^^ typo
>    I actually find the echo more readable

Well, then Thomas can keep it or drop it as he prefers. 

> Change the daemon runnign loop to use a case statement.
> Quote daemon names.
>    Merge these commits
> 
> 
> Both rc.single and rc.shutdown use the same code to kill everything.
>      + # $1 = where we are being called from.
>      + # This is used to determine which hooks to run.
>    ->  Add separater line here...
>      + # Find daemons NOT in the DAEMONS array. Shut these down first
> 
>    Why has this been removed:
>      -if [ -x /etc/rc.local.shutdown ]; then
>      - /etc/rc.local.shutdown
>      -fi
>    Ah... it has been moved to another place in another commit.  Please 
> document these sorts of changes in your commit message and preferably do 
> the entire move in one commit.

Will fix.

> Flatten LVM deactivation if block in rc.shutdown.
>    This change does not do the same thing and I do not see where it gets 
> replicated
>      -if [ -f /etc/crypttab -a -n "$(/bin/grep -v ^# /etc/crypttab | 
> /bin/grep -v ^$)" ]; then
>      +if [[ -f /etc/crypttab ]]; then

All the second bit of that test does is to see if there is actual
content in /etc/crypttab.  I handle that in the read loop by zapping
comments and blank lines with parameter expansion and conditional checks
instead -- the greps end up reading the whole file anyways.

Replicated via parameter expansion and conditionals:

        [[ $line && ${line:0:1} != '#' ]] || continue
        eval nspo=("${line%#*}")

>    Also another:
>    +if [[ $USELVM =~ yes|YES
>    -> ${USELVM,,} == yes
> 
> 
> bashify bringing up the loopback adaptor.
>    Add a commit message as that is doing a lot more than bashifing.

Already fixed in the last round of rebasing.

> Bashify locale setting.
>    $LOCALE =~ utf|UTF
> 
> 
> Allan

-- 
Victor Lowther
LPIC2 UCP RHCE