[libvirt] [PATCH] tests: test-lib.sh portability and clean-up

Jim Meyering jim at meyering.net
Wed Mar 24 08:40:24 UTC 2010


No big deal, but I saw recent additions of "test ... -a ..."
(not portable) so fixed the rest, too.

Now, searching for violations shows none:

  git grep '\<test .* -a '

Whether it's possible to rely on test -a in test scripts is debatable:
perhaps you've ensured that the SHELL you use when running tests is
POSIX compliant or better (I do that in coreutils), but at least in
configure.ac, we should toe the line wrt portability (because *it*
has less choice), so those are in a separate commit.

Since this is a global change, it deserves a syntax-check rule.
That's the 3/3 patch, below.

1/3 fixes test-lib.sh
2/3 fixes configure.ac

>From ca7db6cb8000cc283fcee7899140d2fc892b0296 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 24 Mar 2010 09:05:27 +0100
Subject: [PATCH 1/3] tests: shell script portability and clean-up

* tests/test-lib.sh: "echo -n" is not portable.  Use printf instead.
Remove unnecessary uses of "eval-in-subshell" (subshell is sufficient).
Remove uses of tests' -a operator; it is not portable.
Instead, use "test cond && test cond2".
* tests/schematestutils.sh: Replace use of test's -a.
---
 tests/schematestutils.sh |    2 +-
 tests/test-lib.sh        |   20 ++++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh
index 301b9eb..f172857 100644
--- a/tests/schematestutils.sh
+++ b/tests/schematestutils.sh
@@ -21,7 +21,7 @@ do
     ret=$?

     test_result $n $(basename $(dirname $xml))"/"$(basename $xml) $ret
-    if test "$verbose" = "1" -a $ret != 0 ; then
+    if test "$verbose" = "1" && test $ret != 0 ; then
         echo -e "$cmd\n$result"
     fi
     if test "$ret" != 0 ; then
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 57fd438..28b830e 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -19,7 +19,7 @@ test_intro()
   name=$1
   if test "$verbose" = "0" ; then
     echo "TEST: $name"
-    echo -n "      "
+    printf "      "
   fi
 }

@@ -29,15 +29,15 @@ test_result()
   name=$2
   status=$3
   if test "$verbose" = "0" ; then
-    mod=`eval "expr \( $counter - 1 \) % 40"`
-    if test "$counter" != 1 -a "$mod" = 0 ; then
-        printf " %-3d\n" `eval "expr $counter - 1"`
-        echo -n "      "
+    mod=`expr \( $counter + 40 - 1 \) % 40`
+    if test "$counter" != 1 && test "$mod" = 0 ; then
+        printf " %-3d\n" `expr $counter - 1`
+        printf "      "
     fi
     if test "$status" = "0" ; then
-        echo -n "."
+        printf "."
     else
-        echo -n "!"
+        printf "!"
     fi
   else
     if test "$status" = "0" ; then
@@ -54,11 +54,11 @@ test_final()
   status=$2

   if test "$verbose" = "0" ; then
-    mod=`eval "expr \( $counter + 1 \) % 40"`
-    if test "$mod" != "0" -a "$mod" != "1" ; then
+    mod=`expr \( $counter + 1 \) % 40`
+    if test "$mod" != "0" && test "$mod" != "1" ; then
       for i in `seq $mod 40`
       do
-        echo -n " "
+        printf " "
       done
     fi
     if test "$status" = "0" ; then
--
1.7.0.3.435.g097f4


>From 7998714d60b997357bfea15d6f2d0f729fc8fb29 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 24 Mar 2010 09:10:13 +0100
Subject: [PATCH 2/3] build: don't use "test cond1 -a cond2" in configure: it's not portable

* configure.ac: Use "test cond1 && test cond2" instead.
---
 configure.ac |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/configure.ac b/configure.ac
