[PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
the NO_REGEX build variable because the platform regex library failed
some of the tests (t4018 and t4034), which passed just fine with the
compat library.

After some time (may a year or two), the platform library had been
updated (with an import from FreeBSD, I believe) and now passed the full
test-suite. This would be about the time of the v1.7 -> v2.0 transition
in 2015. I had a patch ready to send, but just didn't get around to
submitting it to the list. At some point in the interim, the official
cygwin git package used the autoconf build system, which sets the
NO_REGEX variable to use the platform regex library functions. The new
meson build system does likewise.

In order to produce the same NO_REGEX configuration from autoconf, meson
and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index ae6ba15586..b6adce0bc4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -245,7 +245,9 @@ ifeq ($(uname_O),Cygwin)
 		# Try commenting this out if you suspect MMAP is more efficient
 		NO_MMAP = YesPlease
         else
-		NO_REGEX = UnfortunatelyYes
+                ifeq ($(shell expr "$(uname_R)" : '1\.7\.'),4)
+		        NO_REGEX = UnfortunatelyYes
+                endif
         endif
 	HAVE_DEV_TTY = YesPlease
 	HAVE_ALLOCA_H = YesPlease
-- 
2.49.0

Re: [PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

On 15/03/2025 02:47, Ramsay Jones wrote:
> 
> Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
> the NO_REGEX build variable because the platform regex library failed
> some of the tests (t4018 and t4034), which passed just fine with the
> compat library.
> 
> After some time (may a year or two), the platform library had been
> updated (with an import from FreeBSD, I believe) and now passed the full
> test-suite. This would be about the time of the v1.7 -> v2.0 transition
> in 2015. I had a patch ready to send, but just didn't get around to
> submitting it to the list.

I forgot to mention, that one of the reasons that I didn't get around
to submitting this patch then, was because of a '# TODO known breakage
vanished' in test t7815-grep-binary.sh:

  $ ./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
  $ 

The platform regex library is happy to match a NUL byte with the '.'
pattern. (presumably this is also true on FreeBSD?).

I could not decide on the best way to 'fix' this issue. The options
seemed to be: do nothing (it's not hurting anyone), disable the test
on cygwin or simply remove the test.

[I think I prefer to simply delete the test, since it doesn't seem to
be testing anything useful, as far as I can see.]

What do you think?

ATB,
Ramsay Jones

Re: [PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

>> After some time (may a year or two), the platform library had been
>> updated (with an import from FreeBSD, I believe) and now passed the full
>> test-suite. This would be about the time of the v1.7 -> v2.0 transition
>> in 2015. I had a patch ready to send, but just didn't get around to
>> submitting it to the list.

So is it safe for us to just drop the bit that sets NO_REGEX and
require Cygwin that is less than 10 years old?  As long as people
are willing to actively maintain the compatibility wart for older
systems there is no strong reason to do so, but at some point it
would become diminishing returns even for those who have hardware to
develop, build, and test on, when the reason they keep such an old
system becomes only to maintain it instead of actively using it,
and I am wondering if Cygwin 1.7 has past that point.

> I forgot to mention, that one of the reasons that I didn't get around
> to submitting this patch then, was because of a '# TODO known breakage
> vanished' in test t7815-grep-binary.sh:
> ...
> The platform regex library is happy to match a NUL byte with the '.'
> pattern. (presumably this is also true on FreeBSD?).

In this test, the haystack has "binary\0file\0m..." and the needle
to be sought is ".fi".  The system I have at hand uses glibc 2.40
and it refuses to match NUL with '.', it seems.

> I could not decide on the best way to 'fix' this issue. The options
> seemed to be: do nothing (it's not hurting anyone), disable the test
> on cygwin or simply remove the test.

The part "On Cygwin" somewhat puzzled me; aren't folks on various
BSD variants seeing the same symptom?

Do we want to eventually turn it to test_expect_success?  I think
the "fix" depends on this single question, and I am not sure if we
do.

Is the behaviour that '.' matches NUL on some platform and doesn't
on some others hurting anybody?  I dislike a tool that behaves
differently depending on the platform, but not strongly enough in
this case somehow.

On the same system, GNU grep and sed seem to consider that '.'
matches NUL there, i.e.

    $ grep -a '.fi' a | cat -v -T
    binary^@file^@m[*]c^@*M-CM-&^@M-CM-0
    $ sed -n -e '/.fi/p' a | cat -v -T


> [I think I prefer to simply delete the test, since it doesn't seem to
> be testing anything useful, as far as I can see.]
>
> What do you think?
>
> ATB,
> Ramsay Jones

Re: [PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

>> After some time (may a year or two), the platform library had been
>> updated (with an import from FreeBSD, I believe) and now passed the full
>> test-suite. This would be about the time of the v1.7 -> v2.0 transition
>> in 2015. I had a patch ready to send, but just didn't get around to
>> submitting it to the list.

So is it safe for us to just drop the bit that sets NO_REGEX and
require Cygwin that is less than 10 years old?  As long as people
are willing to actively maintain the compatibility wart for older
systems there is no strong reason to do so, but at some point it
would become diminishing returns even for those who have hardware to
develop, build, and test on, when the reason they keep such an old
system becomes only to maintain it instead of actively using it,
and I am wondering if Cygwin 1.7 has past that point.

> I forgot to mention, that one of the reasons that I didn't get around
> to submitting this patch then, was because of a '# TODO known breakage
> vanished' in test t7815-grep-binary.sh:
> ...
> The platform regex library is happy to match a NUL byte with the '.'
> pattern. (presumably this is also true on FreeBSD?).

In this test, the haystack has "binary\0file\0m..." and the needle
to be sought is ".fi".  The system I have at hand uses glibc 2.40
and it refuses to match NUL with '.', it seems.

> I could not decide on the best way to 'fix' this issue. The options
> seemed to be: do nothing (it's not hurting anyone), disable the test
> on cygwin or simply remove the test.

The part "On Cygwin" somewhat puzzled me; aren't folks on various
BSD variants seeing the same symptom?

Do we want to eventually turn it to test_expect_success?  I think
the "fix" depends on this single question, and I am not sure if we
do.

Is the behaviour that '.' matches NUL on some platform and doesn't
on some others hurting anybody?  I dislike a tool that behaves
differently depending on the platform, but not strongly enough in
this case somehow.

On the same system, GNU grep and sed seem to consider that '.'
matches NUL there, i.e.

    $ grep -a '.fi' a | cat -v -T
    binary^@file^@m[*]c^@*M-CM-&^@M-CM-0
    $ sed -n -e '/.fi/p' a | cat -v -T
    binary^@file^@m[*]c^@*M-CM-&^@M-CM-0

They ought to be using the same library as Git compiled on this
system does, so it is somewhat curious.

> [I think I prefer to simply delete the test, since it doesn't seem to
> be testing anything useful, as far as I can see.]
>
> What do you think?
>
> ATB,
> Ramsay Jones

Re: [PATCH 06/12] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

On 17/03/2025 15:56, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>>> After some time (may a year or two), the platform library had been
>>> updated (with an import from FreeBSD, I believe) and now passed the full
>>> test-suite. This would be about the time of the v1.7 -> v2.0 transition
>>> in 2015. I had a patch ready to send, but just didn't get around to
>>> submitting it to the list.
> 
> So is it safe for us to just drop the bit that sets NO_REGEX and
> require Cygwin that is less than 10 years old?  As long as people
> are willing to actively maintain the compatibility wart for older
> systems there is no strong reason to do so, but at some point it
> would become diminishing returns even for those who have hardware to
> develop, build, and test on, when the reason they keep such an old
> system becomes only to maintain it instead of actively using it,
> and I am wondering if Cygwin 1.7 has past that point.

Heh, I have a patch (without a commit message) which I had intended
to add at the very end, so that it is easy to drop, that looks like:

  diff --git a/config.mak.uname b/config.mak.uname
  index 1a897bd022..1dc69fc65b 100644
  --- a/config.mak.uname
  +++ b/config.mak.uname
  @@ -235,22 +235,6 @@ ifeq ($(uname_S),SunOS)
   	BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
   endif
   ifeq ($(uname_O),Cygwin)
  -        ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
  -		NO_D_TYPE_IN_DIRENT = YesPlease
  -		NO_STRCASESTR = YesPlease
  -		NO_MEMMEM = YesPlease
  -		NO_SYMLINK_HEAD = YesPlease
  -		NO_IPV6 = YesPlease
  -		OLD_ICONV = UnfortunatelyYes
  -		# There are conflicting reports about this.
  -		# On some boxes NO_MMAP is needed, and not so elsewhere.
  -		# Try commenting this out if you suspect MMAP is more efficient
  -		NO_MMAP = YesPlease
  -        else
  -                ifeq ($(shell expr "$(uname_R)" : '1\.7\.'),4)
  -		        NO_REGEX = UnfortunatelyYes
  -                endif
  -        endif
   	HAVE_DEV_TTY = YesPlease
   	HAVE_GETDELIM = YesPlease
   	HAVE_CLOCK_GETTIME=YesPlease

This would effectively drop 'support' for cygwin versions less than
the v2.0 (which was tagged on 27-apr-2015). Note that I don't know
if it is possible to build git from source on those versions, even
with that conditional intact, but I guess it's more likely!

Given that it is unlikely to be me fielding the complaints from the
cygwin git community, I wanted Adam (cygwin git maintainer) to
provide his input into the decision to incorporate such a patch. :)

[I doubt there are many people building git from source on a very old
version of cygwin, but I just don't know! ;) ]

I have been building (and running) git from source since about 2006,
on both Linux and cygwin, but I suspect that the vast majority of
cygwin users just install Adam's package.

>> I forgot to mention, that one of the reasons that I didn't get around
>> to submitting this patch then, was because of a '# TODO known breakage
>> vanished' in test t7815-grep-binary.sh:
>> ...
>> The platform regex library is happy to match a NUL byte with the '.'
>> pattern. (presumably this is also true on FreeBSD?).
> 
> In this test, the haystack has "binary\0file\0m..." and the needle
> to be sought is ".fi".  The system I have at hand uses glibc 2.40
> and it refuses to match NUL with '.', it seems.
> 
>> I could not decide on the best way to 'fix' this issue. The options
>> seemed to be: do nothing (it's not hurting anyone), disable the test
>> on cygwin or simply remove the test.
> 
> The part "On Cygwin" somewhat puzzled me; aren't folks on various
> BSD variants seeing the same symptom?

Again 'I don't know what I don't know'. :) I would have thought that
(at least) FreeBSD users should also be seeing this 'known breakage
vanished' issue, but we have not heard about it, so ... we *know*
that cygwin has this issue, hence the suggestion to disable the
test there.

[Also, it is possible that either cygwin or FreeBSD changed the
implementation after the import.]

Thanks.

ATB,
Ramsay Jones