[PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread René Scharfe
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.scha...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread Junio C Hamano
René Scharfe rene.scha...@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.scha...@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'
   
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread René Scharfe

Am 08.01.2013 21:39, schrieb Junio C Hamano:

René Scharfe rene.scha...@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é

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread Junio C Hamano
René Scharfe rene.scha...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread Greg Troxel

René Scharfe rene.scha...@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.


pgpeZdzuWnVQF.pgp
Description: PGP signature


Re: [PATCH] t1402: work around shell quoting issue on NetBSD

2013-01-08 Thread Greg Troxel

René Scharfe rene.scha...@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




pgpTeLAlCVU0y.pgp
Description: PGP signature