index bcf1d5a..2e6d2e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,10 +197,10 @@ dnl if --prefix is /usr, don't use /usr/var for localstatedir
 dnl or /usr/etc for sysconfdir
 dnl as this makes a lot of things break in testing situations

-if test "$prefix" = "/usr" -a "$localstatedir" = '${prefix}/var' ; then
+if test "$prefix" = "/usr" && test "$localstatedir" = '${prefix}/var' ; then
     localstatedir='/var'
 fi
-if test "$prefix" = "/usr" -a "$sysconfdir" = '${prefix}/etc' ; then
+if test "$prefix" = "/usr" && test "$sysconfdir" = '${prefix}/etc' ; then
     sysconfdir='/etc'
 fi

@@ -240,7 +240,7 @@ AC_ARG_WITH([libvirtd],
 dnl
 dnl specific tests to setup DV devel environments with debug etc ...
 dnl
-if [[ "${LOGNAME}" = "veillard" -a "`pwd`" = "/u/veillard/libvirt" ]] ; then
+if [[ "${LOGNAME}" = "veillard" && test "`pwd`" = "/u/veillard/libvirt" ]] ; then
     STATIC_BINARIES="-static"
 else
     STATIC_BINARIES=
@@ -351,7 +351,7 @@ LIBXENSERVER_LIBS=""
 LIBXENSERVER_CFLAGS=""
 dnl search for the XenServer library
 if test "$with_xenapi" != "no" ; then
-    if test "$with_xenapi" != "yes" -a "$with_xenapi" != "check" ; then
+    if test "$with_xenapi" != "yes" && test "$with_xenapi" != "check" ; then
         LIBXENSERVER_CFLAGS="-I$with_xenapi/include"
         LIBXENSERVER_LIBS="-L$with_xenapi"
     fi
@@ -390,7 +390,7 @@ XEN_LIBS=""
 XEN_CFLAGS=""
 dnl search for the Xen store library
 if test "$with_xen" != "no" ; then
-    if test "$with_xen" != "yes" -a "$with_xen" != "check" ; then
+    if test "$with_xen" != "yes" && test "$with_xen" != "check" ; then
         XEN_CFLAGS="-I$with_xen/include"
         XEN_LIBS="-L$with_xen/lib64 -L$with_xen/lib"
     fi
@@ -571,7 +571,7 @@ AC_ARG_WITH([libxml], AC_HELP_STRING([--with-libxml=@<:@PFX@:>@], [libxml2 locat
 if test "x$with_libxml" = "xno" ; then
     AC_MSG_CHECKING(for libxml2 libraries >= $LIBXML_REQUIRED)
     AC_MSG_ERROR([libxml2 >= $LIBXML_REQUIRED is required for libvirt])
-elif test "x$with_libxml" = "x" -a "x$PKG_CONFIG" != "x" ; then
+elif test "x$with_libxml" = "x" && test "x$PKG_CONFIG" != "x" ; then
     PKG_CHECK_MODULES(LIBXML, libxml-2.0 >= $LIBXML_REQUIRED, [LIBXML_FOUND=yes], [LIBXML_FOUND=no])
 fi
 if test "$LIBXML_FOUND" = "no" ; then
@@ -661,7 +661,7 @@ AC_ARG_WITH([sasl],
 SASL_CFLAGS=
 SASL_LIBS=
 if test "x$with_sasl" != "xno"; then
-  if test "x$with_sasl" != "xyes" -a "x$with_sasl" != "xcheck"; then
+  if test "x$with_sasl" != "xyes" && test "x$with_sasl" != "xcheck"; then
     SASL_CFLAGS="-I$with_sasl"
     SASL_LIBS="-L$with_sasl"
   fi
@@ -716,7 +716,7 @@ AC_ARG_WITH([yajl],
 YAJL_CFLAGS=
 YAJL_LIBS=
 if test "x$with_yajl" != "xno"; then
-  if test "x$with_yajl" != "xyes" -a "x$with_yajl" != "xcheck"; then
+  if test "x$with_yajl" != "xyes" && test "x$with_yajl" != "xcheck"; then
     YAJL_CFLAGS="-I$with_yajl/include"
     YAJL_LIBS="-L$with_yajl/lib"
   fi
@@ -1004,7 +1004,7 @@ AC_ARG_WITH([numactl],

 NUMACTL_CFLAGS=
 NUMACTL_LIBS=
-if test "$with_qemu" = "yes" -a "$with_numactl" != "no"; then
+if test "$with_qemu" = "yes" && test "$with_numactl" != "no"; then
   old_cflags="$CFLAGS"
   old_libs="$LIBS"
   if test "$with_numactl" = "check"; then
@@ -1062,7 +1062,7 @@ dnl
 dnl libssh checks
 dnl

-if test "$with_libssh2" != "yes" -a "$with_libssh2" != "no"; then
+if test "$with_libssh2" != "yes" && test "$with_libssh2" != "no"; then
 		libssh2_path="$with_libssh2"
 elif test "$with_libssh2" = "yes"; then
 		libssh2_path="/usr/local/lib/"
@@ -1143,7 +1143,7 @@ dnl introduced in 0.4.0 release which need as minimum
 dnl
 CAPNG_CFLAGS=
 CAPNG_LIBS=
-if test "$with_qemu" = "yes" -a "$with_capng" != "no"; then
+if test "$with_qemu" = "yes" && test "$with_capng" != "no"; then
   old_cflags="$CFLAGS"
   old_libs="$LIBS"
   if test "$with_capng" = "check"; then
@@ -1453,7 +1453,7 @@ if test "$with_storage_disk" = "yes" -o "$with_storage_disk" = "check"; then
     PARTED_FOUND=yes
   fi

-  if test "$with_storage_disk" != "no" -a "x$PKG_CONFIG" != "x" ; then
+  if test "$with_storage_disk" != "no" && test "x$PKG_CONFIG" != "x" ; then
     PKG_CHECK_MODULES(LIBPARTED, libparted >= $PARTED_REQUIRED, [], [PARTED_FOUND=no])
   fi
   if test "$PARTED_FOUND" = "no"; then
@@ -1635,7 +1635,7 @@ else
 fi
 AC_MSG_RESULT($RUNNING_XEND)

-AM_CONDITIONAL([ENABLE_XEN_TESTS], [test "$RUNNING_XEN" != "no" -a "$RUNNING_XEND" != "no"])
+AM_CONDITIONAL([ENABLE_XEN_TESTS], [test "$RUNNING_XEN" != "no" && test "$RUNNING_XEND" != "no"])

 AC_ARG_ENABLE([test-coverage],
   AC_HELP_STRING([--enable-test-coverage], [turn on code coverage instrumentation @<:@default=no@:>@]),
--
1.7.0.3.435.g097f4


>From 95c8ddd2eca90e3024a6f74af84517c1e0115a60 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 24 Mar 2010 09:32:43 +0100
Subject: [PATCH 3/3] maint: add syntax-check rule to prohibit use of test's -a operator

* cfg.mk (sc_prohibit_test_minus_a): New rule.
---
 cfg.mk |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 2d0d278..4302338 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -269,6 +269,12 @@ sc_preprocessor_indentation:
 	  echo '$(ME): skipping test $@: cppi not installed' 1>&2;	\
 	fi

+# Using test's -a operator is not portable.
+sc_prohibit_test_minus_a:
+	@re='\<test .+ -[a] '						\
+	msg='use "test C1 && test C2, not "test C1 -''a C2"'		\
+	  $(_prohibit_regexp)
+
 sc_copyright_format:
 	@$(VC_LIST_EXCEPT) | xargs grep -ni 'copyright .*Red 'Hat	\
 	  | grep -v Inc							\
--
1.7.0.3.435.g097f4




More information about the libvir-list mailing list