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@xxxxxxxxxx>
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@xxxxxxxxxx>
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@xxxxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list