* [PATCH] t1402: work around shell quoting issue on NetBSD
@ 2013-01-08 20:23 René Scharfe
2013-01-08 20:39 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: René Scharfe @ 2013-01-08 20:23 UTC (permalink / raw
To: git discussion list; +Cc: Junio C Hamano, Jonathan Nieder, Michael Haggerty
The test fails for me on NetBSD 6.0.1 and reports:
ok 1 - ref name '' is invalid
ok 2 - ref name '/' is invalid
ok 3 - ref name '/' is invalid with options --allow-onelevel
ok 4 - ref name '/' is invalid with options --normalize
error: bug in the test script: not 2 or 3 parameters to test-expect-success
The alleged bug is in this line:
invalid_ref NOT_MINGW '/' '--allow-onelevel --normalize'
invalid_ref() constructs a test case description using its last argument,
but the shell seems to split it up into two pieces if it contains a
space. Minimal test case:
# on NetBSD with /bin/sh
$ a() { echo $#-$1-$2; }
$ t="x"; a "${t:+$t}"
1-x-
$ t="x y"; a "${t:+$t}"
2-x-y
$ t="x y"; a "${t:+x y}"
1-x y-
# and with bash
$ t="x y"; a "${t:+$t}"
1-x y-
$ t="x y"; a "${t:+x y}"
1-x y-
This may be a bug in the shell, but here's a simple workaround: Construct
the description string first and store it in a variable, and then use
that to call test_expect_success().
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t1402-check-ref-format.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1ae4d87..1a5a5f3 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -11,7 +11,8 @@ valid_ref() {
prereq=$1
shift
esac
- test_expect_success $prereq "ref name '$1' is valid${2:+ with options $2}" "
+ desc="ref name '$1' is valid${2:+ with options $2}"
+ test_expect_success $prereq "$desc" "
git check-ref-format $2 '$1'
"
}
@@ -22,7 +23,8 @@ invalid_ref() {
prereq=$1
shift
esac
- test_expect_success $prereq "ref name '$1' is invalid${2:+ with options $2}" "
+ desc="ref name '$1' is invalid${2:+ with options $2}"
+ test_expect_success $prereq "$desc" "
test_must_fail git check-ref-format $2 '$1'
"
}
--
1.7.12
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
2013-01-08 20:23 [PATCH] t1402: work around shell quoting issue on NetBSD René Scharfe
@ 2013-01-08 20:39 ` Junio C Hamano
2013-01-08 21:13 ` René Scharfe
2013-01-09 1:27 ` [PATCH] t1402: work around shell quoting issue on NetBSD Greg Troxel
2013-01-09 2:07 ` Greg Troxel
2 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2013-01-08 20:39 UTC (permalink / raw
To: René Scharfe; +Cc: git discussion list, Jonathan Nieder, Michael Haggerty
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The test fails for me on NetBSD 6.0.1 and reports:
>
> ok 1 - ref name '' is invalid
> ok 2 - ref name '/' is invalid
> ok 3 - ref name '/' is invalid with options --allow-onelevel
> ok 4 - ref name '/' is invalid with options --normalize
> error: bug in the test script: not 2 or 3 parameters to test-expect-success
>
> The alleged bug is in this line:
>
> invalid_ref NOT_MINGW '/' '--allow-onelevel --normalize'
>
> invalid_ref() constructs a test case description using its last argument,
> but the shell seems to split it up into two pieces if it contains a
> space. Minimal test case:
>
> # on NetBSD with /bin/sh
> $ a() { echo $#-$1-$2; }
> $ t="x"; a "${t:+$t}"
> 1-x-
> $ t="x y"; a "${t:+$t}"
> 2-x-y
> $ t="x y"; a "${t:+x y}"
> 1-x y-
>
> # and with bash
> $ t="x y"; a "${t:+$t}"
> 1-x y-
> $ t="x y"; a "${t:+x y}"
> 1-x y-
>
> This may be a bug in the shell, but here's a simple workaround: Construct
> the description string first and store it in a variable, and then use
> that to call test_expect_success().
Interesting. I notice that t0008 added recently to 'pu' has the
same construct.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> t/t1402-check-ref-format.sh | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 1ae4d87..1a5a5f3 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -11,7 +11,8 @@ valid_ref() {
> prereq=$1
> shift
> esac
> - test_expect_success $prereq "ref name '$1' is valid${2:+ with options $2}" "
> + desc="ref name '$1' is valid${2:+ with options $2}"
> + test_expect_success $prereq "$desc" "
> git check-ref-format $2 '$1'
> "
> }
> @@ -22,7 +23,8 @@ invalid_ref() {
> prereq=$1
> shift
> esac
> - test_expect_success $prereq "ref name '$1' is invalid${2:+ with options $2}" "
> + desc="ref name '$1' is invalid${2:+ with options $2}"
> + test_expect_success $prereq "$desc" "
> test_must_fail git check-ref-format $2 '$1'
> "
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
2013-01-08 20:39 ` Junio C Hamano
@ 2013-01-08 21:13 ` René Scharfe
2013-01-08 21:37 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: René Scharfe @ 2013-01-08 21:13 UTC (permalink / raw
To: Junio C Hamano
Cc: git discussion list, Jonathan Nieder, Michael Haggerty,
Adam Spiers
Am 08.01.2013 21:39, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>> # on NetBSD with /bin/sh
>> $ a() { echo $#-$1-$2; }
>> $ t="x"; a "${t:+$t}"
>> 1-x-
>> $ t="x y"; a "${t:+$t}"
>> 2-x-y
>> $ t="x y"; a "${t:+x y}"
>> 1-x y-
>>
>> # and with bash
>> $ t="x y"; a "${t:+$t}"
>> 1-x y-
>> $ t="x y"; a "${t:+x y}"
>> 1-x y-
>>
>> This may be a bug in the shell, but here's a simple workaround: Construct
>> the description string first and store it in a variable, and then use
>> that to call test_expect_success().
>
> Interesting. I notice that t0008 added recently to 'pu' has the
> same construct.
A quick check shows that subtests 64-68 and 89-93 of t0008 fail for me
on Debian (10 in total) and subtests 64 and 89 fail on NetBSD (2 in
total). Unlike t1402 they don't report "bug in the test script".
t0008 only uses ${:+} substitution on variables that don't contain
spaces. With the test changed to store the description in a variable
first I still get the same 2 failures.
There must be something else going on here. The different results are
interesting, especially the higher number of failures on Debian.
René
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
2013-01-08 21:13 ` René Scharfe
@ 2013-01-08 21:37 ` Junio C Hamano
2013-01-09 23:49 ` [PATCH] t0008: avoid brace expansion René Scharfe
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2013-01-08 21:37 UTC (permalink / raw
To: René Scharfe
Cc: git discussion list, Jonathan Nieder, Michael Haggerty,
Adam Spiers
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> A quick check shows that subtests 64-68 and 89-93 of t0008 fail for me
> on Debian (10 in total) and subtests 64 and 89 fail on NetBSD (2 in
> total). Unlike t1402 they don't report "bug in the test script".
>
> t0008 only uses ${:+} substitution on variables that don't contain
> spaces. With the test changed to store the description in a variable
> first I still get the same 2 failures.
>
> There must be something else going on here. The different results are
> interesting, especially the higher number of failures on Debian.
I forgot to mention that some of them seem to be broken under dash
but pass under bash.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
2013-01-08 20:23 [PATCH] t1402: work around shell quoting issue on NetBSD René Scharfe
2013-01-08 20:39 ` Junio C Hamano
@ 2013-01-09 1:27 ` Greg Troxel
2013-01-09 2:07 ` Greg Troxel
2 siblings, 0 replies; 11+ messages in thread
From: Greg Troxel @ 2013-01-09 1:27 UTC (permalink / raw
To: René Scharfe
Cc: git discussion list, Junio C Hamano, Jonathan Nieder,
Michael Haggerty
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> invalid_ref() constructs a test case description using its last argument,
> but the shell seems to split it up into two pieces if it contains a
> space. Minimal test case:
This is indeed a bug in NetBSD's shell, which I reported after finding
this test case problem, and I think someone is working on a fix. But
because git does not intend to be a shell torture test, if it's possible
to avoid bugs in a reasonable way, I think it's nice to do so.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
2013-01-08 20:23 [PATCH] t1402: work around shell quoting issue on NetBSD René Scharfe
2013-01-08 20:39 ` Junio C Hamano
2013-01-09 1:27 ` [PATCH] t1402: work around shell quoting issue on NetBSD Greg Troxel
@ 2013-01-09 2:07 ` Greg Troxel
2 siblings, 0 replies; 11+ messages in thread
From: Greg Troxel @ 2013-01-09 2:07 UTC (permalink / raw
To: René Scharfe; +Cc: git discussion list
[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The test fails for me on NetBSD 6.0.1 and reports:
>
> ok 1 - ref name '' is invalid
> ok 2 - ref name '/' is invalid
> ok 3 - ref name '/' is invalid with options --allow-onelevel
> ok 4 - ref name '/' is invalid with options --normalize
> error: bug in the test script: not 2 or 3 parameters to test-expect-success
>
> The alleged bug is in this line:
>
> invalid_ref NOT_MINGW '/' '--allow-onelevel --normalize'
The bug in NetBSD's sh has been fixed in -current:
http://gnats.netbsd.org/47361
and the change will almost certainly make it to the -6 and -5 release
branches.
With the fixed sh:
414c78c (with the workaround): t1402 passes
69637e5 (without the workaround): t1402 passes
With the buggy sh,
414c78c (with the workaround): t1402 passes
69637e5 (without workaround): t1402 fails
so I can confirm that the workaround is successful on NetBSD 5.
Thanks for addressing this, and sorry I didn't mention it on this list.
Greg
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] t0008: avoid brace expansion
2013-01-08 21:37 ` Junio C Hamano
@ 2013-01-09 23:49 ` René Scharfe
2013-01-09 23:56 ` Adam Spiers
0 siblings, 1 reply; 11+ messages in thread
From: René Scharfe @ 2013-01-09 23:49 UTC (permalink / raw
To: Junio C Hamano, git discussion list; +Cc: Adam Spiers
Brace expansion is not required by POSIX and not supported by dash nor
NetBSD's sh. Explicitly list all combinations instead.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t0008-ignores.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9b0fcd6..0273680 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -129,8 +129,9 @@ test_expect_success 'setup' '
one
ignored-*
EOF
- touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} &&
- git add -f {,a/}ignored-but-in-index
+ touch not-ignored ignored-and-untracked ignored-but-in-index &&
+ touch a/not-ignored a/ignored-and-untracked a/ignored-but-in-index &&
+ git add -f ignored-but-in-index a/ignored-but-in-index &&
cat <<-\EOF >a/.gitignore &&
two*
*three
--
1.8.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] t0008: avoid brace expansion
2013-01-09 23:49 ` [PATCH] t0008: avoid brace expansion René Scharfe
@ 2013-01-09 23:56 ` Adam Spiers
2013-01-10 0:18 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Adam Spiers @ 2013-01-09 23:56 UTC (permalink / raw
To: René Scharfe; +Cc: Junio C Hamano, git discussion list
On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Brace expansion is not required by POSIX and not supported by dash nor
> NetBSD's sh. Explicitly list all combinations instead.
Good catch, thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t0008: avoid brace expansion
2013-01-09 23:56 ` Adam Spiers
@ 2013-01-10 0:18 ` Junio C Hamano
2013-01-10 0:22 ` Adam Spiers
2013-01-10 21:24 ` René Scharfe
0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2013-01-10 0:18 UTC (permalink / raw
To: Adam Spiers; +Cc: René Scharfe, git discussion list
Adam Spiers <git@adamspiers.org> writes:
> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
> <rene.scharfe@lsrfire.ath.cx> wrote:
>> Brace expansion is not required by POSIX and not supported by dash nor
>> NetBSD's sh. Explicitly list all combinations instead.
>
> Good catch, thanks!
Yeah; thanks.
It would also be nice to avoid touch while we are at it, by the way.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t0008: avoid brace expansion
2013-01-10 0:18 ` Junio C Hamano
@ 2013-01-10 0:22 ` Adam Spiers
2013-01-10 21:24 ` René Scharfe
1 sibling, 0 replies; 11+ messages in thread
From: Adam Spiers @ 2013-01-10 0:22 UTC (permalink / raw
To: Junio C Hamano; +Cc: René Scharfe, git discussion list
On Thu, Jan 10, 2013 at 12:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
>> <rene.scharfe@lsrfire.ath.cx> wrote:
>>> Brace expansion is not required by POSIX and not supported by dash nor
>>> NetBSD's sh. Explicitly list all combinations instead.
>>
>> Good catch, thanks!
>
> Yeah; thanks.
>
> It would also be nice to avoid touch while we are at it, by the way.
Noted.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] t0008: avoid brace expansion
2013-01-10 0:18 ` Junio C Hamano
2013-01-10 0:22 ` Adam Spiers
@ 2013-01-10 21:24 ` René Scharfe
1 sibling, 0 replies; 11+ messages in thread
From: René Scharfe @ 2013-01-10 21:24 UTC (permalink / raw
To: Junio C Hamano; +Cc: Adam Spiers, git discussion list
Am 10.01.2013 01:18, schrieb Junio C Hamano:
> Adam Spiers <git@adamspiers.org> writes:
>
>> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
>> <rene.scharfe@lsrfire.ath.cx> wrote:
>>> Brace expansion is not required by POSIX and not supported by dash nor
>>> NetBSD's sh. Explicitly list all combinations instead.
>>
>> Good catch, thanks!
>
> Yeah; thanks.
>
> It would also be nice to avoid touch while we are at it, by the way.
Good idea! Replacement patch:
--- >8 ---
Brace expansion is a shell feature that's not required by POSIX and not
supported by dash nor NetBSD's sh. Explicitly list all combinations
instead. Also avoid calling touch by creating the test files with a
redirection instead, as suggested by Junio.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t0008-ignores.sh | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9b0fcd6..d7df719 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -129,8 +129,13 @@ test_expect_success 'setup' '
one
ignored-*
EOF
- touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} &&
- git add -f {,a/}ignored-but-in-index
+ for dir in . a
+ do
+ : >$dir/not-ignored &&
+ : >$dir/ignored-and-untracked &&
+ : >$dir/ignored-but-in-index
+ done &&
+ git add -f ignored-but-in-index a/ignored-but-in-index &&
cat <<-\EOF >a/.gitignore &&
two*
*three
--
1.8.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-01-10 21:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-08 20:23 [PATCH] t1402: work around shell quoting issue on NetBSD René Scharfe
2013-01-08 20:39 ` Junio C Hamano
2013-01-08 21:13 ` René Scharfe
2013-01-08 21:37 ` Junio C Hamano
2013-01-09 23:49 ` [PATCH] t0008: avoid brace expansion René Scharfe
2013-01-09 23:56 ` Adam Spiers
2013-01-10 0:18 ` Junio C Hamano
2013-01-10 0:22 ` Adam Spiers
2013-01-10 21:24 ` René Scharfe
2013-01-09 1:27 ` [PATCH] t1402: work around shell quoting issue on NetBSD Greg Troxel
2013-01-09 2:07 ` Greg Troxel
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).