{"thread":{"id":"62349","subject":"clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","startedAt":"2024-10-17T03:51:10Z","lastAt":"2024-10-21T19:41:43Z","messageCount":18,"participants":["Bagas Sanjaya","Patrick Steinhardt","Taylor Blau","brian m. carlson","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"505324","messageId":"ZxCJqe4-rsRo1yHg@archie.me","threadId":"62349","inReplyTo":null,"subject":"clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2024-10-17T03:51:05Z","receivedAt":"2024-10-17T03:51:10Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Hi,\n\nSince clar unit testing framework was imported by commit 9b7caa2809cb (t:\nimport the clar unit testing framework, 2024-09-04), Git FTBFS on uclibc\nsystems built by Buildroot:\n\n```\n CC t/unit-tests/unit-test.o\nt/unit-tests/clar/clar.c: In function 'clar__assert_equal':\nt/unit-tests/clar/clar.c:767:23: error: unknown type name 'wchar_t'\n 767 | const wchar_t *wcs1 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nIn file included from t/unit-tests/clar/clar.c:13:\nt/unit-tests/clar/clar.c:767:58: error: unknown type name 'wchar_t'\n 767 | const wchar_t *wcs1 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:768:23: error: unknown type name 'wchar_t'\n 768 | const wchar_t *wcs2 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:768:58: error: unknown type name 'wchar_t'\n 768 | const wchar_t *wcs2 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:769:65: warning: implicit declaration of function 'wcscmp' [-Wimplicit-function-declaration]\n 769 | is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcscmp(wcs1, wcs2);\n | ^~~~~~\nt/unit-tests/clar/clar.c:784:23: error: unknown type name 'wchar_t'\n 784 | const wchar_t *wcs1 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:784:58: error: unknown type name 'wchar_t'\n 784 | const wchar_t *wcs1 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:785:23: error: unknown type name 'wchar_t'\n 785 | const wchar_t *wcs2 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:785:58: error: unknown type name 'wchar_t'\n 785 | const wchar_t *wcs2 = va_arg(args, const wchar_t *);\n | ^~~~~~~\nt/unit-tests/clar/clar.c:787:65: warning: implicit declaration of function 'wcsncmp' [-Wimplicit-function-declaration]\n 787 | is_equal = (!wcs1 || !wcs2) ? (wcs1 == wcs2) : !wcsncmp(wcs1, wcs2, len);\n | ^~~~~~~\nmake[1]: *** [Makefile:2795: t/unit-tests/clar/clar.o] Error 1\n```\n\nSee [1] for the full build log.\n\nThanks.\n\n[1]: https://autobuild.buildroot.org/results/8cc9795dc18277926dd386eb1cb9f8c9b65b0042/build-end.log\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"505369","messageId":"ZxESP0xHV4cK64i0@pks.im","threadId":"62349","inReplyTo":"ZxCJqe4-rsRo1yHg@archie.me","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-17T13:33:51Z","receivedAt":"2024-10-17T13:33:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 17, 2024 at 10:51:05AM +0700, Bagas Sanjaya wrote:\n> Hi,\n> \n> Since clar unit testing framework was imported by commit 9b7caa2809cb (t:\n> import the clar unit testing framework, 2024-09-04), Git FTBFS on uclibc\n> systems built by Buildroot:\n\nWait a second, that doesn't sound right to me. `wchar_t` is part of ISO\nC90, so any system not supporting it would basically be unsupported by\nus from my point of view. And indeed, uclibc _does_ support that type\nalright. I guess the issue is rather that we're relying on some kind of\nplatform-specific behaviour and thus don't include the correct header.\n\nI'll have a look, thanks for the report!\n\nPatrick\n"},{"id":"505370","messageId":"ZxEXFI80i4Q_4NJT@pks.im","threadId":"62349","inReplyTo":"ZxESP0xHV4cK64i0@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-17T13:54:28Z","receivedAt":"2024-10-17T13:54:35Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 17, 2024 at 03:33:51PM +0200, Patrick Steinhardt wrote:\n> On Thu, Oct 17, 2024 at 10:51:05AM +0700, Bagas Sanjaya wrote:\n> > Hi,\n> > \n> > Since clar unit testing framework was imported by commit 9b7caa2809cb (t:\n> > import the clar unit testing framework, 2024-09-04), Git FTBFS on uclibc\n> > systems built by Buildroot:\n> \n> Wait a second, that doesn't sound right to me. `wchar_t` is part of ISO\n> C90, so any system not supporting it would basically be unsupported by\n> us from my point of view. And indeed, uclibc _does_ support that type\n> alright. I guess the issue is rather that we're relying on some kind of\n> platform-specific behaviour and thus don't include the correct header.\n> \n> I'll have a look, thanks for the report!\n\nOkay, uclibc indeed has _optional_ support for `wchar_t`. But what\nreally throws me off: \"include/wchar.h\" from uclibc has the following\nsnippet right at the top:\n\n #ifndef __UCLIBC_HAS_WCHAR__\n #error Attempted to include wchar.h when uClibc built without wide char support.\n #endif\n\nWe unconditionally include , and your system does not seem to\nhave support for it built in. So why doesn't the `#error` trigger? It's\nalso not like this is a recent error, it has been added with 581deed72\n(The obligatory forgotten files..., 2002-05-06).\n\nWe can do something like the below patch in clar, but I'd first like to\nunderstand why your platform seems to be broken in such a way.\n\nPatrick\n\ndiff --git a/clar.c b/clar.c\nindex 64879cf..06fe3d1 100644\n--- a/clar.c\n+++ b/clar.c\n@@ -9,6 +9,11 @@\n #define _DARWIN_C_SOURCE\n #define _DEFAULT_SOURCE\n \n+#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)\n+#else\n+#\tdefine HAVE_WCHAR\n+#endif\n+\n #include \n #include \n #include \n@@ -16,7 +21,9 @@\n #include \n #include \n #include \n+#ifdef HAVE_WCHAR\n #include \n+#endif\n #include \n #include \n \n@@ -766,6 +773,7 @@ void clar__assert_equal(\n \t\t\t}\n \t\t}\n \t}\n+#ifdef HAVE_WCHAR\n \telse if (!strcmp(\"%ls\", fmt)) {\n \t\tconst wchar_t *wcs1 = va_arg(args, const wchar_t *);\n \t\tconst wchar_t *wcs2 = va_arg(args, const wchar_t *);\n@@ -801,6 +809,7 @@ void clar__assert_equal(\n \t\t\t}\n \t\t}\n \t}\n+#endif // HAVE_WCHAR\n \telse if (!strcmp(\"%\"PRIuMAX, fmt) || !strcmp(\"%\"PRIxMAX, fmt)) {\n \t\tuintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t);\n \t\tis_equal = (sz1 == sz2);\n\n"},{"id":"505387","messageId":"ZxFuxanlnQCHPial@nand.local","threadId":"62349","inReplyTo":"ZxEXFI80i4Q_4NJT@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-17T20:08:37Z","receivedAt":"2024-10-17T20:08:41Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Oct 17, 2024 at 03:54:28PM +0200, Patrick Steinhardt wrote:\n> We can do something like the below patch in clar, but I'd first like to\n> understand why your platform seems to be broken in such a way.\n\nThanks, both, for the report and investigation.\n\nThanks,\nTaylor\n"},{"id":"505399","messageId":"ZxGN9zzt55GcL4Qj@tapette.crustytoothpaste.net","threadId":"62349","inReplyTo":"ZxEXFI80i4Q_4NJT@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-10-17T22:21:43Z","receivedAt":"2024-10-17T22:21:45Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-10-17 at 13:54:28, Patrick Steinhardt wrote:\n> On Thu, Oct 17, 2024 at 03:33:51PM +0200, Patrick Steinhardt wrote:\n> > On Thu, Oct 17, 2024 at 10:51:05AM +0700, Bagas Sanjaya wrote:\n> > > Hi,\n> > > \n> > > Since clar unit testing framework was imported by commit 9b7caa2809cb (t:\n> > > import the clar unit testing framework, 2024-09-04), Git FTBFS on uclibc\n> > > systems built by Buildroot:\n> > \n> > Wait a second, that doesn't sound right to me. `wchar_t` is part of ISO\n> > C90, so any system not supporting it would basically be unsupported by\n> > us from my point of view. And indeed, uclibc _does_ support that type\n> > alright. I guess the issue is rather that we're relying on some kind of\n> > platform-specific behaviour and thus don't include the correct header.\n> > \n> > I'll have a look, thanks for the report!\n> \n> Okay, uclibc indeed has _optional_ support for `wchar_t`. But what\n> really throws me off: \"include/wchar.h\" from uclibc has the following\n> snippet right at the top:\n> \n> #ifndef __UCLIBC_HAS_WCHAR__\n> #error Attempted to include wchar.h when uClibc built without wide char support.\n> #endif\n> \n> We unconditionally include , and your system does not seem to\n> have support for it built in. So why doesn't the `#error` trigger? It's\n> also not like this is a recent error, it has been added with 581deed72\n> (The obligatory forgotten files..., 2002-05-06).\n> \n> We can do something like the below patch in clar, but I'd first like to\n> understand why your platform seems to be broken in such a way.\n\nYeah, this is definitely broken. We require ISO C99, and according to\nthe draft preceding the ratification[0], `wchar.h` and its contents are not\noptional. The similar draft for C11 also doesn't appear to make these\noptional.\n\nI think users of uclibc will need to compile it with full ISO C99\nsupport. I expect that a wide variety of other software will be\nsimilarly broken without that.\n\n[0] Chosen because it is available for at no charge and the standard is not.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"505406","messageId":"20241018045155.GC2408674@coredump.intra.peff.net","threadId":"62349","inReplyTo":"ZxGN9zzt55GcL4Qj@tapette.crustytoothpaste.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-10-18T04:51:55Z","receivedAt":"2024-10-18T04:51:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 17, 2024 at 10:21:43PM +0000, brian m. carlson wrote:\n\n> > We unconditionally include , and your system does not seem to\n> > have support for it built in. So why doesn't the `#error` trigger? It's\n> > also not like this is a recent error, it has been added with 581deed72\n> > (The obligatory forgotten files..., 2002-05-06).\n> > \n> > We can do something like the below patch in clar, but I'd first like to\n> > understand why your platform seems to be broken in such a way.\n> \n> Yeah, this is definitely broken. We require ISO C99, and according to\n> the draft preceding the ratification[0], `wchar.h` and its contents are not\n> optional. The similar draft for C11 also doesn't appear to make these\n> optional.\n> \n> I think users of uclibc will need to compile it with full ISO C99\n> support. I expect that a wide variety of other software will be\n> similarly broken without that.\n\nPerhaps, but...don't the current releases of Git work just fine on such\na wchar-less uclibc system now? We don't use wchar or include wchar.h\nourselves, except on Windows or via compat/regex (though it is even\nconditional there). This is a new portability problem introduced by the\nclar test harness. And even there I doubt it is something we care about\n(it looks like it's for allowing \"%ls\" in assertions).\n\nOur approach to portability has traditionally been a cost/benefit for\nindividual features. Standards are a nice guideline, but the real world\ndoes not always follow them. Sometimes accommodating platforms that\ndon't strictly follow the standard is cheap enough that it's worth\ndoing.\n\nI think more recent discussions have trended to looking at standards in\na bit stronger way: giving minimum requirements and sticking to them.\nCertainly I'm sympathetic to that viewpoint, as it can reduce noise.\n\nBut IMHO this is a good example of where the flexibility of the first\napproach shines. We could accommodate this platform without any real\ncost (and indeed, we should be able to _drop_ some clar code).\n\n-Peff\n"},{"id":"505407","messageId":"ZxHrIBCdnwdRdXAv@pks.im","threadId":"62349","inReplyTo":"20241018045155.GC2408674@coredump.intra.peff.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-18T04:59:17Z","receivedAt":"2024-10-18T04:59:23Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 12:51:55AM -0400, Jeff King wrote:\n> On Thu, Oct 17, 2024 at 10:21:43PM +0000, brian m. carlson wrote:\n> \n> > > We unconditionally include , and your system does not seem to\n> > > have support for it built in. So why doesn't the `#error` trigger? It's\n> > > also not like this is a recent error, it has been added with 581deed72\n> > > (The obligatory forgotten files..., 2002-05-06).\n> > > \n> > > We can do something like the below patch in clar, but I'd first like to\n> > > understand why your platform seems to be broken in such a way.\n> > \n> > Yeah, this is definitely broken. We require ISO C99, and according to\n> > the draft preceding the ratification[0], `wchar.h` and its contents are not\n> > optional. The similar draft for C11 also doesn't appear to make these\n> > optional.\n> > \n> > I think users of uclibc will need to compile it with full ISO C99\n> > support. I expect that a wide variety of other software will be\n> > similarly broken without that.\n> \n> Perhaps, but...don't the current releases of Git work just fine on such\n> a wchar-less uclibc system now? We don't use wchar or include wchar.h\n> ourselves, except on Windows or via compat/regex (though it is even\n> conditional there). This is a new portability problem introduced by the\n> clar test harness. And even there I doubt it is something we care about\n> (it looks like it's for allowing \"%ls\" in assertions).\n> \n> Our approach to portability has traditionally been a cost/benefit for\n> individual features. Standards are a nice guideline, but the real world\n> does not always follow them. Sometimes accommodating platforms that\n> don't strictly follow the standard is cheap enough that it's worth\n> doing.\n> \n> I think more recent discussions have trended to looking at standards in\n> a bit stronger way: giving minimum requirements and sticking to them.\n> Certainly I'm sympathetic to that viewpoint, as it can reduce noise.\n> \n> But IMHO this is a good example of where the flexibility of the first\n> approach shines. We could accommodate this platform without any real\n> cost (and indeed, we should be able to _drop_ some clar code).\n\nWell, dropping doesn't work as it breaks other projects that depend on\nthe clar-features that depend on `wchar_t`. But other than that I agree\nand would like to fix this issue, also because it potentially benefits\nother users of the clar.\n\nThe only problem is that the platform seems to be severely broken. As\nmentioned elsewhere, we have this snippet in uclibc's \"wchar.h\":\n\n #ifndef __UCLIBC_HAS_WCHAR__\n #error Attempted to include wchar.h when uClibc built without wide char support.\n #endif\n\nSo if __UCLIBC_HAS_WCHAR__ is not defined, we should see the error. But\nthe report didn't show the error, which means that the define has to be\nset. And consequently we have no way to patch around this, because the\nmacro that we're supposed to use is broken.\n\nMight be I'm missing something with how uclibc is intended to work. But\nif I'm right then this is just a broken platform, and I don't think\nworking around it would be sensible. Otherwise I'd be happy to make the\nwchar-related code conditional as shown in the preliminary patch posted\nin [1].\n\nPatrick\n\n[1]: \n"},{"id":"505408","messageId":"20241018052448.GD2408674@coredump.intra.peff.net","threadId":"62349","inReplyTo":"ZxHrIBCdnwdRdXAv@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-10-18T05:24:48Z","receivedAt":"2024-10-18T05:24:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 18, 2024 at 06:59:17AM +0200, Patrick Steinhardt wrote:\n\n> > But IMHO this is a good example of where the flexibility of the first\n> > approach shines. We could accommodate this platform without any real\n> > cost (and indeed, we should be able to _drop_ some clar code).\n> \n> Well, dropping doesn't work as it breaks other projects that depend on\n> the clar-features that depend on `wchar_t`. But other than that I agree\n> and would like to fix this issue, also because it potentially benefits\n> other users of the clar.\n\nSo that's a rabbit hole I didn't go down in my other message. ;)\n\nBut another traditional philosophy the Git project has had is to be very\nconservative in our dependencies. And now we have this new dependency,\nand already it is causing a portability problem.\n\nI don't think that means we should throw away the dependency. But if we\nare inheriting portability problems from imported code, I think we\nshould consider to what degree we can lightly tweak that code to match\nour project. I don't care what clar does upstream. If _we_ don't need\nwchar support, we can drop it or #ifdef it out.\n\nOverall, I'm a little sad to see all of the #includes in clar.c. We have\nspent 20 years building up git-compat-util.h to meet our needs for\nportability, and there are lots of subtle bits in there about what is\nincluded and when, along with various wrappers. And now we have a new\nsubsystem which doesn't use _any_ of that, and has its own set of\nincludes and wrappers. It seems inevitable that we are going to run into\ncases where a platform we support isn't handled by clar, or that we'll\nhave to duplicate our solution in both places. I wish it were just using\ngit-compat-util.h. I know that means essentially forking, but I think I\nmay prefer that to inheriting some other project's portability problems.\n\n> The only problem is that the platform seems to be severely broken. As\n> mentioned elsewhere, we have this snippet in uclibc's \"wchar.h\":\n> \n> #ifndef __UCLIBC_HAS_WCHAR__\n> #error Attempted to include wchar.h when uClibc built without wide char support.\n> #endif\n\nYeah, I have no clue what's going on there. Certainly I have no problem\nif you want to dig further to get confidence in the direction we choose.\n\n-Peff\n"},{"id":"505410","messageId":"ZxHylOLHaxP8crom@pks.im","threadId":"62349","inReplyTo":"20241018052448.GD2408674@coredump.intra.peff.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-18T05:31:00Z","receivedAt":"2024-10-18T05:31:07Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 01:24:48AM -0400, Jeff King wrote:\n> On Fri, Oct 18, 2024 at 06:59:17AM +0200, Patrick Steinhardt wrote:\n> \n> > > But IMHO this is a good example of where the flexibility of the first\n> > > approach shines. We could accommodate this platform without any real\n> > > cost (and indeed, we should be able to _drop_ some clar code).\n> > \n> > Well, dropping doesn't work as it breaks other projects that depend on\n> > the clar-features that depend on `wchar_t`. But other than that I agree\n> > and would like to fix this issue, also because it potentially benefits\n> > other users of the clar.\n> \n> So that's a rabbit hole I didn't go down in my other message. ;)\n> \n> But another traditional philosophy the Git project has had is to be very\n> conservative in our dependencies. And now we have this new dependency,\n> and already it is causing a portability problem.\n> \n> I don't think that means we should throw away the dependency. But if we\n> are inheriting portability problems from imported code, I think we\n> should consider to what degree we can lightly tweak that code to match\n> our project. I don't care what clar does upstream. If _we_ don't need\n> wchar support, we can drop it or #ifdef it out.\n> \n> Overall, I'm a little sad to see all of the #includes in clar.c. We have\n> spent 20 years building up git-compat-util.h to meet our needs for\n> portability, and there are lots of subtle bits in there about what is\n> included and when, along with various wrappers. And now we have a new\n> subsystem which doesn't use _any_ of that, and has its own set of\n> includes and wrappers. It seems inevitable that we are going to run into\n> cases where a platform we support isn't handled by clar, or that we'll\n> have to duplicate our solution in both places. I wish it were just using\n> git-compat-util.h. I know that means essentially forking, but I think I\n> may prefer that to inheriting some other project's portability problems.\n\nWell, I'm of a different mind here. It sure is more work for now, and I\nhave been chipping away at the issues. But in the end, it's not only us\nwho benefit, but the overall ecosystem because others can use clar on\nmore or less esoteric platforms, too. It's part of the reason why I have\nbeen advocating for clar in the first place: we have a good relationship\nto its maintainers, so it is easy to upstream changes.\n\nSo yes, right now we feel a bit of pain there. But that's going to go\naway, and from thereon everyone benefits.\n\n> > The only problem is that the platform seems to be severely broken. As\n> > mentioned elsewhere, we have this snippet in uclibc's \"wchar.h\":\n> > \n> > #ifndef __UCLIBC_HAS_WCHAR__\n> > #error Attempted to include wchar.h when uClibc built without wide char support.\n> > #endif\n> \n> Yeah, I have no clue what's going on there. Certainly I have no problem\n> if you want to dig further to get confidence in the direction we choose.\n\nYup, I'll do that, but first need additional input from the reporter. I\ndon't have a uclibc platform, and couldn't really find obvious Docker\nimages to reproduce the issue with.\n\nPatrick\n"},{"id":"505439","messageId":"ZxJnfYtuxnAEBc1E@archie.me","threadId":"62349","inReplyTo":"ZxEXFI80i4Q_4NJT@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2024-10-18T13:49:49Z","receivedAt":"2024-10-18T13:49:54Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On Thu, Oct 17, 2024 at 03:54:28PM +0200, Patrick Steinhardt wrote:\n> Okay, uclibc indeed has _optional_ support for `wchar_t`. But what\n> really throws me off: \"include/wchar.h\" from uclibc has the following\n> snippet right at the top:\n> \n> #ifndef __UCLIBC_HAS_WCHAR__\n> #error Attempted to include wchar.h when uClibc built without wide char support.\n> #endif\n> \n> We unconditionally include , and your system does not seem to\n> have support for it built in. So why doesn't the `#error` trigger? It's\n> also not like this is a recent error, it has been added with 581deed72\n> (The obligatory forgotten files..., 2002-05-06).\n> \n> We can do something like the below patch in clar, but I'd first like to\n> understand why your platform seems to be broken in such a way.\n> \n> Patrick\n> \n> diff --git a/clar.c b/clar.c\n> index 64879cf..06fe3d1 100644\n> --- a/clar.c\n> +++ b/clar.c\n> @@ -9,6 +9,11 @@\n> #define _DARWIN_C_SOURCE\n> #define _DEFAULT_SOURCE\n> \n> +#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)\n> +#else\n> +#\tdefine HAVE_WCHAR\n> +#endif\n> +\n> #include \n> #include \n> #include \n> @@ -16,7 +21,9 @@\n> #include \n> #include \n> #include \n> +#ifdef HAVE_WCHAR\n> #include \n> +#endif\n> #include \n> #include \n> \n> @@ -766,6 +773,7 @@ void clar__assert_equal(\n> \t\t\t}\n> \t\t}\n> \t}\n> +#ifdef HAVE_WCHAR\n> \telse if (!strcmp(\"%ls\", fmt)) {\n> \t\tconst wchar_t *wcs1 = va_arg(args, const wchar_t *);\n> \t\tconst wchar_t *wcs2 = va_arg(args, const wchar_t *);\n> @@ -801,6 +809,7 @@ void clar__assert_equal(\n> \t\t\t}\n> \t\t}\n> \t}\n> +#endif // HAVE_WCHAR\n> \telse if (!strcmp(\"%\"PRIuMAX, fmt) || !strcmp(\"%\"PRIxMAX, fmt)) {\n> \t\tuintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t);\n> \t\tis_equal = (sz1 == sz2);\n> \n\nHi,\n\nOn Buildroot site, Edgar Bonet (Cc:'ed) suggests to improve your patch by\nwrapping strcmps [1]:\n\n---- >8 ----\ndiff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c\nindex cef0f023c2..6de0b415b1 100644\n--- a/t/unit-tests/clar/clar.c\n+++ b/t/unit-tests/clar/clar.c\n@@ -18,6 +18,13 @@\n #include \n #include \n \n+#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)\n+ /* uClibc can be built without wchar support, in which case the\n+ installed is a stub that does not define wchar_t. */\n+#else\n+# define HAVE_WCHAR\n+#endif\n+\n #ifdef _WIN32\n #\tdefine WIN32_LEAN_AND_MEAN\n #\tinclude \n@@ -763,6 +770,7 @@ void clar__assert_equal(\n \t\t\t}\n \t\t}\n \t}\n+#ifdef HAVE_WCHAR\n \telse if (!strcmp(\"%ls\", fmt)) {\n \t\tconst wchar_t *wcs1 = va_arg(args, const wchar_t *);\n \t\tconst wchar_t *wcs2 = va_arg(args, const wchar_t *);\n@@ -798,6 +806,7 @@ void clar__assert_equal(\n \t\t\t}\n \t\t}\n \t}\n+#endif // HAVE_WCHAR\n \telse if (!strcmp(\"%\"PRIuZ, fmt) || !strcmp(\"%\"PRIxZ, fmt)) {\n \t\tsize_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t);\n \t\tis_equal = (sz1 == sz2);\n\nThanks.\n\n[1]: https://lore.kernel.org/buildroot/f517190c-6fcd-4101-afa6-f6ea521feb9e@grenoble.cnrs.fr/\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"505457","messageId":"ZxLAC-c4y7_sQqzw@tapette.crustytoothpaste.net","threadId":"62349","inReplyTo":"20241018045155.GC2408674@coredump.intra.peff.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-10-18T20:07:39Z","receivedAt":"2024-10-18T20:07:41Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-10-18 at 04:51:55, Jeff King wrote:\n> Perhaps, but...don't the current releases of Git work just fine on such\n> a wchar-less uclibc system now? We don't use wchar or include wchar.h\n> ourselves, except on Windows or via compat/regex (though it is even\n> conditional there). This is a new portability problem introduced by the\n> clar test harness. And even there I doubt it is something we care about\n> (it looks like it's for allowing \"%ls\" in assertions).\n> \n> Our approach to portability has traditionally been a cost/benefit for\n> individual features. Standards are a nice guideline, but the real world\n> does not always follow them. Sometimes accommodating platforms that\n> don't strictly follow the standard is cheap enough that it's worth\n> doing.\n> \n> I think more recent discussions have trended to looking at standards in\n> a bit stronger way: giving minimum requirements and sticking to them.\n> Certainly I'm sympathetic to that viewpoint, as it can reduce noise.\n\nI think there's a tradeoff here in what is reasonable to support. For\nexample, we know FreeBSD and NetBSD return something than the\nPOSIX-mandated ELOOP for an open(2) on a symlink with O_NOFOLLOW. This\nis really minor to work around, and overall, both operating systems\noverwhelmingly are easy to support and run almost all POSIX-compatible\nsoftware with ease and support a modern POSIX version.\n\nAnd then we have uclibc, which has decided to optionally fail to\nimplement obligatory functionality that has been around for 35 years,\nwhich is longer than some of my colleagues have been alive. This isn't\neven that it doesn't implement all of the POSIX functionality, but that\nit doesn't even support what was standardized in C89—not C17, or C11, or\nC99, but C89.\n\nI should point out that the entirety of musl, a competing libc which\ndoes ship with fully functional wide character support, is less than 800\nKiB in shared object form, so there's really no defensible reason for\nthis option to even exist. Nobody is shipping even embedded Linux\nsystems with so little storage or memory that this option matters\n(because you need a decent amount of space for the kernel as well),\nespecially considering that flash can be cheaper than CAD 0.06 per GB.\n\nThe difference is one set of systems has a minor incompatibility that\nrequires little work to work around and has few practical effects, and\nthe other tried to exclude major functionality from a tiny, ancient\nstandard, the result of which is a wide variety of software that's\nbroken. (For example, ncurses normally builds a wide character\nversion of the shared library in addition to the byte-based version.)\n\nSo I agree that we should allow minor variances for nonconformance,\nbecause very few systems practically comply with all of the standards\n(Linux, for example, does not). That's a prudent and sensible thing to\ndo, and we should definitely continue to do that in the future. But\ngiven that this is major core functionality in the standard and there's\nactually an option to include it which this distributor has just chosen\nnot to enable, I think it's fine for us to tell the distributor to just\nuse the appropriate compile-time option for their libc.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"505459","messageId":"ZxLKLJCgiA3oY4Nr@nand.local","threadId":"62349","inReplyTo":"ZxHylOLHaxP8crom@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-18T20:50:52Z","receivedAt":"2024-10-18T20:50:56Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Oct 18, 2024 at 07:31:00AM +0200, Patrick Steinhardt wrote:\n> > Overall, I'm a little sad to see all of the #includes in clar.c. We have\n> > spent 20 years building up git-compat-util.h to meet our needs for\n> > portability, and there are lots of subtle bits in there about what is\n> > included and when, along with various wrappers. And now we have a new\n> > subsystem which doesn't use _any_ of that, and has its own set of\n> > includes and wrappers. It seems inevitable that we are going to run into\n> > cases where a platform we support isn't handled by clar, or that we'll\n> > have to duplicate our solution in both places. I wish it were just using\n> > git-compat-util.h. I know that means essentially forking, but I think I\n> > may prefer that to inheriting some other project's portability problems.\n>\n> Well, I'm of a different mind here. It sure is more work for now, and I\n> have been chipping away at the issues. But in the end, it's not only us\n> who benefit, but the overall ecosystem because others can use clar on\n> more or less esoteric platforms, too. It's part of the reason why I have\n> been advocating for clar in the first place: we have a good relationship\n> to its maintainers, so it is easy to upstream changes.\n>\n> So yes, right now we feel a bit of pain there. But that's going to go\n> away, and from thereon everyone benefits.\n\nI'm not sure I agree wholly here. In particular, I think saying \"a bit\"\nof pain may not be capturing the full picture.\n\nWill it take us another 20 years to resolve all of the portability\nissues which Clar suffers from (but git-compat-util.h doesn't)?\nProbably not 20 years, but I don't think that it's on the complete other\nend of the spectrum, either.\n\nTBH, I would not at all be sad to see us \"fork\" Clar into our own tree\nor have a new repository upstream that we track with submodules, etc.\n\nThanks,\nTaylor\n"},{"id":"505587","messageId":"ZxX4AsMRoC0Botcj@pks.im","threadId":"62349","inReplyTo":"ZxLKLJCgiA3oY4Nr@nand.local","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T06:44:01Z","receivedAt":"2024-10-21T06:44:11Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 04:50:52PM -0400, Taylor Blau wrote:\n> On Fri, Oct 18, 2024 at 07:31:00AM +0200, Patrick Steinhardt wrote:\n> > > Overall, I'm a little sad to see all of the #includes in clar.c. We have\n> > > spent 20 years building up git-compat-util.h to meet our needs for\n> > > portability, and there are lots of subtle bits in there about what is\n> > > included and when, along with various wrappers. And now we have a new\n> > > subsystem which doesn't use _any_ of that, and has its own set of\n> > > includes and wrappers. It seems inevitable that we are going to run into\n> > > cases where a platform we support isn't handled by clar, or that we'll\n> > > have to duplicate our solution in both places. I wish it were just using\n> > > git-compat-util.h. I know that means essentially forking, but I think I\n> > > may prefer that to inheriting some other project's portability problems.\n> >\n> > Well, I'm of a different mind here. It sure is more work for now, and I\n> > have been chipping away at the issues. But in the end, it's not only us\n> > who benefit, but the overall ecosystem because others can use clar on\n> > more or less esoteric platforms, too. It's part of the reason why I have\n> > been advocating for clar in the first place: we have a good relationship\n> > to its maintainers, so it is easy to upstream changes.\n> >\n> > So yes, right now we feel a bit of pain there. But that's going to go\n> > away, and from thereon everyone benefits.\n> \n> I'm not sure I agree wholly here. In particular, I think saying \"a bit\"\n> of pain may not be capturing the full picture.\n> \n> Will it take us another 20 years to resolve all of the portability\n> issues which Clar suffers from (but git-compat-util.h doesn't)?\n> Probably not 20 years, but I don't think that it's on the complete other\n> end of the spectrum, either.\n\nMy assumption is that we'll iron out the issues in this release. Our\n\"git-compat-util.h\" has grown _huge_, but that's mostly because it needs\nto support a ton of different things. The Git codebase is orders of\nmagnitude bigger than the clar, so it is totally expected that it also\nexercises way more edge cases in C. Conversely, I expect that the compat\nheaders in clar need to only be a fraction of what we have.\n\nI don't really understand where the claim comes from that this is such a\nhuge pain. Sure, there's been a bit of back and forth now. But all of\nthe reports I received were easy to fix, and I've fixed them upstream in\na matter of days.\n\nI'd really like us to take a step back here and take things a bit more\nrelaxed. If we see that this continues to be a major pain to maintain\nthen yes, I agree, we should likely rope in our own compat headers. But\nfrom my point of view there isn't really indication that this is going\nto be the case.\n\n> TBH, I would not at all be sad to see us \"fork\" Clar into our own tree\n> or have a new repository upstream that we track with submodules, etc.\n\nI don't see any reason for this. If we eventually figure out that we\nwant to rope in our own compat headers we can wire up support for this\nwithout forking clar.\n\nIn any case, I'll reroll my patch series that updates the clar today or\ntomorrow to incorporate a fix for the reported issue.\n\nPatrick\n"},{"id":"505588","messageId":"ZxX4YtZ5lW0axToT@pks.im","threadId":"62349","inReplyTo":"ZxJnfYtuxnAEBc1E@archie.me","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T06:44:50Z","receivedAt":"2024-10-21T06:44:57Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 08:49:49PM +0700, Bagas Sanjaya wrote:\n> On Thu, Oct 17, 2024 at 03:54:28PM +0200, Patrick Steinhardt wrote:\n[snip]\n> On Buildroot site, Edgar Bonet (Cc:'ed) suggests to improve your patch by\n> wrapping strcmps [1]:\n\nThanks. So with the proposed patch things work?\n\nOne thing I still don't understand: why don't you get an error from\nincludeng ? As shown above, it should contain the following\nsnippet:\n\n> > #ifndef __UCLIBC_HAS_WCHAR__\n> > #error Attempted to include wchar.h when uClibc built without wide char support.\n> > #endif\n\nSo I'd expect that to trigger and cause the build to abort. Am I missing\nanything here? Let me have another look at uclibc...\n\nOh, yes! There's also a \"wchar-stub.h\" file that replaces \"wchar.h\" when\ncompiled without UCLIBC_HAS_WCHAR=Yes. And that file indeed does not\ncause us to error out, but is a stub. It also explains why the logs do\nnot complain about the missing `wchar_t` type, as that type does get\ndefined by the stub header. Weird, but so be it.\n\nI've picked this up upstream via [1].\n\n[1]: https://github.com/clar-test/clar/pull/108\n\nPatrick\n"},{"id":"505727","messageId":"20241021191934.GE1219228@coredump.intra.peff.net","threadId":"62349","inReplyTo":"ZxLAC-c4y7_sQqzw@tapette.crustytoothpaste.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-10-21T19:19:34Z","receivedAt":"2024-10-21T19:19:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 18, 2024 at 08:07:39PM +0000, brian m. carlson wrote:\n\n> The difference is one set of systems has a minor incompatibility that\n> requires little work to work around and has few practical effects, and\n> the other tried to exclude major functionality from a tiny, ancient\n> standard, the result of which is a wide variety of software that's\n> broken. (For example, ncurses normally builds a wide character\n> version of the shared library in addition to the byte-based version.)\n\nI think this is the crux of where we see things differently. I don't see\nwchar as major functionality, since it's not something that Git has ever\nused! It's an include that we pulled in via a dependency to implement a\nminor feature that we aren't even making use of. We can just not compile\nthat otherwise dead code, and everything works as it did in the past. We\ndo not have to pass judgement on uclibc's feature completeness or make\nlife any harder for users on such systems.\n\n(This is all assuming that we are dealing with a uclibc with disabled\nfeatures in the first place. I don't think we've seen an answer to\nPatrick's questions about why the #error is not triggering).\n\n-Peff\n"},{"id":"505728","messageId":"20241021193024.GF1219228@coredump.intra.peff.net","threadId":"62349","inReplyTo":"ZxX4AsMRoC0Botcj@pks.im","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-10-21T19:30:24Z","receivedAt":"2024-10-21T19:30:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 21, 2024 at 08:44:01AM +0200, Patrick Steinhardt wrote:\n\n> > Will it take us another 20 years to resolve all of the portability\n> > issues which Clar suffers from (but git-compat-util.h doesn't)?\n> > Probably not 20 years, but I don't think that it's on the complete other\n> > end of the spectrum, either.\n> \n> My assumption is that we'll iron out the issues in this release. Our\n> \"git-compat-util.h\" has grown _huge_, but that's mostly because it needs\n> to support a ton of different things. The Git codebase is orders of\n> magnitude bigger than the clar, so it is totally expected that it also\n> exercises way more edge cases in C. Conversely, I expect that the compat\n> headers in clar need to only be a fraction of what we have.\n> \n> I don't really understand where the claim comes from that this is such a\n> huge pain. Sure, there's been a bit of back and forth now. But all of\n> the reports I received were easy to fix, and I've fixed them upstream in\n> a matter of days.\n> \n> I'd really like us to take a step back here and take things a bit more\n> relaxed. If we see that this continues to be a major pain to maintain\n> then yes, I agree, we should likely rope in our own compat headers. But\n> from my point of view there isn't really indication that this is going\n> to be the case.\n\nI'm OK with that direction. Just to be clear, I think you've done a\ngreat job (as you always do) of responding to the issue promptly and\nkeeping things moving forward. And you're right that there is a good\nchance that we iron out this wrinkle and never worry about it again. If\nthat doesn't turn out to be the case, we can iterate from there.\n\nMy thinking / response was mostly just: git-compat-util.h has many\nsubtle fixes we've accumulated through battle-testing. To the point that\nI don't think we even know which ones are important and which are not\nanymore. That's why our guidelines say that everything should include it\nfirst, rather than trying to handle system headers themselves. Clar\nviolates that rule, and if it were original code within Git it probably\nwould have been flagged in review as such. But since it's imported,\nthere's some tension there between making the code as Git-like as\npossible, and keeping it easy to track upstream.\n\nI tend to err on the \"fork and make it Git-like\" side of that line, but\ncertainly there is an argument for the other way. Anyway, let's deal\nwith this wchar.h issue and then see if it ever even comes up again.\n\n-Peff\n"},{"id":"505729","messageId":"20241021193135.GG1219228@coredump.intra.peff.net","threadId":"62349","inReplyTo":"20241021191934.GE1219228@coredump.intra.peff.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-10-21T19:31:35Z","receivedAt":"2024-10-21T19:31:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 21, 2024 at 03:19:34PM -0400, Jeff King wrote:\n\n> (This is all assuming that we are dealing with a uclibc with disabled\n> features in the first place. I don't think we've seen an answer to\n> Patrick's questions about why the #error is not triggering).\n\nAh, nevermind, I just saw his response from this morning about the stub\nheader. So I think we do know that this is a tweaked uclibc build.\n\n-Peff\n"},{"id":"505731","messageId":"ZxaudJWlJbEJ3LeO@nand.local","threadId":"62349","inReplyTo":"20241021193024.GF1219228@coredump.intra.peff.net","subject":"Re: clar unit testing framework FTBFS on uclibc systems (wchar_t unsupported)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-21T19:41:40Z","receivedAt":"2024-10-21T19:41:43Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 21, 2024 at 03:30:24PM -0400, Jeff King wrote:\n> On Mon, Oct 21, 2024 at 08:44:01AM +0200, Patrick Steinhardt wrote:\n>\n> > > Will it take us another 20 years to resolve all of the portability\n> > > issues which Clar suffers from (but git-compat-util.h doesn't)?\n> > > Probably not 20 years, but I don't think that it's on the complete other\n> > > end of the spectrum, either.\n> >\n> > My assumption is that we'll iron out the issues in this release. Our\n> > \"git-compat-util.h\" has grown _huge_, but that's mostly because it needs\n> > to support a ton of different things. The Git codebase is orders of\n> > magnitude bigger than the clar, so it is totally expected that it also\n> > exercises way more edge cases in C. Conversely, I expect that the compat\n> > headers in clar need to only be a fraction of what we have.\n> >\n> > I don't really understand where the claim comes from that this is such a\n> > huge pain. Sure, there's been a bit of back and forth now. But all of\n> > the reports I received were easy to fix, and I've fixed them upstream in\n> > a matter of days.\n> >\n> > I'd really like us to take a step back here and take things a bit more\n> > relaxed. If we see that this continues to be a major pain to maintain\n> > then yes, I agree, we should likely rope in our own compat headers. But\n> > from my point of view there isn't really indication that this is going\n> > to be the case.\n>\n> I'm OK with that direction. Just to be clear, I think you've done a\n> great job (as you always do) of responding to the issue promptly and\n> keeping things moving forward. And you're right that there is a good\n> chance that we iron out this wrinkle and never worry about it again. If\n> that doesn't turn out to be the case, we can iterate from there.\n\nYeah, to be clear on my own position, I agree with Peff here. I was\nmerely suggesting that there might be more work here than we estimate,\nand that it would be nice to take advantage of the experience and years\nof work that have gone into git-compat-util.h if that were the case.\n\nCertainly you have done a great job at responding promptly to any\nbreakages, which is greatly appreciated by myself and the project.\n\nThanks,\nTaylor\n"}]}