You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Louis Guillaume <lg...@berklee.edu> on 2007/03/22 01:57:22 UTC

Fix for portability of subversion/tests/cmdline/davautocheck.sh

Hi,

The patch below fixes the portability problems with
subversion/tests/cmdline/davautocheck.sh, which are described further below.


--- subversion/tests/cmdline/davautocheck.sh.orig       2007-03-21
22:01:10.000000000 +0000
+++ subversion/tests/cmdline/davautocheck.sh    2007-03-21
22:24:01.000000000 +0000
@@ -62,7 +62,7 @@
   echo -n "$SCRIPT: $1 (y/n)? [$2] "
   read -n 1 -t 32
   echo
-  [ "${REPLY:-$2}" == 'y' ]
+  [ "${REPLY:-$2}" = 'y' ]
 }

 function get_loadmodule_config() {
@@ -170,7 +170,7 @@
     || fail "Authz_User module not found."
 }

-HTTPD_PORT=$(($RANDOM+1024))
+HTTPD_PORT=$((($$ % 32768 ) +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"
@@ -268,7 +268,7 @@

 say "HTTPD is good, starting the tests..."

-if [ $# == 0 ]; then
+if [ $# = 0 ]; then
   time make check "BASE_URL=$BASE_URL"
   r=$?
 else










#######################################
Hello,

I just attempted to build subversion from the pkgsrc environment on NetBSD.
Pkgsrc recently gained some portability-checking routines, which are quite
nice.

Problems are encountered with the subversion-base package, see below. While
this is easy to get around, these seem like simple fixes that would ensure
the portability of subversion.

Not sure how to fix the $RANDOM issue but the test one seems pretty
straightforward...

Louis



=> Checking for portability problems in extracted files
ERROR: [check-portability.awk] => Found test ... == ...:
ERROR: [check-portability.awk] subversion/tests/cmdline/davautocheck.sh:
[ "${REPLY:-$2}" == 'y' ]
WARNING: [check-portability.awk] => Found $RANDOM:
WARNING: [check-portability.awk] subversion/tests/cmdline/davautocheck.sh:
HTTPD_PORT=$(($RANDOM+1024))
ERROR: [check-portability.awk] => Found test ... == ...:
ERROR: [check-portability.awk] subversion/tests/cmdline/davautocheck.sh:
if [
$# == 0 ]; then

Explanation:
===========================================================================
The variable $RANDOM is not required for a POSIX-conforming shell, and
many implementations of /bin/sh do not support it. It should therefore
not be used in shell programs that are meant to be portable across a
large number of POSIX-like systems.
===========================================================================


Explanation:
===========================================================================
The "test" command, as well as the "[" command, are not required to know
the "==" operator. Only a few implementations like bash and some
versions of ksh support it.

When you run "test foo == foo" on a platform that does not support the
"==" operator, the result will be "false" instead of "true". This can
lead to unexpected behavior.

There are two ways to fix this error message. If the file that contains
the "test ==" is needed for building the package, you should create a
patch for it, replacing the "==" operator with "=". If the file is not
needed, add its name to the CHECK_PORTABILITY_SKIP variable in the
package Makefile.
===========================================================================

*** Error code 1

Stop.
make: stopped in /usr/pkgsrc/devel/subversion-base

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Fix for portability of subversion/tests/cmdline/davautocheck.sh

Posted by Louis Guillaume <lg...@berklee.edu>.
Blair Zajac wrote:
> Hi Louis,
> 
> To get these fixes into Subversion, I recommend:
> 
> 1) Emailing this to dev@subversion.tigris.org.
> 2) Putting the [PATCH] string in the subject title.
> 3) Attaching the patch instead of including it inline, but have the
> patch end in .txt so that it's reabable in email clients.
> 4) Add a log message.  See this URL on how to write a log message:
> 
>    http://subversion.tigris.org/hacking.html#log-messages
> 
> While some of this is extra work, it'll make it easier for it to get
> into the code base, work that otherwise a committer would have to do.
> 
> BTW, which platform are you on that these fixes are needed?
> 
> Regards,
> Blair
> 


Thank you Blair; will do.

I noticed the problem when pkgsrc found these non-portable things: (1) the "==" operator for "[" or "test" and (2) the use of
$RANDOM. Both of these items are not part of the POSIX specification, so any sh that adheres strictly to POSIX is not required to
implement these.

For example, the version of sh that is delivered with NetBSD does not support either of these, so here is a case where things would
definitely break.


Louis

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Fix for portability of subversion/tests/cmdline/davautocheck.sh

Posted by Blair Zajac <bl...@orcaware.com>.
Hi Louis,

To get these fixes into Subversion, I recommend:

1) Emailing this to dev@subversion.tigris.org.
2) Putting the [PATCH] string in the subject title.
3) Attaching the patch instead of including it inline, but have the 
patch end in .txt so that it's reabable in email clients.
4) Add a log message.  See this URL on how to write a log message:

    http://subversion.tigris.org/hacking.html#log-messages

