Re: [bug-patch] patch 2.7.1 tests fail when $(SHELL) is not bash or bash-like

[Harald van Dijk]

On 11/13/2012 07:17 PM, Harald van Dijk wrote:
> Hi,
>
> patch 2.7.1 appears to build fine here, but several tests report bogus
> errors due to nonportable shell script constructs in the testsuite.
> These may also have been present in older versions, but I have only
> tested 2.7.1. On my system, /bin/sh is dash, and that is what runs the
> tests. Firstly, a major problem:
>
> --- patch-2.7.1/tests/test-lib.sh
> +++ patch-2.7.1/tests/test-lib.sh
> @@ -118,7 +118,7 @@
>  }
>  
>  if test -z "`echo -n`"; then
> -    if eval 'test -n "${BASH_LINENO[0]}" 2>/dev/null'; then
> +    if (eval 'test -n "${BASH_LINENO[0]}"') 2>/dev/null; then
>      eval '
>          _start_test() {
>          echo -n "[${BASH_LINENO[2]}] $* -- "
>
> This needs to run in a subshell to prevent it from terminating the
> currently executing shell. dash reports a syntax error on
> ${BASH_LINENO[0]} and just exits, and no tests run at all.
>
> Secondly, echo -e is nonportable (see `man 1p echo` on most Linux
> systems), and doesn't behave on dash as it does on bash, causing a test
> failure in crlf-handling.
>
> --- patch-2.7.1/tests/crlf-handling
> +++ patch-2.7.1/tests/crlf-handling
> @@ -14,7 +14,7 @@
>  use_tmpdir
>  
>  lf2crlf() {
> -    while read l; do echo -e "$l\r"; done
> +    while read l; do printf "%s\r\n" "$l"; done
>  }
>  
>  echo 1 > a
>
> printf is valid POSIX sh, which is what I've replaced it with, but I
> don't know how portable it is to not-quite-POSIX shells that you may
> wish to support.
>
> Thirdly, test a == b is specific to bash, the standard syntax is test a
> = b. Fourthly, shift may report an error when shifting more than the
> remaining arguments (bash -c 'set --; shift' reports no error, dash -c
> 'set --; shift' reports "dash: 1: shift: can't shift that many"). These
> both appear in tests/merge:
>
> --- patch-2.7.1/tests/merge
> +++ patch-2.7.1/tests/merge
> @@ -32,18 +32,20 @@
>      shift
>      done > a.sed
>      echo "$body" | sed -f a.sed > b
> -    shift
> -    while test $# -gt 0 ; do
> -    echo "$1"
> +    if test $# -gt 0 ; then
>      shift
> -    done > b.sed
> +    while test $# -gt 0 ; do
> +        echo "$1"
> +        shift
> +    done
> +    fi > b.sed
>      echo "$body" | sed -f b.sed > c
>      rm -f a.sed b.sed
>      output=`diff -u a b | patch $ARGS -f c`
>      status=$?
>      echo "$output" | sed -e '/^$/d' -e '/^patching file c$/d'
>      cat c
> -    test $status == 0 || echo "Status: $status"
> +    test $status = 0 || echo "Status: $status"
>  }
>  
>  x() {
>
> The change for shift could alternatively be solved by making sure
> there's always at least one argument (the "--"), if you like.
>
> With those changes in the testsuite, all tests pass, so patch does
> appear to be running correctly.
Follow up:

tests/read-only-files is correctly skipped when running as root, but
fails as non-root for roughly the same reason as test-lib.sh:

--- patch-2.7.1/tests/read-only-files
+++ patch-2.7.1/tests/read-only-files
@@ -16,7 +16,7 @@

 : > read-only
 chmod a-w read-only
-if : 2> /dev/null > read-only; then
+if (: > read-only) 2> /dev/null; then
     echo "Files with read-only permissions are writable" \
         "(probably running as superuser)" >&2
     exit 77

dash exits the shell completely when redirection fails for a special
built-in utility such as : (as required by POSIX), so again, a subshell
is needed to get the test to continue running.
> Cheers,
> Harald