* v2.25.0-rc0 test failure on cygwin
@ 2025-06-02 22:33 Ramsay Jones
2025-06-02 22:48 ` Ramsay Jones
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ramsay Jones @ 2025-06-02 22:33 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Adam Dinwoodie, Junio C Hamano, jayatheerthkulkarni2005
I noticed on Friday, while testing the v2.25.0-rc0 build, that test t6137
failed on cygwin:
$ tail test-out-2-50-rc0
Test Summary Report
-------------------
t6137-pathspec-wildcards-literal.sh (Wstat: 256 (exited 1) Tests: 25 Failed: 11)
Failed tests: 2, 7, 9, 11, 14-15, 17, 19, 21, 23, 25
Non-zero exit status: 1
Files=1023, Tests=30946, 21783 wallclock secs (19.08 usr 42.17 sys + 4031.65 cusr 12965.78 csys = 17058.68 CPU)
Result: FAIL
make[1]: *** [Makefile:78: prove] Error 1
make[1]: Leaving directory '/home/ramsay/git/t'
make: *** [Makefile:3286: test] Error 2
$
A quick squint at the failing tests made it clear that the failure was
caused by the cygwin build treating a quoted glob character sequence
(e.g. '\*') as a directory separator char followed by a glob character.
To show this, I quickly hacked up a patch which causes the test to pass:
diff --git a/abspath.h b/abspath.h
index 4653080d5e..a5e30a3033 100644
--- a/abspath.h
+++ b/abspath.h
@@ -27,7 +27,7 @@ char *prefix_filename_except_for_dash(const char *prefix, const char *path);
static inline int is_absolute_path(const char *path)
{
- return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
+ return /*is_dir_sep(path[0])*/ path[0] == '/' || has_dos_drive_prefix(path);
}
/**
diff --git a/path.c b/path.c
index 3b598b2847..f000b9ffff 100644
--- a/path.c
+++ b/path.c
@@ -1223,13 +1223,15 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
end = src + offset_1st_component(src);
while (src < end) {
char c = *src++;
+#ifdef DUMMY
if (is_dir_sep(c))
c = '/';
+#endif
*dst++ = c;
}
dst0 = dst;
- while (is_dir_sep(*src))
+ while (/*is_dir_sep(*src)*/ *src == '/')
src++;
for (;;) {
@@ -1247,10 +1249,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
if (!src[1]) {
/* (1) */
src++;
- } else if (is_dir_sep(src[1])) {
+ } else if (/*is_dir_sep(src[1])*/ src[1] == '/') {
/* (2) */
src += 2;
- while (is_dir_sep(*src))
+ while (/*is_dir_sep(*src)*/ *src == '/')
src++;
continue;
} else if (src[1] == '.') {
@@ -1258,10 +1260,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
/* (3) */
src += 2;
goto up_one;
- } else if (is_dir_sep(src[2])) {
+ } else if (/*is_dir_sep(src[2])*/ src[2] == '/') {
/* (4) */
src += 3;
- while (is_dir_sep(*src))
+ while (/*is_dir_sep(*src)*/ *src == '/')
src++;
goto up_one;
}
@@ -1269,11 +1271,11 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
}
/* copy up to the next '/', and eat all '/' */
- while ((c = *src++) != '\0' && !is_dir_sep(c))
+ while ((c = *src++) != '\0' && !(/*is_dir_sep(c)*/ c == '/'))
*dst++ = c;
- if (is_dir_sep(c)) {
+ if (/*is_dir_sep(c)*/ c == '/') {
*dst++ = '/';
- while (is_dir_sep(c))
+ while (/*is_dir_sep(c)*/ c == '/')
c = *src++;
src--;
} else if (!c)
In other words, in 'is_absolute_path()' and 'normalize_path_copy_len()', then
just disable the '\' character as a path separator! ;)
This was just to demonstrate the problem, not a serious patch, of course!
I was away for the weekend and was expecting to see a patch to fix this
on Gfw when I got back, but to my surprise there has been no mention of
it on the mailing list (having now waded through the backlog!).
To be clear, I can not imagine that this test passes on Gfw. However, this
should have been failing the windows CI for ages, so ... perhaps I don't
have a sufficiently vivid imagination. :)
Anyway, the patch below 'fixes' the issue on cygwin.
Thanks.
ATB,
Ramsay Jones
---- >8 ----
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Mon, 2 Jun 2025 22:51:33 +0100
Subject: [PATCH] t6137: disable 'quoted glob' pathspecs on cygwin
The backslash in a 'quoted glob' character is treated as a directory
separator character on cygwin, which causes all such tests to fail.
Skip all such tests on cygwin using the !CYGWIN prerequisite. While
here, fix a few test titles as well.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
t/t6137-pathspec-wildcards-literal.sh | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
index 20abad5667..229e48282e 100755
--- a/t/t6137-pathspec-wildcards-literal.sh
+++ b/t/t6137-pathspec-wildcards-literal.sh
@@ -39,7 +39,7 @@ test_expect_success 'add wildcard *' '
)
'
-test_expect_success 'add literal \*' '
+test_expect_success !CYGWIN 'add literal \*' '
git init test-asterisk-literal &&
(
cd test-asterisk-literal &&
@@ -125,7 +125,7 @@ test_expect_success 'add wildcard f*' '
)
'
-test_expect_success 'add literal f\*' '
+test_expect_success !CYGWIN 'add literal f\*' '
git init test-f-lit &&
(
cd test-f-lit &&
@@ -156,7 +156,7 @@ test_expect_success 'add wildcard f**' '
)
'
-test_expect_success 'add literal f\*\*' '
+test_expect_success !CYGWIN 'add literal f\*\*' '
git init test-fdstar-lit &&
(
cd test-fdstar-lit &&
@@ -184,7 +184,7 @@ test_expect_success 'add wildcard f?z' '
)
'
-test_expect_success 'add literal \? literal' '
+test_expect_success !CYGWIN 'add literal \? literal' '
git init test-q-lit &&
(
cd test-q-lit &&
@@ -227,7 +227,7 @@ test_expect_success 'add wildcard hello?world' '
)
'
-test_expect_success 'add literal hello\?world' '
+test_expect_success !CYGWIN 'add literal hello\?world' '
git init test-hellolit &&
(
cd test-hellolit &&
@@ -241,7 +241,7 @@ test_expect_success 'add literal hello\?world' '
)
'
-test_expect_success 'add literal [abc]' '
+test_expect_success !CYGWIN 'add literal \[abc\]' '
git init test-brackets-lit &&
(
cd test-brackets-lit &&
@@ -280,7 +280,7 @@ test_expect_success 'commit: wildcard *' '
)
'
-test_expect_success 'commit: literal *' '
+test_expect_success !CYGWIN 'commit: literal \*' '
git init test-c-asterisk-lit &&
(
cd test-c-asterisk-lit &&
@@ -313,7 +313,7 @@ test_expect_success 'commit: wildcard f*' '
)
'
-test_expect_success 'commit: literal f\*' '
+test_expect_success !CYGWIN 'commit: literal f\*' '
git init test-c-flit &&
(
cd test-c-flit &&
@@ -328,7 +328,7 @@ test_expect_success 'commit: literal f\*' '
)
'
-test_expect_success 'commit: wildcard pathspec limits commit' '
+test_expect_success 'commit: wildcard f**' '
git init test-c-pathlimit &&
(
cd test-c-pathlimit &&
@@ -346,7 +346,7 @@ test_expect_success 'commit: wildcard pathspec limits commit' '
)
'
-test_expect_success 'commit: literal f\*\*' '
+test_expect_success !CYGWIN 'commit: literal f\*\*' '
git init test-c-fdstar-lit &&
(
cd test-c-fdstar-lit &&
@@ -379,7 +379,7 @@ test_expect_success 'commit: wildcard ?' '
)
'
-test_expect_success 'commit: literal \?' '
+test_expect_success !CYGWIN 'commit: literal \?' '
git init test-c-qlit &&
(
cd test-c-qlit &&
@@ -411,7 +411,7 @@ test_expect_success 'commit: wildcard hello?world' '
)
'
-test_expect_success 'commit: literal hello\?world' '
+test_expect_success !CYGWIN 'commit: literal hello\?world' '
git init test-c-hellolit &&
(
cd test-c-hellolit &&
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: v2.25.0-rc0 test failure on cygwin
2025-06-02 22:33 v2.25.0-rc0 test failure on cygwin Ramsay Jones
@ 2025-06-02 22:48 ` Ramsay Jones
2025-06-02 22:57 ` Kristoffer Haugsbakk
2025-06-03 14:41 ` JAYATHEERTH K
2025-06-03 16:11 ` Junio C Hamano
2 siblings, 1 reply; 6+ messages in thread
From: Ramsay Jones @ 2025-06-02 22:48 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Adam Dinwoodie, Junio C Hamano, jayatheerthkulkarni2005
Heh, so the subject and ...
On 02/06/2025 23:33, Ramsay Jones wrote:
>
> I noticed on Friday, while testing the v2.25.0-rc0 build, that test t6137
.. this should read v2.50.0-rc0. Hmm ...
ATB,
Ramsay Jones
> failed on cygwin:
>
> $ tail test-out-2-50-rc0
> Test Summary Report
> -------------------
> t6137-pathspec-wildcards-literal.sh (Wstat: 256 (exited 1) Tests: 25 Failed: 11)
> Failed tests: 2, 7, 9, 11, 14-15, 17, 19, 21, 23, 25
> Non-zero exit status: 1
> Files=1023, Tests=30946, 21783 wallclock secs (19.08 usr 42.17 sys + 4031.65 cusr 12965.78 csys = 17058.68 CPU)
> Result: FAIL
> make[1]: *** [Makefile:78: prove] Error 1
> make[1]: Leaving directory '/home/ramsay/git/t'
> make: *** [Makefile:3286: test] Error 2
> $
>
> A quick squint at the failing tests made it clear that the failure was
> caused by the cygwin build treating a quoted glob character sequence
> (e.g. '\*') as a directory separator char followed by a glob character.
>
> To show this, I quickly hacked up a patch which causes the test to pass:
>
>
> diff --git a/abspath.h b/abspath.h
> index 4653080d5e..a5e30a3033 100644
> --- a/abspath.h
> +++ b/abspath.h
> @@ -27,7 +27,7 @@ char *prefix_filename_except_for_dash(const char *prefix, const char *path);
>
> static inline int is_absolute_path(const char *path)
> {
> - return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
> + return /*is_dir_sep(path[0])*/ path[0] == '/' || has_dos_drive_prefix(path);
> }
>
> /**
> diff --git a/path.c b/path.c
> index 3b598b2847..f000b9ffff 100644
> --- a/path.c
> +++ b/path.c
> @@ -1223,13 +1223,15 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> end = src + offset_1st_component(src);
> while (src < end) {
> char c = *src++;
> +#ifdef DUMMY
> if (is_dir_sep(c))
> c = '/';
> +#endif
> *dst++ = c;
> }
> dst0 = dst;
>
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
>
> for (;;) {
> @@ -1247,10 +1249,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> if (!src[1]) {
> /* (1) */
> src++;
> - } else if (is_dir_sep(src[1])) {
> + } else if (/*is_dir_sep(src[1])*/ src[1] == '/') {
> /* (2) */
> src += 2;
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
> continue;
> } else if (src[1] == '.') {
> @@ -1258,10 +1260,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> /* (3) */
> src += 2;
> goto up_one;
> - } else if (is_dir_sep(src[2])) {
> + } else if (/*is_dir_sep(src[2])*/ src[2] == '/') {
> /* (4) */
> src += 3;
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
> goto up_one;
> }
> @@ -1269,11 +1271,11 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> }
>
> /* copy up to the next '/', and eat all '/' */
> - while ((c = *src++) != '\0' && !is_dir_sep(c))
> + while ((c = *src++) != '\0' && !(/*is_dir_sep(c)*/ c == '/'))
> *dst++ = c;
> - if (is_dir_sep(c)) {
> + if (/*is_dir_sep(c)*/ c == '/') {
> *dst++ = '/';
> - while (is_dir_sep(c))
> + while (/*is_dir_sep(c)*/ c == '/')
> c = *src++;
> src--;
> } else if (!c)
>
> In other words, in 'is_absolute_path()' and 'normalize_path_copy_len()', then
> just disable the '\' character as a path separator! ;)
>
> This was just to demonstrate the problem, not a serious patch, of course!
>
> I was away for the weekend and was expecting to see a patch to fix this
> on Gfw when I got back, but to my surprise there has been no mention of
> it on the mailing list (having now waded through the backlog!).
>
> To be clear, I can not imagine that this test passes on Gfw. However, this
> should have been failing the windows CI for ages, so ... perhaps I don't
> have a sufficiently vivid imagination. :)
>
> Anyway, the patch below 'fixes' the issue on cygwin.
>
> Thanks.
>
> ATB,
> Ramsay Jones
>
>
> ---- >8 ----
> From: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Date: Mon, 2 Jun 2025 22:51:33 +0100
> Subject: [PATCH] t6137: disable 'quoted glob' pathspecs on cygwin
>
> The backslash in a 'quoted glob' character is treated as a directory
> separator character on cygwin, which causes all such tests to fail.
> Skip all such tests on cygwin using the !CYGWIN prerequisite. While
> here, fix a few test titles as well.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> t/t6137-pathspec-wildcards-literal.sh | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
> index 20abad5667..229e48282e 100755
> --- a/t/t6137-pathspec-wildcards-literal.sh
> +++ b/t/t6137-pathspec-wildcards-literal.sh
> @@ -39,7 +39,7 @@ test_expect_success 'add wildcard *' '
> )
> '
>
> -test_expect_success 'add literal \*' '
> +test_expect_success !CYGWIN 'add literal \*' '
> git init test-asterisk-literal &&
> (
> cd test-asterisk-literal &&
> @@ -125,7 +125,7 @@ test_expect_success 'add wildcard f*' '
> )
> '
>
> -test_expect_success 'add literal f\*' '
> +test_expect_success !CYGWIN 'add literal f\*' '
> git init test-f-lit &&
> (
> cd test-f-lit &&
> @@ -156,7 +156,7 @@ test_expect_success 'add wildcard f**' '
> )
> '
>
> -test_expect_success 'add literal f\*\*' '
> +test_expect_success !CYGWIN 'add literal f\*\*' '
> git init test-fdstar-lit &&
> (
> cd test-fdstar-lit &&
> @@ -184,7 +184,7 @@ test_expect_success 'add wildcard f?z' '
> )
> '
>
> -test_expect_success 'add literal \? literal' '
> +test_expect_success !CYGWIN 'add literal \? literal' '
> git init test-q-lit &&
> (
> cd test-q-lit &&
> @@ -227,7 +227,7 @@ test_expect_success 'add wildcard hello?world' '
> )
> '
>
> -test_expect_success 'add literal hello\?world' '
> +test_expect_success !CYGWIN 'add literal hello\?world' '
> git init test-hellolit &&
> (
> cd test-hellolit &&
> @@ -241,7 +241,7 @@ test_expect_success 'add literal hello\?world' '
> )
> '
>
> -test_expect_success 'add literal [abc]' '
> +test_expect_success !CYGWIN 'add literal \[abc\]' '
> git init test-brackets-lit &&
> (
> cd test-brackets-lit &&
> @@ -280,7 +280,7 @@ test_expect_success 'commit: wildcard *' '
> )
> '
>
> -test_expect_success 'commit: literal *' '
> +test_expect_success !CYGWIN 'commit: literal \*' '
> git init test-c-asterisk-lit &&
> (
> cd test-c-asterisk-lit &&
> @@ -313,7 +313,7 @@ test_expect_success 'commit: wildcard f*' '
> )
> '
>
> -test_expect_success 'commit: literal f\*' '
> +test_expect_success !CYGWIN 'commit: literal f\*' '
> git init test-c-flit &&
> (
> cd test-c-flit &&
> @@ -328,7 +328,7 @@ test_expect_success 'commit: literal f\*' '
> )
> '
>
> -test_expect_success 'commit: wildcard pathspec limits commit' '
> +test_expect_success 'commit: wildcard f**' '
> git init test-c-pathlimit &&
> (
> cd test-c-pathlimit &&
> @@ -346,7 +346,7 @@ test_expect_success 'commit: wildcard pathspec limits commit' '
> )
> '
>
> -test_expect_success 'commit: literal f\*\*' '
> +test_expect_success !CYGWIN 'commit: literal f\*\*' '
> git init test-c-fdstar-lit &&
> (
> cd test-c-fdstar-lit &&
> @@ -379,7 +379,7 @@ test_expect_success 'commit: wildcard ?' '
> )
> '
>
> -test_expect_success 'commit: literal \?' '
> +test_expect_success !CYGWIN 'commit: literal \?' '
> git init test-c-qlit &&
> (
> cd test-c-qlit &&
> @@ -411,7 +411,7 @@ test_expect_success 'commit: wildcard hello?world' '
> )
> '
>
> -test_expect_success 'commit: literal hello\?world' '
> +test_expect_success !CYGWIN 'commit: literal hello\?world' '
> git init test-c-hellolit &&
> (
> cd test-c-hellolit &&
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: v2.25.0-rc0 test failure on cygwin
2025-06-02 22:48 ` Ramsay Jones
@ 2025-06-02 22:57 ` Kristoffer Haugsbakk
0 siblings, 0 replies; 6+ messages in thread
From: Kristoffer Haugsbakk @ 2025-06-02 22:57 UTC (permalink / raw)
To: Ramsay Jones, GIT Mailing-list
Cc: Adam Dinwoodie, Junio C Hamano, jayatheerthkulkarni2005
On Tue, Jun 3, 2025, at 00:48, Ramsay Jones wrote:
> Heh, so the subject and ...
>
> On 02/06/2025 23:33, Ramsay Jones wrote:
>>
>> I noticed on Friday, while testing the v2.25.0-rc0 build, that test t6137
>
> .. this should read v2.50.0-rc0. Hmm ...
Where does the extra *2* come from‽ ;)
https://lore.kernel.org/git/061401dbd2d8$b4ba03a0$1e2e0ae0$@nexbridge.com/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: v2.25.0-rc0 test failure on cygwin
2025-06-02 22:33 v2.25.0-rc0 test failure on cygwin Ramsay Jones
2025-06-02 22:48 ` Ramsay Jones
@ 2025-06-03 14:41 ` JAYATHEERTH K
2025-06-03 16:11 ` Junio C Hamano
2 siblings, 0 replies; 6+ messages in thread
From: JAYATHEERTH K @ 2025-06-03 14:41 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Adam Dinwoodie, Junio C Hamano
Hi Ramsay,
On Tue, Jun 3, 2025 at 4:03 AM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
>
> I noticed on Friday, while testing the v2.25.0-rc0 build, that test t6137
> failed on cygwin:
>
> $ tail test-out-2-50-rc0
> Test Summary Report
> -------------------
> t6137-pathspec-wildcards-literal.sh (Wstat: 256 (exited 1) Tests: 25 Failed: 11)
> Failed tests: 2, 7, 9, 11, 14-15, 17, 19, 21, 23, 25
> Non-zero exit status: 1
> Files=1023, Tests=30946, 21783 wallclock secs (19.08 usr 42.17 sys + 4031.65 cusr 12965.78 csys = 17058.68 CPU)
> Result: FAIL
> make[1]: *** [Makefile:78: prove] Error 1
> make[1]: Leaving directory '/home/ramsay/git/t'
> make: *** [Makefile:3286: test] Error 2
> $
>
> A quick squint at the failing tests made it clear that the failure was
> caused by the cygwin build treating a quoted glob character sequence
> (e.g. '\*') as a directory separator char followed by a glob character.
>
> To show this, I quickly hacked up a patch which causes the test to pass:
>
>
> diff --git a/abspath.h b/abspath.h
> index 4653080d5e..a5e30a3033 100644
> --- a/abspath.h
> +++ b/abspath.h
> @@ -27,7 +27,7 @@ char *prefix_filename_except_for_dash(const char *prefix, const char *path);
>
> static inline int is_absolute_path(const char *path)
> {
> - return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
> + return /*is_dir_sep(path[0])*/ path[0] == '/' || has_dos_drive_prefix(path);
> }
>
> /**
> diff --git a/path.c b/path.c
> index 3b598b2847..f000b9ffff 100644
> --- a/path.c
> +++ b/path.c
> @@ -1223,13 +1223,15 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> end = src + offset_1st_component(src);
> while (src < end) {
> char c = *src++;
> +#ifdef DUMMY
> if (is_dir_sep(c))
> c = '/';
> +#endif
> *dst++ = c;
> }
> dst0 = dst;
>
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
>
> for (;;) {
> @@ -1247,10 +1249,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> if (!src[1]) {
> /* (1) */
> src++;
> - } else if (is_dir_sep(src[1])) {
> + } else if (/*is_dir_sep(src[1])*/ src[1] == '/') {
> /* (2) */
> src += 2;
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
> continue;
> } else if (src[1] == '.') {
> @@ -1258,10 +1260,10 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> /* (3) */
> src += 2;
> goto up_one;
> - } else if (is_dir_sep(src[2])) {
> + } else if (/*is_dir_sep(src[2])*/ src[2] == '/') {
> /* (4) */
> src += 3;
> - while (is_dir_sep(*src))
> + while (/*is_dir_sep(*src)*/ *src == '/')
> src++;
> goto up_one;
> }
> @@ -1269,11 +1271,11 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
> }
>
> /* copy up to the next '/', and eat all '/' */
> - while ((c = *src++) != '\0' && !is_dir_sep(c))
> + while ((c = *src++) != '\0' && !(/*is_dir_sep(c)*/ c == '/'))
> *dst++ = c;
> - if (is_dir_sep(c)) {
> + if (/*is_dir_sep(c)*/ c == '/') {
> *dst++ = '/';
> - while (is_dir_sep(c))
> + while (/*is_dir_sep(c)*/ c == '/')
> c = *src++;
> src--;
> } else if (!c)
>
Hmmm interesting
> In other words, in 'is_absolute_path()' and 'normalize_path_copy_len()', then
> just disable the '\' character as a path separator! ;)
>
> This was just to demonstrate the problem, not a serious patch, of course!
>
> I was away for the weekend and was expecting to see a patch to fix this
> on Gfw when I got back, but to my surprise there has been no mention of
> it on the mailing list (having now waded through the backlog!).
>
> To be clear, I can not imagine that this test passes on Gfw. However, this
> should have been failing the windows CI for ages, so ... perhaps I don't
> have a sufficiently vivid imagination. :)
>
> Anyway, the patch below 'fixes' the issue on cygwin.
>
> Thanks.
>
> ATB,
> Ramsay Jones
>
>
> ---- >8 ----
> From: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Date: Mon, 2 Jun 2025 22:51:33 +0100
> Subject: [PATCH] t6137: disable 'quoted glob' pathspecs on cygwin
>
> The backslash in a 'quoted glob' character is treated as a directory
> separator character on cygwin, which causes all such tests to fail.
> Skip all such tests on cygwin using the !CYGWIN prerequisite. While
> here, fix a few test titles as well.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> t/t6137-pathspec-wildcards-literal.sh | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
> index 20abad5667..229e48282e 100755
> --- a/t/t6137-pathspec-wildcards-literal.sh
> +++ b/t/t6137-pathspec-wildcards-literal.sh
> @@ -39,7 +39,7 @@ test_expect_success 'add wildcard *' '
> )
> '
>
> -test_expect_success 'add literal \*' '
> +test_expect_success !CYGWIN 'add literal \*' '
> git init test-asterisk-literal &&
> (
> cd test-asterisk-literal &&
> @@ -125,7 +125,7 @@ test_expect_success 'add wildcard f*' '
> )
> '
>
> -test_expect_success 'add literal f\*' '
> +test_expect_success !CYGWIN 'add literal f\*' '
> git init test-f-lit &&
> (
> cd test-f-lit &&
> @@ -156,7 +156,7 @@ test_expect_success 'add wildcard f**' '
> )
> '
>
> -test_expect_success 'add literal f\*\*' '
> +test_expect_success !CYGWIN 'add literal f\*\*' '
> git init test-fdstar-lit &&
> (
> cd test-fdstar-lit &&
> @@ -184,7 +184,7 @@ test_expect_success 'add wildcard f?z' '
> )
> '
>
> -test_expect_success 'add literal \? literal' '
> +test_expect_success !CYGWIN 'add literal \? literal' '
> git init test-q-lit &&
> (
> cd test-q-lit &&
> @@ -227,7 +227,7 @@ test_expect_success 'add wildcard hello?world' '
> )
> '
>
> -test_expect_success 'add literal hello\?world' '
> +test_expect_success !CYGWIN 'add literal hello\?world' '
> git init test-hellolit &&
> (
> cd test-hellolit &&
> @@ -241,7 +241,7 @@ test_expect_success 'add literal hello\?world' '
> )
> '
>
> -test_expect_success 'add literal [abc]' '
> +test_expect_success !CYGWIN 'add literal \[abc\]' '
> git init test-brackets-lit &&
> (
> cd test-brackets-lit &&
> @@ -280,7 +280,7 @@ test_expect_success 'commit: wildcard *' '
> )
> '
>
> -test_expect_success 'commit: literal *' '
> +test_expect_success !CYGWIN 'commit: literal \*' '
> git init test-c-asterisk-lit &&
> (
> cd test-c-asterisk-lit &&
> @@ -313,7 +313,7 @@ test_expect_success 'commit: wildcard f*' '
> )
> '
>
> -test_expect_success 'commit: literal f\*' '
> +test_expect_success !CYGWIN 'commit: literal f\*' '
> git init test-c-flit &&
> (
> cd test-c-flit &&
> @@ -328,7 +328,7 @@ test_expect_success 'commit: literal f\*' '
> )
> '
>
> -test_expect_success 'commit: wildcard pathspec limits commit' '
> +test_expect_success 'commit: wildcard f**' '
> git init test-c-pathlimit &&
> (
> cd test-c-pathlimit &&
> @@ -346,7 +346,7 @@ test_expect_success 'commit: wildcard pathspec limits commit' '
> )
> '
>
> -test_expect_success 'commit: literal f\*\*' '
> +test_expect_success !CYGWIN 'commit: literal f\*\*' '
> git init test-c-fdstar-lit &&
> (
> cd test-c-fdstar-lit &&
> @@ -379,7 +379,7 @@ test_expect_success 'commit: wildcard ?' '
> )
> '
>
> -test_expect_success 'commit: literal \?' '
> +test_expect_success !CYGWIN 'commit: literal \?' '
> git init test-c-qlit &&
> (
> cd test-c-qlit &&
> @@ -411,7 +411,7 @@ test_expect_success 'commit: wildcard hello?world' '
> )
> '
>
> -test_expect_success 'commit: literal hello\?world' '
> +test_expect_success !CYGWIN 'commit: literal hello\?world' '
> git init test-c-hellolit &&
> (
> cd test-c-hellolit &&
> --
> 2.49.0
This is an interesting breakdown of the problem
thanks for digging into it.
Just to clarify :) My change was limited to a minor condition in
`dir.c` to ensure that `MATCHED_EXACTLY` only proceeds when the
`nowildcard_len` matches the full pathspec length (i.e., it's a true
literal). I didn’t touch the `is_dir_sep()` logic or normalization
functions, so your debug trail really helped surface a side effect I
hadn’t anticipated.
That said, I'm more than happy to help work toward a more permanent
solution if needed, beyond the `!CYGWIN` skip. Let me know how you'd
prefer to proceed.
- Jayatheerth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: v2.25.0-rc0 test failure on cygwin
2025-06-02 22:33 v2.25.0-rc0 test failure on cygwin Ramsay Jones
2025-06-02 22:48 ` Ramsay Jones
2025-06-03 14:41 ` JAYATHEERTH K
@ 2025-06-03 16:11 ` Junio C Hamano
2025-06-03 19:03 ` Ramsay Jones
2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2025-06-03 16:11 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Adam Dinwoodie, jayatheerthkulkarni2005
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> I noticed on Friday, while testing the v2.25.0-rc0 build, that test t6137
> failed on cygwin:
>
> $ tail test-out-2-50-rc0
> Test Summary Report
> -------------------
> t6137-pathspec-wildcards-literal.sh (Wstat: 256 (exited 1) Tests: 25 Failed: 11)
> Failed tests: 2, 7, 9, 11, 14-15, 17, 19, 21, 23, 25
> Non-zero exit status: 1
> Files=1023, Tests=30946, 21783 wallclock secs (19.08 usr 42.17 sys + 4031.65 cusr 12965.78 csys = 17058.68 CPU)
> Result: FAIL
> make[1]: *** [Makefile:78: prove] Error 1
> make[1]: Leaving directory '/home/ramsay/git/t'
> make: *** [Makefile:3286: test] Error 2
> $
>
> A quick squint at the failing tests made it clear that the failure was
> caused by the cygwin build treating a quoted glob character sequence
> (e.g. '\*') as a directory separator char followed by a glob character.
Should we revert ec727e18 (dir.c: literal match with wildcard in
pathspec should still glob, 2025-05-03) before deciding to how to
proceed, as we will be deep in prerelease freeze?
It's not a "fix" for something that is gravely wrong but a glitch
people have lived with almost forever, so it can be reattempted in
the next cycle without hurting much.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: v2.25.0-rc0 test failure on cygwin
2025-06-03 16:11 ` Junio C Hamano
@ 2025-06-03 19:03 ` Ramsay Jones
0 siblings, 0 replies; 6+ messages in thread
From: Ramsay Jones @ 2025-06-03 19:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, Adam Dinwoodie, Patrick Steinhardt
On 03/06/2025 17:11, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
[snip]
>> A quick squint at the failing tests made it clear that the failure was
>> caused by the cygwin build treating a quoted glob character sequence
>> (e.g. '\*') as a directory separator char followed by a glob character.
>
> Should we revert ec727e18 (dir.c: literal match with wildcard in
> pathspec should still glob, 2025-05-03) before deciding to how to
> proceed, as we will be deep in prerelease freeze?
Heh, that decision is above my pay grade! :)
BTW, I realized earlier today why we had not seen any CI failures
on windows (yes I'm a bit slow on the uptake!); the test has this
conditional at the top:
test_have_prereq FUNNYNAMES || {
skip_all='skipping: needs FUNNYNAMES (non-Windows only)'
test_done
}
And FUNNYNAMES (as indicated) is set on MINGW/Gfw, but not cygwin.
(cygwin can cope with many 'funny' filenames, but it can't change
the use of '\' as a win32 directory separator).
> It's not a "fix" for something that is gravely wrong but a glitch
> people have lived with almost forever, so it can be reattempted in
> the next cycle without hurting much.
The POSIX/win32 pathname syntax 'issue' is not going away anytime
soon, so I suspect that just skipping these tests (or the entire
test file ala Gfw) is probably the best it gets. ;)
[see Patrick's commit 5f8af25ff9 (t5500, t5601: skip tests which
exercise paths with '[::1]' on Cygwin, 2024-10-16), which had been
failing from v2.25.0-rc0 (25 Dec 2019) to v2.48.0-rc0 (16 Dec 2024),
which was actually caused by a commit which started allowing a win32
drive 'letter' to be an multi-byte UTF8 Unicode char! :) ]
Thanks.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-03 19:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-02 22:33 v2.25.0-rc0 test failure on cygwin Ramsay Jones
2025-06-02 22:48 ` Ramsay Jones
2025-06-02 22:57 ` Kristoffer Haugsbakk
2025-06-03 14:41 ` JAYATHEERTH K
2025-06-03 16:11 ` Junio C Hamano
2025-06-03 19:03 ` Ramsay Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox