{"thread":{"id":"64164","subject":"[Change] Git build issue on NonStop","startedAt":"2025-09-18T02:16:25Z","lastAt":"2025-09-18T15:20:27Z","messageCount":8,"participants":["rsbecker@nexbridge.com","Jeff King","Patrick Steinhardt","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"526650","messageId":"01c101dc2842$38903640$a9b0a2c0$@nexbridge.com","threadId":"64164","inReplyTo":null,"subject":"[Change] Git build issue on NonStop","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2025-09-18T02:16:13Z","receivedAt":"2025-09-18T02:16:25Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"Just a quick FYI. The addition of uintptr_t in clar tests has broken my CI\nbuild\non NonStop x86. I will be fixing this locally. It may take a patch series\nunless\na quick workaround is possible, which I am hoping.\n\nFor those on the list from my platform who are monitoring, this looks like\n-D__NSK_OPTIONAL_TYPES__ is now required for the build. I am unsure\nwhat else may be needed.\n\n--Randall\n\n--\nBrief whoami: NonStop&UNIX developer since approximately\nUNIX(421664400)\nNonStop(211288444200000000)\n-- In real life, I talk too much.\n\n\n"},{"id":"526651","messageId":"20250918022912.GA1135133@coredump.intra.peff.net","threadId":"64164","inReplyTo":"01c101dc2842$38903640$a9b0a2c0$@nexbridge.com","subject":"Re: [Change] Git build issue on NonStop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-09-18T02:29:12Z","receivedAt":"2025-09-18T02:29:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote:\n\n> Just a quick FYI. The addition of uintptr_t in clar tests has broken\n> my CI build on NonStop x86. I will be fixing this locally. It may take\n> a patch series unless a quick workaround is possible, which I am\n> hoping.\n> \n> For those on the list from my platform who are monitoring, this looks like\n> -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am unsure\n> what else may be needed.\n\nWe use uintptr_t in lots of places in the regular code. I guess this bit\nin compat/posix.h is what makes it work:\n\n  #ifdef NO_INTPTR_T\n  /*\n   * On I16LP32, ILP32 and LP64 \"long\" is the safe bet, however\n   * on LLP86, IL33LLP64 and P64 it needs to be \"long long\",\n   * while on IP16 and IP16L32 it is \"int\" (resp. \"short\")\n   * Size needs to match (or exceed) 'sizeof(void *)'.\n   * We can't take \"long long\" here as not everybody has it.\n   */\n  typedef long intptr_t;\n  typedef unsigned long uintptr_t;\n  #endif\n\nBut clar has its own compatibility layer. So it would need to do\nsomething similar. I see the clar line in question also uses PRIxPTR,\nwhich I can imagine might not be available everywhere either. We don't\nuse that ourselves at all.\n\nI kind of wonder if just:\n\ndiff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c\nindex 80c5359425..f408af850f 100644\n--- a/t/unit-tests/clar/clar.c\n+++ b/t/unit-tests/clar/clar.c\n@@ -875,8 +875,8 @@ void clar__assert_equal(\n \t\tvoid *p1 = va_arg(args, void *), *p2 = va_arg(args, void *);\n \t\tis_equal = (p1 == p2);\n \t\tif (!is_equal)\n-\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIxPTR\" != 0x%\"PRIxPTR,\n-\t\t\t\t   (uintptr_t)p1, (uintptr_t)p2);\n+\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIuMAX\" != 0x%\"PRIuMAX,\n+\t\t\t\t   (uintmax_t)p1, (uintmax_t)p2);\n \t}\n \telse {\n \t\tint i1 = va_arg(args, int), i2 = va_arg(args, int);\n\nwould be sufficient.\n\n-Peff\n"},{"id":"526652","messageId":"01c601dc284b$24496400$6cdc2c00$@nexbridge.com","threadId":"64164","inReplyTo":"20250918022912.GA1135133@coredump.intra.peff.net","subject":"RE: [Change] Git build issue on NonStop","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2025-09-18T03:20:05Z","receivedAt":"2025-09-18T03:20:22Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 17, 2025 10:29 PM, Jeff King wrote:\n>On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote:\n>\n>> Just a quick FYI. The addition of uintptr_t in clar tests has broken\n>> my CI build on NonStop x86. I will be fixing this locally. It may take\n>> a patch series unless a quick workaround is possible, which I am\n>> hoping.\n>>\n>> For those on the list from my platform who are monitoring, this looks\n>> like -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am\n>> unsure what else may be needed.\n>\n>We use uintptr_t in lots of places in the regular code. I guess this bit in\n>compat/posix.h is what makes it work:\n>\n>  #ifdef NO_INTPTR_T\n>  /*\n>   * On I16LP32, ILP32 and LP64 \"long\" is the safe bet, however\n>   * on LLP86, IL33LLP64 and P64 it needs to be \"long long\",\n>   * while on IP16 and IP16L32 it is \"int\" (resp. \"short\")\n>   * Size needs to match (or exceed) 'sizeof(void *)'.\n>   * We can't take \"long long\" here as not everybody has it.\n>   */\n>  typedef long intptr_t;\n>  typedef unsigned long uintptr_t;\n>  #endif\n>\n>But clar has its own compatibility layer. So it would need to do something similar. I\n>see the clar line in question also uses PRIxPTR, which I can imagine might not be\n>available everywhere either. We don't use that ourselves at all.\n>\n>I kind of wonder if just:\n>\n>diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index\n>80c5359425..f408af850f 100644\n>--- a/t/unit-tests/clar/clar.c\n>+++ b/t/unit-tests/clar/clar.c\n>@@ -875,8 +875,8 @@ void clar__assert_equal(\n> \t\tvoid *p1 = va_arg(args, void *), *p2 = va_arg(args, void *);\n> \t\tis_equal = (p1 == p2);\n> \t\tif (!is_equal)\n>-\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIxPTR\" !=\n>0x%\"PRIxPTR,\n>-\t\t\t\t   (uintptr_t)p1, (uintptr_t)p2);\n>+\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIuMAX\" !=\n>0x%\"PRIuMAX,\n>+\t\t\t\t   (uintmax_t)p1, (uintmax_t)p2);\n> \t}\n> \telse {\n> \t\tint i1 = va_arg(args, int), i2 = va_arg(args, int);\n>\n>would be sufficient.\n\nYes, it would work. uintmax_t is part of the standard set while uintptr_t is\nconsidered an extension. Not my decision on this grouping. I'm setting\nthe -D in CFLAGS to see if that works, I would be fine going that way, \nalthough better would be adding it into config.uname.mak in the NONSTOP\nsection.\n\n"},{"id":"526655","messageId":"aMuankGhjxXNKErO@pks.im","threadId":"64164","inReplyTo":"01c601dc284b$24496400$6cdc2c00$@nexbridge.com","subject":"Re: [Change] Git build issue on NonStop","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-18T05:37:34Z","receivedAt":"2025-09-18T05:37:41Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Sep 17, 2025 at 11:20:05PM -0400, rsbecker@nexbridge.com wrote:\n> On September 17, 2025 10:29 PM, Jeff King wrote:\n> >On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote:\n> >\n> >> Just a quick FYI. The addition of uintptr_t in clar tests has broken\n> >> my CI build on NonStop x86. I will be fixing this locally. It may take\n> >> a patch series unless a quick workaround is possible, which I am\n> >> hoping.\n> >>\n> >> For those on the list from my platform who are monitoring, this looks\n> >> like -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am\n> >> unsure what else may be needed.\n> >\n> >We use uintptr_t in lots of places in the regular code. I guess this bit in\n> >compat/posix.h is what makes it work:\n> >\n> >  #ifdef NO_INTPTR_T\n> >  /*\n> >   * On I16LP32, ILP32 and LP64 \"long\" is the safe bet, however\n> >   * on LLP86, IL33LLP64 and P64 it needs to be \"long long\",\n> >   * while on IP16 and IP16L32 it is \"int\" (resp. \"short\")\n> >   * Size needs to match (or exceed) 'sizeof(void *)'.\n> >   * We can't take \"long long\" here as not everybody has it.\n> >   */\n> >  typedef long intptr_t;\n> >  typedef unsigned long uintptr_t;\n> >  #endif\n> >\n> >But clar has its own compatibility layer. So it would need to do something similar. I\n> >see the clar line in question also uses PRIxPTR, which I can imagine might not be\n> >available everywhere either. We don't use that ourselves at all.\n> >\n> >I kind of wonder if just:\n> >\n> >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index\n> >80c5359425..f408af850f 100644\n> >--- a/t/unit-tests/clar/clar.c\n> >+++ b/t/unit-tests/clar/clar.c\n> >@@ -875,8 +875,8 @@ void clar__assert_equal(\n> > \t\tvoid *p1 = va_arg(args, void *), *p2 = va_arg(args, void *);\n> > \t\tis_equal = (p1 == p2);\n> > \t\tif (!is_equal)\n> >-\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIxPTR\" !=\n> >0x%\"PRIxPTR,\n> >-\t\t\t\t   (uintptr_t)p1, (uintptr_t)p2);\n> >+\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIuMAX\" !=\n> >0x%\"PRIuMAX,\n> >+\t\t\t\t   (uintmax_t)p1, (uintmax_t)p2);\n> > \t}\n> > \telse {\n> > \t\tint i1 = va_arg(args, int), i2 = va_arg(args, int);\n> >\n> >would be sufficient.\n> \n> Yes, it would work. uintmax_t is part of the standard set while uintptr_t is\n> considered an extension. Not my decision on this grouping. I'm setting\n> the -D in CFLAGS to see if that works, I would be fine going that way, \n> although better would be adding it into config.uname.mak in the NONSTOP\n> section.\n\nThat should work alright, yeah. Peff, do you want to create a PR in\nhttps://github.com/clar-test/clar to fix this? Otherwise I can handle\nthis.\n\nThanks, both!\n\nPatrick\n"},{"id":"526676","messageId":"20250918063152.GA1168297@coredump.intra.peff.net","threadId":"64164","inReplyTo":"aMuankGhjxXNKErO@pks.im","subject":"Re: [Change] Git build issue on NonStop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-09-18T06:31:52Z","receivedAt":"2025-09-18T06:31:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 18, 2025 at 07:37:34AM +0200, Patrick Steinhardt wrote:\n\n> > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index\n> > >80c5359425..f408af850f 100644\n> > >--- a/t/unit-tests/clar/clar.c\n> > >+++ b/t/unit-tests/clar/clar.c\n> > >@@ -875,8 +875,8 @@ void clar__assert_equal(\n> > > \t\tvoid *p1 = va_arg(args, void *), *p2 = va_arg(args, void *);\n> > > \t\tis_equal = (p1 == p2);\n> > > \t\tif (!is_equal)\n> > >-\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIxPTR\" !=\n> > >0x%\"PRIxPTR,\n> > >-\t\t\t\t   (uintptr_t)p1, (uintptr_t)p2);\n> > >+\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIuMAX\" !=\n> > >0x%\"PRIuMAX,\n> > >+\t\t\t\t   (uintmax_t)p1, (uintmax_t)p2);\n> > > \t}\n> > > \telse {\n> > > \t\tint i1 = va_arg(args, int), i2 = va_arg(args, int);\n> > >\n> > >would be sufficient.\n> > \n> > Yes, it would work. uintmax_t is part of the standard set while uintptr_t is\n> > considered an extension. Not my decision on this grouping. I'm setting\n> > the -D in CFLAGS to see if that works, I would be fine going that way, \n> > although better would be adding it into config.uname.mak in the NONSTOP\n> > section.\n> \n> That should work alright, yeah. Peff, do you want to create a PR in\n> https://github.com/clar-test/clar to fix this? Otherwise I can handle\n> this.\n\nI'd be happy if you take it from here. Note in what I posted above it\nshould probably be PRIxMAX to show hex (not \"u\").\n\n-Peff\n"},{"id":"526699","messageId":"aMwCcw-6l1x85Wz-@pks.im","threadId":"64164","inReplyTo":"20250918063152.GA1168297@coredump.intra.peff.net","subject":"Re: [Change] Git build issue on NonStop","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-09-18T13:00:35Z","receivedAt":"2025-09-18T13:00:47Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Sep 18, 2025 at 02:31:52AM -0400, Jeff King wrote:\n> On Thu, Sep 18, 2025 at 07:37:34AM +0200, Patrick Steinhardt wrote:\n> \n> > > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index\n> > > >80c5359425..f408af850f 100644\n> > > >--- a/t/unit-tests/clar/clar.c\n> > > >+++ b/t/unit-tests/clar/clar.c\n> > > >@@ -875,8 +875,8 @@ void clar__assert_equal(\n> > > > \t\tvoid *p1 = va_arg(args, void *), *p2 = va_arg(args, void *);\n> > > > \t\tis_equal = (p1 == p2);\n> > > > \t\tif (!is_equal)\n> > > >-\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIxPTR\" !=\n> > > >0x%\"PRIxPTR,\n> > > >-\t\t\t\t   (uintptr_t)p1, (uintptr_t)p2);\n> > > >+\t\t\tp_snprintf(buf, sizeof(buf), \"0x%\"PRIuMAX\" !=\n> > > >0x%\"PRIuMAX,\n> > > >+\t\t\t\t   (uintmax_t)p1, (uintmax_t)p2);\n> > > > \t}\n> > > > \telse {\n> > > > \t\tint i1 = va_arg(args, int), i2 = va_arg(args, int);\n> > > >\n> > > >would be sufficient.\n> > > \n> > > Yes, it would work. uintmax_t is part of the standard set while uintptr_t is\n> > > considered an extension. Not my decision on this grouping. I'm setting\n> > > the -D in CFLAGS to see if that works, I would be fine going that way, \n> > > although better would be adding it into config.uname.mak in the NONSTOP\n> > > section.\n> > \n> > That should work alright, yeah. Peff, do you want to create a PR in\n> > https://github.com/clar-test/clar to fix this? Otherwise I can handle\n> > this.\n> \n> I'd be happy if you take it from here. Note in what I posted above it\n> should probably be PRIxMAX to show hex (not \"u\").\n\nOne thing I missed: `uintmax_t` doesn't work on 32 bit systems:\n\n    ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]\n      879 |        (uintmax_t)p1, (uintmax_t)p2);\n          |        ^\n\nI'm inclined to just use \"%p\" instead and accept that this has\nplatform-dependent behaviour. Means we'll have to drop the test for\nthis, but that's the lesser evil from my point of view.\n\nPatrick\n"},{"id":"526701","messageId":"xmqqbjn7akca.fsf@gitster.g","threadId":"64164","inReplyTo":"aMwCcw-6l1x85Wz-@pks.im","subject":"Re: [Change] Git build issue on NonStop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-18T14:47:33Z","receivedAt":"2025-09-18T14:47:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> One thing I missed: `uintmax_t` doesn't work on 32 bit systems:\n>\n>     ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]\n>       879 |        (uintmax_t)p1, (uintmax_t)p2);\n>           |        ^\n>\n> I'm inclined to just use \"%p\" instead and accept that this has\n> platform-dependent behaviour. Means we'll have to drop the test for\n> this, but that's the lesser evil from my point of view.\n\nAs long as %p works everywhere and with stable output, that is the\nmost appropriate solution, I would think.  After all, this is used\nonly for \"oops, the test expects these two pointers are pointing at\nthe same address, but they differ; they point at these places...\".\n\nTo test such a test, wouldn't it be sufficient to perform \"does it\ngive a bit of output or not?\" check in isolation?\n\n"},{"id":"526706","messageId":"020001dc28af$bfec1420$3fc43c60$@nexbridge.com","threadId":"64164","inReplyTo":"xmqqbjn7akca.fsf@gitster.g","subject":"RE: [Change] Git build issue on NonStop","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2025-09-18T15:20:15Z","receivedAt":"2025-09-18T15:20:27Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 18, 2025 10:48 AM, Junio C Hamano wrote:\n>Patrick Steinhardt <ps@pks.im> writes:\n>\n>> One thing I missed: `uintmax_t` doesn't work on 32 bit systems:\n>>\n>>     ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to\ninteger of different\n>size [-Werror=pointer-to-int-cast]\n>>       879 |        (uintmax_t)p1, (uintmax_t)p2);\n>>           |        ^\n>>\n>> I'm inclined to just use \"%p\" instead and accept that this has\n>> platform-dependent behaviour. Means we'll have to drop the test for\n>> this, but that's the lesser evil from my point of view.\n>\n>As long as %p works everywhere and with stable output, that is the most\n>appropriate solution, I would think.  After all, this is used only for\n\"oops, the test\n>expects these two pointers are pointing at the same address, but they\ndiffer; they\n>point at these places...\".\n>\n>To test such a test, wouldn't it be sufficient to perform \"does it give a\nbit of output\n>or not?\" check in isolation?\n\nSurprisingly, this actually works on NonStop x86 in both memory models\ndespite\nwarnings to the contrary in their man page.. Thanks :)\n\n"}]}