[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-patch] patch 2.7.1 tests fail when $(SHELL) is not bash or bash
From: |
Harald van Dijk |
Subject: |
Re: [bug-patch] patch 2.7.1 tests fail when $(SHELL) is not bash or bash-like |
Date: |
Tue, 13 Nov 2012 22:44:27 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:16.0) Gecko/20121104 Thunderbird/16.0.2 |
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