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: Jim Meyering <meyering(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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