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..."