Subject: check-portability.awk
To: NetBSD Users <netbsd-users@NetBSD.org>
From: Louis Guillaume <lguillaume@berklee.edu>
List: netbsd-users
Date: 03/27/2007 01:42:01
I recently posted to the dev@subversion mailing list to report some
portability issues that pkgsrc's check-portability.awk found in
subversion-base. See
http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=56991-
The response below came up. Perhaps check-portability.awk could be
improved from this??
Louis
-------- Original Message --------
Subject: Re: [PATCH] for sh portability issue in davautocheck.sh
Date: Mon, 26 Mar 2007 23:28:44 -0500
From: Peter Samuelson <peter@p12n.org>
To: Louis Guillaume <lguillaume@berklee.edu>
CC: dev@subversion.tigris.org
References: <4603C14F.6070007@berklee.edu>
[Louis Guillaume]
> * subversion/tests/cmdline/davautocheck.sh
> Fixed some instances of Bourne shell features that are not portable:
> (1) Removed all instances of `==' in `test' or `[' statements.
> (2) Replaced use of `$RANDOM' with `(( $$ % 32768 ))'.
Uh ... I don't think you really mean "Bourne shell features". Because
if you really want to target all Bourne shells, you also need to stop
using $() syntax (use ``) and $(()) syntax (use `eval`). Also, you
can't use shell functions, which is pretty inconvenient - just ask the
autoconf maintainers!
But I suppose you really meant POSIX shell compatibility, which is a
lot easier. (The POSIX shell spec is based largely on the 1988 Korn
Shell, aka ksh88.) For that, you need several more changes:
* Remove the word "function" from "function foo() {...}".
* Don't use pushd or popd; use a subshell or save the cwd by hand.
* Don't use "echo -n", do the dance to figure out whether to use -n
or \c. I believe POSIX declined to settle the matter either way.
* Don't use "read -n" or "-t", and you must pass it a variable name.
Also note (though this isn't a shell issue),
* ldd is not terribly portable. But I'm not sure what to replace
that with.
> The problem was noticed by the pkgsrc's portability checking routines: please see...
Maybe pkgsrc could learn a few tricks from checkbashisms:
svn://svn.debian.org/devscripts/trunk/scripts/checkbashisms.pl
[[[
Remove bashisms in davautocheck script so it can be run by any POSIX
shell.
* subversion/tests/cmdline/davautocheck.sh
(echo_n) New function.
(query) Use echo_n instead of echo -n. Use generic "read" without
bash extensions.
(top level) Do not use redundant "function" keyword. Eliminate use
of pushd/popd. Use ($$ % 64000) instead of $RANDOM.
Patch by: Louis Guillaume <lguillaume@berklee.edu>
Peter Samuelson <peter@p12n.org>
]]]
Index: subversion/tests/cmdline/davautocheck.sh
===================================================================
--- subversion/tests/cmdline/davautocheck.sh (revisione 24185)
+++ subversion/tests/cmdline/davautocheck.sh (copia locale)
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
# -*- mode: shell-script; -*-
# $Id$
@@ -43,28 +43,36 @@
trap stop_httpd_and_die SIGHUP SIGTERM SIGINT
-function stop_httpd_and_die() {
+stop_httpd_and_die() {
[ -e "$HTTPD_PID" ] && kill $(cat "$HTTPD_PID")
exit 1
}
-function say() {
+say() {
echo "$SCRIPT: $*"
}
-function fail() {
+fail() {
say $*
stop_httpd_and_die
}
-function query() {
- echo -n "$SCRIPT: $1 (y/n)? [$2] "
- read -n 1 -t 32
- echo
- [ "${REPLY:-$2}" == 'y' ]
+echo_n() {
+ if [ -z "$(echo -n)" ]; then
+ echo -n "$@"
+ else
+ echo "$@\c"
+ fi
}
-function get_loadmodule_config() {
+query() {
+ local reply
+ echo_n "$SCRIPT: $1 (y/n)? [$2] "
+ read reply
+ [ "${reply:-$2}" = 'y' ]
+}
+
+get_loadmodule_config() {
local SO="$($APXS -q LIBEXECDIR)/$1.so"
# shared object module?
@@ -81,7 +89,7 @@
}
# Check apxs's SBINDIR and BINDIR for given program names
-function get_prog_name() {
+get_prog_name() {
for prog in $*
do
for dir in $($APXS -q SBINDIR) $($APXS -q BINDIR)
@@ -120,9 +128,7 @@
if [ -x subversion/svn/svn ]; then
ABS_BUILDDIR=$(pwd)
elif [ -x $SCRIPTDIR/../../svn/svn ]; then
- pushd $SCRIPTDIR/../../../ >/dev/null
- ABS_BUILDDIR=$(pwd)
- popd >/dev/null
+ ABS_BUILDDIR=$(cd $SCRIPTDIR/../../..; pwd)
else
fail "Run this script from the root of Subversion's build tree!"
fi
@@ -188,7 +194,7 @@
|| fail "Authz_User module not found."
}
-HTTPD_PORT=$(($RANDOM+1024))
+HTTPD_PORT=$((($$ % 64000) + 1024))
HTTPD_ROOT="$ABS_BUILDDIR/subversion/tests/cmdline/httpd-$(date
'+%Y%m%d-%H%M%S')"
HTTPD_CFG="$HTTPD_ROOT/cfg"
HTTPD_PID="$HTTPD_ROOT/pid"
@@ -308,16 +314,16 @@
say "HTTPD is good, starting the tests..."
-if [ $# == 0 ]; then
+if [ $# = 0 ]; then
time make check "BASE_URL=$BASE_URL"
r=$?
-else
- pushd "$ABS_BUILDDIR/subversion/tests/cmdline/" >/dev/null
+else (
+ cd "$ABS_BUILDDIR/subversion/tests/cmdline/"
TEST="$1"
shift
time "$SCRIPTDIR/${TEST}_tests.py" "--url=$BASE_URL" $*
r=$?
- popd >/dev/null
+)
fi
say "Finished testing..."