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

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

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