patch, 12 partsconfig.mak.uname: only set NO_REGEX on cygwin for v1.7

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(-)

On 15/03/2025 02:47, Ramsay Jones wrote:

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

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

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.

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.

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

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

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.

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.

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.

On 17/03/2025 15:56, Junio C Hamano wrote:

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.

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