xfs
[Top] [All Lists]

Re: [PATCH 06/25] xfstests: clean up and simply check CLI option parsing

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 06/25] xfstests: clean up and simply check CLI option parsing
From: Phil White <pwhite@xxxxxxx>
Date: Sat, 23 Mar 2013 03:19:07 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1363350489-22257-7-git-send-email-david@xxxxxxxxxxxxx>
References: <1363350489-22257-1-git-send-email-david@xxxxxxxxxxxxx> <1363350489-22257-7-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
Ok, though you might want to fix the typo on "parse" in the commit message.

Reviewed-by: Phil White <pwhite@xxxxxxx>

On Fri, Mar 15, 2013 at 11:27:50PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> The option parise in a messy loop of option parsing and actions on
> secondary arguments. Turn it into something much neater and esay to
> understand rather than a mess of temporary variables and tortured
> logic...
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  check     |  420 
> +++++++++++++++++++++++--------------------------------------
>  common.rc |   19 +++
>  2 files changed, 174 insertions(+), 265 deletions(-)
> 
> diff --git a/check b/check
> index 57c143a..7d563e9 100755
> --- a/check
> +++ b/check
> @@ -28,47 +28,41 @@ n_bad=0
>  bad=""
>  notrun=""
>  interrupt=true
> +diff="diff -u"
> +showme=false
> +expunge=true
> +have_test_arg=false
> +randomize=false
> +here=`pwd`
> +FSTYP=xfs
> +
> +SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
> +
> +# generic initialization
> +iam=check
>  
>  export QA_CHECK_FS=${QA_CHECK_FS:=true}
> -# number of diff lines from a failed test, 0 for whole output
> -export DIFF_LENGTH=${DIFF_LENGTH:=10}
>  
>  # by default don't output timestamps
>  timestamp=${TIMESTAMP:=false}
>  
> -# generic initialization
> -iam=check
> -
> -# we need common.config
> -if ! . ./common.config
> -then
> -    echo "$iam: failed to source common.config"
> -    exit 1
> -fi
> +# number of diff lines from a failed test, 0 for whole output
> +export DIFF_LENGTH=${DIFF_LENGTH:=10}
>  
> -# argument parsing first - currently very messy, needs cleanup.
> -_setenvironment()
> -{
> -    MSGVERB="text:action"
> -    export MSGVERB
> -}
> +# by default don't output timestamps
> +timestamp=${TIMESTAMP:=false}
>  
>  usage()
>  {
>      echo "Usage: $0 [options] [testlist]"'
>  
> -common options
> -    -v                       verbose
> -
>  check options
>      -xfs                test XFS (default)
>      -udf                test UDF
>      -nfs                test NFS
>      -l                       line mode diff
> -    -xdiff           graphical mode diff
>      -udiff           show unified diff (default)
>      -n                       show me, do not run tests
> -    -q                       quick [deprecated]
>      -T                       output timestamps
>      -r                       randomize test order
>      --large-fs               optimise scratch device for large filesystems
> @@ -82,258 +76,181 @@ testlist options
>           exit 0
>  }
>  
> -here=`pwd`
> -rm -f $here/$iam.out
> -_setenvironment
> -
> -check=${check-true}
> +_setenvironment()
> +{
> +    MSGVERB="text:action"
> +    export MSGVERB
> +}
>  
> -diff="diff -u"
> -verbose=false
> -group=false
> -xgroup=false
> -showme=false
> -sortme=false
> -expunge=true
> -have_test_arg=false
> -randomize=false
> -rm -f $tmp.list $tmp.tmp $tmp.sed
> +get_group_list()
> +{
> +     grp=$1
>  
> -# Autodetect fs type based on what's on $TEST_DEV
> -if [ "$HOSTOS" == "Linux" ]
> -then
> -    export FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> -else
> -    export FSTYP=xfs
> -fi
> +     grpl=$(sed -n < group \
> +                    -e 's/#.*//' \
> +                    -e 's/$/ /' \
> +                    -e "/^[0-9][0-9][0-9].* $grp /"'{ s/ .*//p }')
> +     echo $grpl
> +}
>  
> -for r
> -do
> +expand_test_numbers()
> +{
> +     # strip leading zeros, could be considered octal.
> +     start=`echo $1 | sed 's/^0*//'`
> +     end=`echo $2 | sed 's/^0*//'`
>  
> -    if $group
> -    then
> -     # arg after -g
> -       group_list=$(sed -n < group \
> -                       -e 's/#.*//' \
> -                       -e 's/$/ /' \
> -                       -e "/^[0-9][0-9][0-9].* $r /"'{ s/ .*//p }')
> -     if [ -z "$group_list" ]
> -     then
> -         echo "Group \"$r\" is empty or not defined?"
> -         exit 1
> -     fi
> -     [ ! -s $tmp.list ] && touch $tmp.list
> -     for t in $group_list
> +     $AWK_PROG </dev/null '
> +BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
> +     | while read id
>       do
> -         if grep -s "^$t\$" $tmp.list >/dev/null
> -         then
> -             :
> -         else
> -             echo "$t" >>$tmp.list
> -         fi
> +             if grep -s "^$id " group >/dev/null ; then
> +                     # in group file ... OK
> +                     echo $id >>$tmp.list
> +             elif [ -f expunged ] && $expunge && \
> +                             egrep "^$id([   ]|\$)" expunged >/dev/null ; 
> then
> +                     # expunged ... will be reported, but not run, later
> +                     echo $id >>$tmp.list
> +             else
> +                     # oops
> +                     echo "$id - unknown test, ignored"
> +             fi
>       done
> -     group=false
> -     continue
> +}
>  
> -    elif $xgroup
> -    then
> -     # arg after -x
> -     [ ! -s $tmp.list ] && ls [0-9][0-9][0-9] [0-9][0-9][0-9][0-9] 
> >$tmp.list 2>/dev/null
> -     group_list=`sed -n <group -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{
> -s/ .*//p
> -}'`
> -     if [ -z "$group_list" ]
> -     then
> -         echo "Group \"$r\" is empty or not defined?"
> -         exit 1
> -     fi
> -     numsed=0
> -     rm -f $tmp.sed
> -     for t in $group_list
> -     do
> -         if [ $numsed -gt 100 ]
> -         then
> -             sed -f $tmp.sed <$tmp.list >$tmp.tmp
> -             mv $tmp.tmp $tmp.list
> -             numsed=0
> -             rm -f $tmp.sed
> -         fi
> -         echo "/^$t\$/d" >>$tmp.sed
> -         numsed=`expr $numsed + 1`
> -     done
> -     sed -f $tmp.sed <$tmp.list >$tmp.tmp
> -     mv $tmp.tmp $tmp.list
> -     xgroup=false
> -     continue
> -    fi
> +_wallclock()
> +{
> +    date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
> +}
>  
> -    xpand=true
> -    case "$r"
> -    in
> +_timestamp()
> +{
> +    now=`date "+%T"`
> +    echo -n " [$now]"
> +}
>  
> -     -\? | -h | --help)      # usage
> -         usage
> -         ;;
> +# start the initialisation work now
> +_setenvironment
>  
> -     -udf)   # -udf ... set FSTYP to udf
> -         FSTYP=udf
> -         xpand=false
> -         ;;
> +rm -f $tmp.list $tmp.tmp $tmp.sed $here/$iam.out
>  
> -     -xfs)   # -xfs ... set FSTYP to xfs
> -         FSTYP=xfs
> -         xpand=false
> -         ;;
> +# Autodetect fs type based on what's on $TEST_DEV
> +if [ "$HOSTOS" == "Linux" ]; then
> +    FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
> +fi
> +export FSTYP
>  
> -     -nfs)   # -nfs ... set FSTYP to nfs
> -         FSTYP=nfs
> -         xpand=false
> -         ;;
> +# we need common.config
> +if ! . ./common.config
> +then
> +    echo "$iam: failed to source common.config"
> +    exit 1
> +fi
>  
> -     -g)     # -g group ... pick from group file
> -         group=true
> -         xpand=false
> -         ;;
> +while [ $# -gt 0 ]; do
> +     case "$1" in
> +     -\? | -h | --help) usage ;;
>  
> -     -l)     # line mode for diff, was default before
> -         diff="diff"
> -         xpand=false
> -         ;;
> +     -udf)   FSTYP=udf ;;
> +     -xfs)   FSTYP=xfs ;;
> +     -nfs)   FSTYP=nfs ;;
>  
> -     -xdiff) # graphical diff mode
> -         xpand=false
> +     -g)     group=$2 ; shift ;
> +             group_list=$(get_group_list $group)
> +             if [ -z "$group_list" ]; then
> +                 echo "Group \"$group\" is empty or not defined?"
> +                 exit 1
> +             fi
>  
> -         if [ ! -z "$DISPLAY" ]
> -         then
> -             which xdiff >/dev/null 2>&1 && diff=xdiff
> -             which gdiff >/dev/null 2>&1 && diff=gdiff
> -             which tkdiff >/dev/null 2>&1 && diff=tkdiff
> -             which xxdiff >/dev/null 2>&1 && diff=xxdiff
> -         fi
> -         ;;
> -
> -     -udiff) # show a unified diff, default now, keep for backward compat
> -         xpand=false
> -         diff="$diff -u"
> -         ;;
> -
> -     -q)     # "quick", no longer used - always quick :-)
> -         xpand=false
> -         ;;
> -
> -     -n)     # show me, don't do it
> -         showme=true
> -         xpand=false
> -         ;;
> -        -r)  # randomize test order
> -         randomize=true
> -         xpand=false
> -         ;;
> -
> -     -T)     # turn on timestamp output
> -         timestamp=true
> -         xpand=false
> -         ;;
> -
> -     -v)
> -         verbose=true
> -         xpand=false
> -         ;;
> -     -x)     # -x group ... exclude from group file
> -         xgroup=true
> -         xpand=false
> -         ;;
> -     '[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]')
> -         echo "No tests?"
> -         status=1
> -         exit $status
> -         ;;
> +             [ ! -s $tmp.list ] && touch $tmp.list
> +             for t in $group_list; do
> +                     grep -s "^$t\$" $tmp.list >/dev/null || \
> +                                                     echo "$t" >>$tmp.list
> +             done
>  
> -     [0-9]*-[0-9]*)
> -         eval `echo $r | sed -e 's/^/start=/' -e 's/-/ end=/'`
> -         ;;
> +             ;;
>  
> -     [0-9]*-)
> -         eval `echo $r | sed -e 's/^/start=/' -e 's/-//'`
> -         end=`echo [0-9][0-9][0-9] [0-9][0-9][0-9][0-9] | sed -e 
> 's/\[0-9]//g' -e 's/  *$//' -e 's/.* //'`
> -         if [ -z "$end" ]
> -         then
> -             echo "No tests in range \"$r\"?"
> -             status=1
> -             exit $status
> -         fi
> -         ;;
> +     -x)     xgroup=$2 ; shift ;
> +             [ ! -s $tmp.list ] &&  ls $SUPPORTED_TESTS >$tmp.list 
> 2>/dev/null
> +             group_list=$(get_group_list $xgroup)
> +             if [ -z "$group_list" ]; then
> +                 echo "Group \"$xgroup\" is empty or not defined?"
> +                 exit 1
> +             fi
>  
> -     --large-fs)
> -         export LARGE_SCRATCH_DEV=yes
> -         xpand=false
> -         ;;
> +             rm -f $tmp.sed
> +             numsed=0
> +             for t in $group_list
> +             do
> +                 if [ $numsed -gt 100 ]; then
> +                     sed -f $tmp.sed <$tmp.list >$tmp.tmp
> +                     mv $tmp.tmp $tmp.list
> +                     numsed=0
> +                     rm -f $tmp.sed
> +                 fi
> +                 echo "/^$t\$/d" >>$tmp.sed
> +                 numsed=`expr $numsed + 1`
> +             done
> +             sed -f $tmp.sed <$tmp.list >$tmp.tmp
> +             mv $tmp.tmp $tmp.list
> +             ;;
>  
> -     -*)
> -         usage
> -         ;;
> +     -l)     diff="diff" ;;
> +     -udiff) diff="$diff -u" ;;
>  
> -     --extra-space=*)
> -         export SCRATCH_DEV_EMPTY_SPACE=${r#*=}
> -         xpand=false
> -         ;;
> +     -n)     showme=true ;;
> +        -r)  randomize=true ;;
>  
> -     *)
> -         start=$r
> -         end=$r
> -         ;;
> +     -T)     timestamp=true ;;
>  
> -    esac
> +     "$SUPPORTED_TESTS")
> +             echo "No tests?"
> +             status=1
> +             exit $status
> +             ;;
>  
> -    # get rid of leading 0s as can be interpreted as octal
> -    start=`echo $start | sed 's/^0*//'`
> -    end=`echo $end | sed 's/^0*//'`
> +     [0-9]*-[0-9]*)
> +             eval `echo $1 | sed -e 's/^/start=/' -e 's/-/ end=/'`
> +             expand_test_numbers $start $end
> +             have_test_arg=true
> +             ;;
>  
> -    if $xpand
> -    then
> -     have_test_arg=true
> -     $AWK_PROG </dev/null '
> -BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
> -     | while read id
> -     do
> -         if grep -s "^$id " group >/dev/null
> -         then
> -             # in group file ... OK
> -             echo $id >>$tmp.list
> -         else
> -             if [ -f expunged ] && $expunge && egrep "^$id([         ]|\$)" 
> expunged >/dev/null
> -             then
> -                 # expunged ... will be reported, but not run, later
> -                 echo $id >>$tmp.list
> -             else
> -                 # oops
> -                 echo "$id - unknown test, ignored"
> +     [0-9]*-)
> +             eval `echo $1 | sed -e 's/^/start=/' -e 's/-//'`
> +             end=`echo $SUPPORTED_TESTS | sed -e 's/\[0-9]//g' -e 's/  *$//' 
> -e 's/.* //'`
> +             if [ -z "$end" ]; then
> +                     echo "No tests in range \"$1\"?"
> +                     status=1
> +                     exit $status
>               fi
> -         fi
> -     done
> -    fi
> +             expand_test_numbers $start $end
> +             have_test_arg=true
> +             ;;
> +
> +     --large-fs) export LARGE_SCRATCH_DEV=yes ;;
> +     --extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
> +
> +     -*)     usage ;;
> +     *)      expand_test_numbers $1 $1 ;
> +             have_test_arg=true
> +             ;;
> +     esac
>  
> +     shift
>  done
>  
> -if [ -s $tmp.list ]
> -then
> +if [ -s $tmp.list ]; then
>      # found some valid test numbers ... this is good
>      :
> -else
> -    if $have_test_arg
> -    then
> +elif $have_test_arg; then
>       # had test numbers, but none in group file ... do nothing
>       touch $tmp.list
> -    else
> +else
>       # no test numbers, do everything from group file
>       sed -n -e '/^[0-9][0-9][0-9]*/s/[       ].*//p' <group >$tmp.list
> -    fi
>  fi
>  
> -# should be sort -n, but this did not work for Linux when this
> -# was ported from IRIX
> -#
> -list=`sort $tmp.list`
> +# sort the list of tests into numeric order
> +list=`sort -n $tmp.list`
>  rm -f $tmp.list $tmp.tmp $tmp.sed
>  
>  if $randomize
> @@ -341,24 +258,6 @@ then
>      list=`echo $list | awk -f randomize.awk`
>  fi
>  
> -case "$FSTYP" in
> -    xfs)
> -      [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found"
> -      [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found"
> -      [ "$XFS_CHECK_PROG" = "" ] && _fatal "xfs_check not found"
> -      [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found"
> -      [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found" 
> -      ;;
> -    udf)
> -      [ "$MKFS_UDF_PROG" = "" ] && _fatal "mkfs_udf/mkudffs not found"
> -      ;;
> -    btrfs)
> -      [ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found"
> -      ;;
> -    nfs)
> -      ;;
> -esac
> -
>  # we need common.rc
>  if ! . ./common.rc
>  then
> @@ -372,16 +271,7 @@ then
>      exit 1
>  fi
>  
> -_wallclock()
> -{
> -    date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
> -}
> -
> -_timestamp()
> -{
> -    now=`date "+%T"`
> -    echo -n " [$now]"
> -}
> +# Ok, time to start running...
>  
>  _wrapup()
>  {
> diff --git a/common.rc b/common.rc
> index 0972d15..0babfed 100644
> --- a/common.rc
> +++ b/common.rc
> @@ -161,6 +161,25 @@ then
>      fi
>  fi
>  
> +# check for correct setup
> +case "$FSTYP" in
> +    xfs)
> +      [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found"
> +      [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found"
> +      [ "$XFS_CHECK_PROG" = "" ] && _fatal "xfs_check not found"
> +      [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found"
> +      [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found" 
> +      ;;
> +    udf)
> +      [ "$MKFS_UDF_PROG" = "" ] && _fatal "mkfs_udf/mkudffs not found"
> +      ;;
> +    btrfs)
> +      [ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found"
> +      ;;
> +    nfs)
> +      ;;
> +esac
> +
>  # make sure we have a standard umask
>  umask 022
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>