[PATCH v2 modular 1/1] build.sh: use meaningful names for parameter variables

Peter Hutterer peter.hutterer at who-t.net
Sun Jan 9 16:24:12 PST 2011


On Fri, Jan 07, 2011 at 08:38:48AM -0500, Gaetan Nadon wrote:
> This makes it easier to review code diff.
> 
> Reported-by: Peter Hutterer <peter.hutterer at who-t.net>
> 
> Signed-off-by: Gaetan Nadon <memsize at videotron.ca>
> ---

thanks!

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>


Cheers,
  Peter

>  build.sh |  194 +++++++++++++++++++++++++++++++++----------------------------
>  1 files changed, 105 insertions(+), 89 deletions(-)
> 
> diff --git a/build.sh b/build.sh
> index 51fe800..a731480 100755
> --- a/build.sh
> +++ b/build.sh
> @@ -137,104 +137,110 @@ setup_buildenv() {
>  # returns:
>  #   (irrelevant)
>  failed() {
> -    echo "build.sh: \"$1\" failed on $2/$3"
> -    failed_components="$failed_components $2/$3"
> +    cmd=$1
> +    module=$2
> +    component=$3
> +    echo "build.sh: \"$cmd\" failed on $module/$component"
> +    failed_components="$failed_components $module/$component"
>  }
>  
>  # print a pretty title to separate the processing of each module
>  # arguments:
> -#   $1 - string to format into title
> +#   $1 - module
> +#   $2 - component
>  # returns:
>  #   (irrelevant)
>  module_title() {
> +    module=$1
> +    component=$2
>      # preconds
> -    if [ X"$1" = X ]; then
> +    if [ X"$module" = X ]; then
>  	return
>      fi
>  
>      echo ""
>      echo "======================================================================"
> -    echo "==  Processing module/component:  \"$1/$2\""
> +    echo "==  Processing module/component:  \"$module/$component\""
>  }
>  
>  checkfortars() {
> -    M=$1
> -    C=$2
> -    case $M in
> +    module=$1
> +    component=$2
> +    case $module in
>          "data")
> -            case $C in
> -                "cursors") C="xcursor-themes" ;;
> -                "bitmaps") C="xbitmaps" ;;
> +            case $component in
> +                "cursors") component="xcursor-themes" ;;
> +                "bitmaps") component="xbitmaps" ;;
>              esac
>              ;;
>          "font")
> -            if [ X"$C" != X"encodings" ]; then
> -                C="font-$C"
> +            if [ X"$component" != X"encodings" ]; then
> +                component="font-$component"
>              fi
>              ;;
>          "lib")
> -            case $C in
> -                "libXRes") C="libXres" ;;
> -                "libxtrans") C="xtrans" ;;
> +            case $component in
> +                "libXRes") component="libXres" ;;
> +                "libxtrans") component="xtrans" ;;
>              esac
>              ;;
>          "pixman")
> -            M="lib"
> -            C="pixman"
> +            module="lib"
> +            component="pixman"
>              ;;
>          "proto")
> -            case $C in
> -                "x11proto") C="xproto" ;;
> +            case $component in
> +                "x11proto") component="xproto" ;;
>              esac
>              ;;
>          "util")
> -            case $C in
> -                "cf") C="xorg-cf-files" ;;
> -                "macros") C="util-macros" ;;
> +            case $component in
> +                "cf") component="xorg-cf-files" ;;
> +                "macros") component="util-macros" ;;
>              esac
>              ;;
>          "xcb")
> -            case $C in
> +            case $component in
>                  "proto")
> -                    M="xcb/proto"
> -                    C="xcb-proto"
> +                    module="xcb/proto"
> +                    component="xcb-proto"
>                      ;;
>                  "pthread-stubs")
> -                    M="xcb/pthread-stubs"
> -                    C="libpthread-stubs"
> +                    module="xcb/pthread-stubs"
> +                    component="libpthread-stubs"
>                      ;;
>                  "libxcb")
> -                    M="xcb/libxcb"
> -                    C="libxcb"
> +                    module="xcb/libxcb"
> +                    component="libxcb"
>                      ;;
>                  "util")
> -                    M="xcb/util"
> -                    C="xcb-util"
> +                    module="xcb/util"
> +                    component="xcb-util"
>                      ;;
>              esac
>              ;;
>          "mesa")
> -            case $C in
> +            case $component in
>                  "drm")
> -                    M="mesa/drm"
> -                    C="libdrm"
> +                    module="mesa/drm"
> +                    component="libdrm"
>                      ;;
>                  "mesa")
> -                    M="mesa/mesa"
> -                    C="MesaLib"
> +                    module="mesa/mesa"
> +                    component="MesaLib"
>                      ;;
>              esac
>              ;;
>          "xkeyboard-config")
> -            C="xkeyboard-config"
> +            component="xkeyboard-config"
>              ;;
>          "xserver")
> -            C="xorg-server"
> +            component="xorg-server"
>              ;;
>      esac
> -    for ii in $M .; do
> +    for ii in $module .; do
>          for jj in bz2 gz; do
> -            TARFILE=`ls -1rt $ii/$C-*.tar.$jj 2> /dev/null | tail -n 1`
> +            TARFILE=`ls -1rt $ii/$component-*.tar.$jj 2> /dev/null | tail -n 1`
>              if [ X"$TARFILE" != X ]; then
>                  SRCDIR=`echo $TARFILE | sed "s,.tar.$jj,,"`
>                  SRCDIR=`echo $SRCDIR | sed "s,MesaLib,Mesa,"`
> @@ -245,7 +251,7 @@ checkfortars() {
>                      fi
>                      tar $TAROPTS $TARFILE -C $ii
>  		    if [ $? -ne 0 ]; then
> -			failed tar $1 $2
> +			failed tar $module $component
>  			return 1
>  		    fi
>                  fi
> @@ -267,13 +273,15 @@ checkfortars() {
>  #   0 - good
>  #   1 - bad
>  clone() {
> +    module=$1
> +    component=$2
>      # preconds
> -    if [ X"$1" = X ]; then
> -	echo "clone() required argument \$1 missing"
> +    if [ X"$module" = X ]; then
> +	echo "clone() required first argument is missing"
>  	return 1
>      fi
>  
> -    case $1 in
> +    case $module in
>      "pixman")
>          BASEDIR=""
>          ;;
> @@ -291,18 +299,18 @@ clone() {
>          ;;
>      esac
>  
> -    DIR="$1/$2"
> +    DIR="$module/$component"
>      GITROOT=${GITROOT:="git://anongit.freedesktop.org/git"}
>  
>      if [ ! -d "$DIR" ]; then
>          git clone "$GITROOT/$BASEDIR$DIR" "$DIR"
>          if [ $? -ne 0 ]; then
> -            echo "Failed to clone $1 module component $2. Ignoring."
> -            clonefailed_components="$clonefailed_components $1/$2"
> +            echo "Failed to clone $module module component $component. Ignoring."
> +            clonefailed_components="$clonefailed_components $module/$component"
>              return 1
>          fi
>      else
> -        echo "git cannot clone into an existing directory $1/$2"
> +        echo "git cannot clone into an existing directory $module/$component"
>  	return 1
>      fi
>  
> @@ -319,45 +327,47 @@ clone() {
>  process() {
>      needs_config=0
>  
> +    module=$1
> +    component=$2
>      # preconds
> -    if [ X"$1" = X ]; then
> -	echo "process() required argument \$1 missing"
> +    if [ X"$module" = X ]; then
> +	echo "process() required first argument is missing"
>  	return 1
>      fi
>  
> -    module_title $1 $2
> +    module_title $module $component
>  
>      SRCDIR=""
>      CONFCMD=""
> -    if [ -f $1/$2/autogen.sh ]; then
> -        SRCDIR="$1/$2"
> +    if [ -f $module/$component/autogen.sh ]; then
> +        SRCDIR="$module/$component"
>          CONFCMD="autogen.sh"
>      elif [ X"$CLONE" != X ]; then
> -        clone $1 $2
> +        clone $module $component
>          if [ $? -eq 0 ]; then
> -	    SRCDIR="$1/$2"
> +	    SRCDIR="$module/$component"
>  	    CONFCMD="autogen.sh"
>          fi
>  	needs_config=1
>      else
> -        checkfortars $1 $2
> +        checkfortars $module $component
>          CONFCMD="configure"
>      fi
>  
>      if [ X"$SRCDIR" = X ]; then
> -        echo "$1 module component $2 does not exist, skipping."
> -        nonexistent_components="$nonexistent_components $1/$2"
> +        echo "$module module component $component does not exist, skipping."
> +        nonexistent_components="$nonexistent_components $module/$component"
>          return 0
>      fi
>  
>      if [ X"$BUILT_MODULES_FILE" != X ]; then
> -        echo "$1/$2" >> $BUILT_MODULES_FILE
> +        echo "$module/$component" >> $BUILT_MODULES_FILE
>      fi
>  
>      old_pwd=`pwd`
>      cd $SRCDIR
>      if [ $? -ne 0 ]; then
> -	failed cd1 $1 $2
> +	failed cd1 $module $component
>  	return 1
>      fi
>  
> @@ -367,7 +377,7 @@ process() {
>  	cd $old_pwd
>  
>  	if [ $rtn -ne 0 ]; then
> -	    failed "$GITCMD" $1 $2
> +	    failed "$GITCMD" $module $component
>  	    return 1
>  	fi
>  	return 0
> @@ -376,7 +386,7 @@ process() {
>      if [ X"$PULL" != X ]; then
>  	git pull --rebase
>  	if [ $? -ne 0 ]; then
> -	    failed "git pull" $1 $2
> +	    failed "git pull" $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -386,13 +396,13 @@ process() {
>      if [ X"$DIR_ARCH" != X ] ; then
>  	mkdir -p "$DIR_ARCH"
>  	if [ $? -ne 0 ]; then
> -	    failed mkdir $1 $2
> +	    failed mkdir $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
>  	cd "$DIR_ARCH"
>  	if [ $? -ne 0 ]; then
> -	    failed cd2 $1 $2
> +	    failed cd2 $module $component
>  	    cd ${old_pwd}
>  	    return 1
>  	fi
> @@ -416,7 +426,7 @@ process() {
>  	    ${CFLAGS:+CFLAGS="$CFLAGS"} \
>  	    ${LDFLAGS:+LDFLAGS="$LDFLAGS"}
>  	if [ $? -ne 0 ]; then
> -	    failed ${CONFCMD} $1 $2
> +	    failed ${CONFCMD} $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -429,7 +439,7 @@ process() {
>  	cd $old_pwd
>  
>  	if [ $rtn -ne 0 ]; then
> -	    failed "$MAKE $MAKEFLAGS $MAKECMD" $1 $2
> +	    failed "$MAKE $MAKEFLAGS $MAKECMD" $module $component
>  	    return 1
>  	fi
>  	return 0
> @@ -437,7 +447,7 @@ process() {
>  
>      ${MAKE} $MAKEFLAGS
>      if [ $? -ne 0 ]; then
> -	failed "$MAKE $MAKEFLAGS" $1 $2
> +	failed "$MAKE $MAKEFLAGS" $module $component
>  	cd $old_pwd
>  	return 1
>      fi
> @@ -445,7 +455,7 @@ process() {
>      if [ X"$CHECK" != X ]; then
>  	${MAKE} $MAKEFLAGS check
>  	if [ $? -ne 0 ]; then
> -	    failed "$MAKE $MAKEFLAGS check" $1 $2
> +	    failed "$MAKE $MAKEFLAGS check" $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -454,7 +464,7 @@ process() {
>      if [ X"$CLEAN" != X ]; then
>  	${MAKE} $MAKEFLAGS clean
>  	if [ $? -ne 0 ]; then
> -	    failed "$MAKE $MAKEFLAGS clean" $1 $2
> +	    failed "$MAKE $MAKEFLAGS clean" $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -463,7 +473,7 @@ process() {
>      if [ X"$DIST" != X ]; then
>  	${MAKE} $MAKEFLAGS dist
>  	if [ $? -ne 0 ]; then
> -	    failed "$MAKE $MAKEFLAGS dist" $1 $2
> +	    failed "$MAKE $MAKEFLAGS dist" $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -472,7 +482,7 @@ process() {
>      if [ X"$DISTCHECK" != X ]; then
>  	${MAKE} $MAKEFLAGS distcheck
>  	if [ $? -ne 0 ]; then
> -	    failed "$MAKE $MAKEFLAGS distcheck" $1 $2
> +	    failed "$MAKE $MAKEFLAGS distcheck" $module $component
>  	    cd $old_pwd
>  	    return 1
>  	fi
> @@ -480,7 +490,7 @@ process() {
>  
>      $SUDO env LD_LIBRARY_PATH=$LD_LIBRARY_PATH ${MAKE} $MAKEFLAGS install
>      if [ $? -ne 0 ]; then
> -	failed "$SUDO env LD_LIBRARY_PATH=$LD_LIBRARY_PATH $MAKE $MAKEFLAGS install" $1 $2
> +	failed "$SUDO env LD_LIBRARY_PATH=$LD_LIBRARY_PATH $MAKE $MAKEFLAGS install" $module $component
>  	cd $old_pwd
>  	return 1
>      fi
> @@ -498,24 +508,26 @@ process() {
>  #   0 - good
>  #   1 - bad
>  build() {
> +    module=$1
> +    component=$2
>      if [ X"$LISTONLY" != X ]; then
> -	echo "$1/$2"
> +	echo "$module/$component"
>  	return 0
>      fi
>  
>      if [ X"$RESUME" != X ]; then
> -	if [ X"$RESUME" = X"$1/$2" ]; then
> +	if [ X"$RESUME" = X"$module/$component" ]; then
>  	    unset RESUME
>  	    # Resume build at this module
>  	else
> -	    echo "Skipping $1 module component $2..."
> +	    echo "Skipping $module module component $component..."
>  	    return 0
>  	fi
>      fi
>  
> -    process $1 $2
> +    process $module $component
>      if [ $? -ne 0 ]; then
> -	echo "build.sh: error processing module/component:  \"$1/$2\""
> +	echo "build.sh: error processing module/component:  \"$module/$component\""
>  	if [ X"$NOQUIT" = X ]; then
>  	    exit 1
>  	fi
> @@ -1021,8 +1033,10 @@ usage() {
>  # returns:
>  #   returns nothing or exit on error with message
>  check_full_path () {
> -    if [ X"`expr substr $1 1 1`" != X/ ]; then
> -	echo "The path \"$1\" supplied by \"$2\" must be a full path name"
> +    path=$1
> +    varname=$2
> +    if [ X"`expr substr $path 1 1`" != X/ ]; then
> +	echo "The path \"$path\" supplied by \"$varname\" must be a full path name"
>  	echo ""
>  	usage
>  	exit 1
> @@ -1036,11 +1050,11 @@ check_full_path () {
>  # returns:
>  #   returns nothing or exit on error with message
>  check_writable_dir () {
> -    dir=$1
> +    path=$1
>      varname=$2
>      if [ X"$SUDO" = X ]; then
> -	if [ ! -d "$dir" ] || [ ! -w "$dir" ]; then
> -	    echo "The path \"$dir\" supplied by \"$varname\" must be a writable directory"
> +	if [ ! -d "$path" ] || [ ! -w "$path" ]; then
> +	    echo "The path \"$path\" supplied by \"$varname\" must be a writable directory"
>  	    echo ""
>  	    usage
>  	    exit 1
> @@ -1056,24 +1070,26 @@ check_writable_dir () {
>  #   if it returns, everything is good
>  #   otherwise it exit's
>  required_arg() {
> +    option=$1
> +    arg=$2
>      # preconds
> -    if [ X"$1" = X ]; then
> -	echo "internal required_arg() error, missing \$1 argument"
> +    if [ X"$option" = X ]; then
> +	echo "internal required_arg() error, missing first argument"
>  	exit 1
>      fi
>  
>      # check for an argument
> -    if [ X"$2" = X ]; then
> -	echo "the '$1' option is missing its required argument"
> +    if [ X"$arg" = X ]; then
> +	echo "the '$option' option is missing its required argument"
>  	echo ""
>  	usage
>  	exit 1
>      fi
>  
>      # does the argument look like an option?
> -    echo $2 | grep "^-" > /dev/null
> +    echo $arg | grep "^-" > /dev/null
>      if [ $? -eq 0 ]; then
> -	echo "the argument '$2' of option '$1' looks like an option itself"
> +	echo "the argument '$arg' of option '$option' looks like an option itself"
>  	echo ""
>  	usage
>  	exit 1
> -- 
> 1.6.0.4



More information about the xorg-devel mailing list