While some of this is extra work, it'll make it easier for it to get 
into the code base, work that otherwise a committer would have to do.

BTW, which platform are you on that these fixes are needed?

Regards,
Blair

Louis Guillaume wrote:
> Sorry, that patch was mangled by line breaks... A good version is below.
> 
> Louis Guillaume wrote:
>> The patch below fixes the portability problems with
>> subversion/tests/cmdline/davautocheck.sh
> 
> --- subversion/tests/cmdline/davautocheck.sh.orig       2007-03-21 22:01:10.000000000 +0000
> +++ subversion/tests/cmdline/davautocheck.sh    2007-03-21 22:24:01.000000000 +0000
> @@ -62,7 +62,7 @@
>    echo -n "$SCRIPT: $1 (y/n)? [$2] "
>    read -n 1 -t 32
>    echo
> -  [ "${REPLY:-$2}" == 'y' ]
> +  [ "${REPLY:-$2}" = 'y' ]
>  }
> 
>  function get_loadmodule_config() {
> @@ -170,7 +170,7 @@
>      || fail "Authz_User module not found."
>  }
> 
> -HTTPD_PORT=$(($RANDOM+1024))
> +HTTPD_PORT=$((($$ % 32768 ) +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"
> @@ -268,7 +268,7 @@
> 
>  say "HTTPD is good, starting the tests..."
> 
> -if [ $# == 0 ]; then
> +if [ $# = 0 ]; then
>    time make check "BASE_URL=$BASE_URL"
>    r=$?
>  else

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Fix for portability of subversion/tests/cmdline/davautocheck.sh

Posted by Louis Guillaume <lg...@berklee.edu>.
Sorry, that patch was mangled by line breaks... A good version is below.

Louis Guillaume wrote:
> The patch below fixes the portability problems with
> subversion/tests/cmdline/davautocheck.sh

--- subversion/tests/cmdline/davautocheck.sh.orig       2007-03-21 22:01:10.000000000 +0000
+++ subversion/tests/cmdline/davautocheck.sh    2007-03-21 22:24:01.000000000 +0000
@@ -62,7 +62,7 @@
   echo -n "$SCRIPT: $1 (y/n)? [$2] "
   read -n 1 -t 32
   echo
-  [ "${REPLY:-$2}" == 'y' ]
+  [ "${REPLY:-$2}" = 'y' ]
 }

 function get_loadmodule_config() {
@@ -170,7 +170,7 @@
     || fail "Authz_User module not found."
 }

-HTTPD_PORT=$(($RANDOM+1024))
+HTTPD_PORT=$((($$ % 32768 ) +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"
@@ -268,7 +268,7 @@

 say "HTTPD is good, starting the tests..."

-if [ $# == 0 ]; then
+if [ $# = 0 ]; then
   time make check "BASE_URL=$BASE_URL"
   r=$?
 else

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org