On 23/05/2025 17:40, Ramsay Jones wrote:
>
>
> On 23/05/2025 16:58, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@xxxxxx> writes:
>>
>>> ... The problem is that we have a test that unexpectedly
>>> passes on macOS:
>>>
>>> ▶ 868/1023 - git grep .fi a UNEXPECTEDPASS
>>>
>>> The test in question is this one:
>>>
>>> test_expect_failure !CYGWIN 'git grep .fi a' '
>>> git grep .fi a
>>> '
>>>
>>> The test passes if '.' matches a NUL byte, which we expect to only
>>> happen on Cygwin. 064eed36c7f (config.mak.uname: only set NO_REGEX on
>>> cygwin for v1.7, 2025-04-17) mentions that this behaviour was probably
>>> imported from FreeBSD, which makes me wonder whether macOS eventually
>>> also inherited the same code given its BSD lineage.
>>
>> Yup, I was wondering about the same thing. Thanks for confirming.
>> It is unfortunate that we have blanket USES_BSD_REGEXP prerequisite
>> ;-)
>>
>>> I think we probably want something like the below patch to fix this. We
>>> could also have a prereq, but that prereq would look almost the exact
>>> same as the test. It does make me question the value of the test itself
>>> as the behaviour is completely platform specific.
>
> Yep, I very nearly sent that patch with a hunk which deleted that
> test. In the end, I decided to play it more conservatively. :)
>
>> Curious.
>>
>> Don't we run the same set of tests on macOS without Meson? the
>> exact same test must be passing unexpectedly. Why do we see the
>> complaint on only osx-meson job without osx-{clang,reftable,gcc}
>> jobs?
>
> Yes, for about a decade, git built on cygwin with the system regex library
> would have shown an 'unexpected pass' on this test. However, unless you
> look for it, you wouldn't know; make and prove are quite happy with the
> situation, since it is an unexpected _pass_:
>
> $ git diff
> diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
> index b7d83f9a5d..3bd91da970 100755
> --- a/t/t7815-grep-binary.sh
> +++ b/t/t7815-grep-binary.sh
> @@ -63,7 +63,7 @@ test_expect_success 'git grep ile a' '
> git grep ile a
> '
>
> -test_expect_failure !CYGWIN 'git grep .fi a' '
> +test_expect_failure 'git grep .fi a' '
> git grep .fi a
> '
>
> $ cd t
> $ ./t7815-grep-binary.sh
> ok 1 - setup
> ok 2 - git grep ina a
> ok 3 - git grep -ah ina a
> ok 4 - git grep -I ina a
> ok 5 - git grep -c ina a
> ok 6 - git grep -l ina a
> ok 7 - git grep -L bar a
> ok 8 - git grep -q ina a
> ok 9 - git grep -F ile a
> ok 10 - git grep -Fi iLE a
> ok 11 - git grep ile a
> not ok 12 - git grep .fi a # TODO known breakage
> ok 13 - grep respects binary diff attribute
> ok 14 - grep --cached respects binary diff attribute
> ok 15 - grep --cached respects binary diff attribute (2)
> ok 16 - grep revision respects binary diff attribute
> ok 17 - grep respects not-binary diff attribute
> ok 18 - setup textconv filters
> ok 19 - grep does not honor textconv
> ok 20 - grep --textconv honors textconv
> ok 21 - grep --no-textconv does not honor textconv
> ok 22 - grep --textconv blob honors textconv
> # still have 1 known breakage(s)
> # passed all remaining 21 test(s)
> 1..22
> $ echo $?
> 0
> $
>
> Again, prove doesn't care:
>
> $ prove t7815-grep-binary.sh
> t7815-grep-binary.sh .. ok
> All tests successful.
> Files=1, Tests=22, 0 wallclock secs ( 0.03 usr 0.00 sys + 0.09 cusr 0.13 csys = 0.25 CPU)
> Result: PASS
> $ echo $?
> 0
> $
Sigh! Of course, when I did the above I was on Linux! It looks like the
following on cygwin:
$ ./t7815-grep-binary.sh
ok 1 - setup
ok 2 - git grep ina a
ok 3 - git grep -ah ina a
ok 4 - git grep -I ina a
ok 5 - git grep -c ina a
ok 6 - git grep -l ina a
ok 7 - git grep -L bar a
ok 8 - git grep -q ina a
ok 9 - git grep -F ile a
ok 10 - git grep -Fi iLE a
ok 11 - git grep ile a
ok 12 - git grep .fi a # TODO known breakage vanished
ok 13 - grep respects binary diff attribute
ok 14 - grep --cached respects binary diff attribute
ok 15 - grep --cached respects binary diff attribute (2)
ok 16 - grep revision respects binary diff attribute
ok 17 - grep respects not-binary diff attribute
ok 18 - setup textconv filters
ok 19 - grep does not honor textconv
ok 20 - grep --textconv honors textconv
ok 21 - grep --no-textconv does not honor textconv
ok 22 - grep --textconv blob honors textconv
# 1 known breakage(s) vanished; please update test(s)
# passed all remaining 21 test(s)
1..22
$ echo $?
0
$
Note that #12 is 'ok' and '... please update test(s)'
Sorry about that!
> When I tested the patch (without !CYGWIN) with 'meson test' it didn't care
> either - that was *before* Patrick's recent patch to make meson parse TAP
> output. ;)
>
> Question: should meson (or indeed prove) fail the test because of an
> unexpected _pass_?
ATB,
Ramsay Jones
