| Message ID | a4272c4a-7073-4671-a883-50e9413b0384@ramsayjones.plus.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/12] meson.build: remove -DCURL_DISABLE_TYPECHECK | expand |
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
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
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
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
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
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(-)