# [Change] Git build issue on NonStop 8 messages from 2025-09-18 to 2025-09-18. Participants: rsbecker@nexbridge.com, Jeff King, Patrick Steinhardt, Junio C Hamano. Thread: https://gitlist.dev/t/64164 ## rsbecker@nexbridge.com, 2025-09-18 02:16 Subject: [Change] Git build issue on NonStop Message-ID: <01c101dc2842$38903640$a9b0a2c0$@nexbridge.com> URL: https://gitlist.dev/e/01c101dc2842%2438903640%24a9b0a2c0%24%40nexbridge.com ``` Just a quick FYI. The addition of uintptr_t in clar tests has broken my CI build on NonStop x86. I will be fixing this locally. It may take a patch series unless a quick workaround is possible, which I am hoping. For those on the list from my platform who are monitoring, this looks like -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am unsure what else may be needed. --Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400) NonStop(211288444200000000) -- In real life, I talk too much. ``` ## Jeff King, 2025-09-18 02:29 Subject: Re: [Change] Git build issue on NonStop Message-ID: <20250918022912.GA1135133@coredump.intra.peff.net> URL: https://gitlist.dev/e/20250918022912.GA1135133%40coredump.intra.peff.net In-Reply-To: <01c101dc2842$38903640$a9b0a2c0$@nexbridge.com> ``` On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote: > Just a quick FYI. The addition of uintptr_t in clar tests has broken > my CI build on NonStop x86. I will be fixing this locally. It may take > a patch series unless a quick workaround is possible, which I am > hoping. > > For those on the list from my platform who are monitoring, this looks like > -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am unsure > what else may be needed. We use uintptr_t in lots of places in the regular code. I guess this bit in compat/posix.h is what makes it work: #ifdef NO_INTPTR_T /* * On I16LP32, ILP32 and LP64 "long" is the safe bet, however * on LLP86, IL33LLP64 and P64 it needs to be "long long", * while on IP16 and IP16L32 it is "int" (resp. "short") * Size needs to match (or exceed) 'sizeof(void *)'. * We can't take "long long" here as not everybody has it. */ typedef long intptr_t; typedef unsigned long uintptr_t; #endif But clar has its own compatibility layer. So it would need to do something similar. I see the clar line in question also uses PRIxPTR, which I can imagine might not be available everywhere either. We don't use that ourselves at all. I kind of wonder if just: diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index 80c5359425..f408af850f 100644 --- a/t/unit-tests/clar/clar.c +++ b/t/unit-tests/clar/clar.c @@ -875,8 +875,8 @@ void clar__assert_equal( void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); is_equal = (p1 == p2); if (!is_equal) - p_snprintf(buf, sizeof(buf), "0x%"PRIxPTR" != 0x%"PRIxPTR, - (uintptr_t)p1, (uintptr_t)p2); + p_snprintf(buf, sizeof(buf), "0x%"PRIuMAX" != 0x%"PRIuMAX, + (uintmax_t)p1, (uintmax_t)p2); } else { int i1 = va_arg(args, int), i2 = va_arg(args, int); would be sufficient. -Peff ``` ## rsbecker@nexbridge.com, 2025-09-18 03:20 Subject: RE: [Change] Git build issue on NonStop Message-ID: <01c601dc284b$24496400$6cdc2c00$@nexbridge.com> URL: https://gitlist.dev/e/01c601dc284b%2424496400%246cdc2c00%24%40nexbridge.com In-Reply-To: <20250918022912.GA1135133@coredump.intra.peff.net> ``` On September 17, 2025 10:29 PM, Jeff King wrote: >On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote: > >> Just a quick FYI. The addition of uintptr_t in clar tests has broken >> my CI build on NonStop x86. I will be fixing this locally. It may take >> a patch series unless a quick workaround is possible, which I am >> hoping. >> >> For those on the list from my platform who are monitoring, this looks >> like -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am >> unsure what else may be needed. > >We use uintptr_t in lots of places in the regular code. I guess this bit in >compat/posix.h is what makes it work: > > #ifdef NO_INTPTR_T > /* > * On I16LP32, ILP32 and LP64 "long" is the safe bet, however > * on LLP86, IL33LLP64 and P64 it needs to be "long long", > * while on IP16 and IP16L32 it is "int" (resp. "short") > * Size needs to match (or exceed) 'sizeof(void *)'. > * We can't take "long long" here as not everybody has it. > */ > typedef long intptr_t; > typedef unsigned long uintptr_t; > #endif > >But clar has its own compatibility layer. So it would need to do something similar. I >see the clar line in question also uses PRIxPTR, which I can imagine might not be >available everywhere either. We don't use that ourselves at all. > >I kind of wonder if just: > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index >80c5359425..f408af850f 100644 >--- a/t/unit-tests/clar/clar.c >+++ b/t/unit-tests/clar/clar.c >@@ -875,8 +875,8 @@ void clar__assert_equal( > void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); > is_equal = (p1 == p2); > if (!is_equal) >- p_snprintf(buf, sizeof(buf), "0x%"PRIxPTR" != >0x%"PRIxPTR, >- (uintptr_t)p1, (uintptr_t)p2); >+ p_snprintf(buf, sizeof(buf), "0x%"PRIuMAX" != >0x%"PRIuMAX, >+ (uintmax_t)p1, (uintmax_t)p2); > } > else { > int i1 = va_arg(args, int), i2 = va_arg(args, int); > >would be sufficient. Yes, it would work. uintmax_t is part of the standard set while uintptr_t is considered an extension. Not my decision on this grouping. I'm setting the -D in CFLAGS to see if that works, I would be fine going that way, although better would be adding it into config.uname.mak in the NONSTOP section. ``` ## Patrick Steinhardt, 2025-09-18 05:37 Subject: Re: [Change] Git build issue on NonStop Message-ID: URL: https://gitlist.dev/e/aMuankGhjxXNKErO%40pks.im In-Reply-To: <01c601dc284b$24496400$6cdc2c00$@nexbridge.com> ``` On Wed, Sep 17, 2025 at 11:20:05PM -0400, rsbecker@nexbridge.com wrote: > On September 17, 2025 10:29 PM, Jeff King wrote: > >On Wed, Sep 17, 2025 at 10:16:13PM -0400, rsbecker@nexbridge.com wrote: > > > >> Just a quick FYI. The addition of uintptr_t in clar tests has broken > >> my CI build on NonStop x86. I will be fixing this locally. It may take > >> a patch series unless a quick workaround is possible, which I am > >> hoping. > >> > >> For those on the list from my platform who are monitoring, this looks > >> like -D__NSK_OPTIONAL_TYPES__ is now required for the build. I am > >> unsure what else may be needed. > > > >We use uintptr_t in lots of places in the regular code. I guess this bit in > >compat/posix.h is what makes it work: > > > > #ifdef NO_INTPTR_T > > /* > > * On I16LP32, ILP32 and LP64 "long" is the safe bet, however > > * on LLP86, IL33LLP64 and P64 it needs to be "long long", > > * while on IP16 and IP16L32 it is "int" (resp. "short") > > * Size needs to match (or exceed) 'sizeof(void *)'. > > * We can't take "long long" here as not everybody has it. > > */ > > typedef long intptr_t; > > typedef unsigned long uintptr_t; > > #endif > > > >But clar has its own compatibility layer. So it would need to do something similar. I > >see the clar line in question also uses PRIxPTR, which I can imagine might not be > >available everywhere either. We don't use that ourselves at all. > > > >I kind of wonder if just: > > > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index > >80c5359425..f408af850f 100644 > >--- a/t/unit-tests/clar/clar.c > >+++ b/t/unit-tests/clar/clar.c > >@@ -875,8 +875,8 @@ void clar__assert_equal( > > void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); > > is_equal = (p1 == p2); > > if (!is_equal) > >- p_snprintf(buf, sizeof(buf), "0x%"PRIxPTR" != > >0x%"PRIxPTR, > >- (uintptr_t)p1, (uintptr_t)p2); > >+ p_snprintf(buf, sizeof(buf), "0x%"PRIuMAX" != > >0x%"PRIuMAX, > >+ (uintmax_t)p1, (uintmax_t)p2); > > } > > else { > > int i1 = va_arg(args, int), i2 = va_arg(args, int); > > > >would be sufficient. > > Yes, it would work. uintmax_t is part of the standard set while uintptr_t is > considered an extension. Not my decision on this grouping. I'm setting > the -D in CFLAGS to see if that works, I would be fine going that way, > although better would be adding it into config.uname.mak in the NONSTOP > section. That should work alright, yeah. Peff, do you want to create a PR in https://github.com/clar-test/clar to fix this? Otherwise I can handle this. Thanks, both! Patrick ``` ## Jeff King, 2025-09-18 06:31 Subject: Re: [Change] Git build issue on NonStop Message-ID: <20250918063152.GA1168297@coredump.intra.peff.net> URL: https://gitlist.dev/e/20250918063152.GA1168297%40coredump.intra.peff.net In-Reply-To: ``` On Thu, Sep 18, 2025 at 07:37:34AM +0200, Patrick Steinhardt wrote: > > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index > > >80c5359425..f408af850f 100644 > > >--- a/t/unit-tests/clar/clar.c > > >+++ b/t/unit-tests/clar/clar.c > > >@@ -875,8 +875,8 @@ void clar__assert_equal( > > > void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); > > > is_equal = (p1 == p2); > > > if (!is_equal) > > >- p_snprintf(buf, sizeof(buf), "0x%"PRIxPTR" != > > >0x%"PRIxPTR, > > >- (uintptr_t)p1, (uintptr_t)p2); > > >+ p_snprintf(buf, sizeof(buf), "0x%"PRIuMAX" != > > >0x%"PRIuMAX, > > >+ (uintmax_t)p1, (uintmax_t)p2); > > > } > > > else { > > > int i1 = va_arg(args, int), i2 = va_arg(args, int); > > > > > >would be sufficient. > > > > Yes, it would work. uintmax_t is part of the standard set while uintptr_t is > > considered an extension. Not my decision on this grouping. I'm setting > > the -D in CFLAGS to see if that works, I would be fine going that way, > > although better would be adding it into config.uname.mak in the NONSTOP > > section. > > That should work alright, yeah. Peff, do you want to create a PR in > https://github.com/clar-test/clar to fix this? Otherwise I can handle > this. I'd be happy if you take it from here. Note in what I posted above it should probably be PRIxMAX to show hex (not "u"). -Peff ``` ## Patrick Steinhardt, 2025-09-18 13:00 Subject: Re: [Change] Git build issue on NonStop Message-ID: URL: https://gitlist.dev/e/aMwCcw-6l1x85Wz-%40pks.im In-Reply-To: <20250918063152.GA1168297@coredump.intra.peff.net> ``` On Thu, Sep 18, 2025 at 02:31:52AM -0400, Jeff King wrote: > On Thu, Sep 18, 2025 at 07:37:34AM +0200, Patrick Steinhardt wrote: > > > > >diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index > > > >80c5359425..f408af850f 100644 > > > >--- a/t/unit-tests/clar/clar.c > > > >+++ b/t/unit-tests/clar/clar.c > > > >@@ -875,8 +875,8 @@ void clar__assert_equal( > > > > void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); > > > > is_equal = (p1 == p2); > > > > if (!is_equal) > > > >- p_snprintf(buf, sizeof(buf), "0x%"PRIxPTR" != > > > >0x%"PRIxPTR, > > > >- (uintptr_t)p1, (uintptr_t)p2); > > > >+ p_snprintf(buf, sizeof(buf), "0x%"PRIuMAX" != > > > >0x%"PRIuMAX, > > > >+ (uintmax_t)p1, (uintmax_t)p2); > > > > } > > > > else { > > > > int i1 = va_arg(args, int), i2 = va_arg(args, int); > > > > > > > >would be sufficient. > > > > > > Yes, it would work. uintmax_t is part of the standard set while uintptr_t is > > > considered an extension. Not my decision on this grouping. I'm setting > > > the -D in CFLAGS to see if that works, I would be fine going that way, > > > although better would be adding it into config.uname.mak in the NONSTOP > > > section. > > > > That should work alright, yeah. Peff, do you want to create a PR in > > https://github.com/clar-test/clar to fix this? Otherwise I can handle > > this. > > I'd be happy if you take it from here. Note in what I posted above it > should probably be PRIxMAX to show hex (not "u"). One thing I missed: `uintmax_t` doesn't work on 32 bit systems: ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] 879 | (uintmax_t)p1, (uintmax_t)p2); | ^ I'm inclined to just use "%p" instead and accept that this has platform-dependent behaviour. Means we'll have to drop the test for this, but that's the lesser evil from my point of view. Patrick ``` ## Junio C Hamano, 2025-09-18 14:47 Subject: Re: [Change] Git build issue on NonStop Message-ID: URL: https://gitlist.dev/e/xmqqbjn7akca.fsf%40gitster.g In-Reply-To: ``` Patrick Steinhardt writes: > One thing I missed: `uintmax_t` doesn't work on 32 bit systems: > > ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] > 879 | (uintmax_t)p1, (uintmax_t)p2); > | ^ > > I'm inclined to just use "%p" instead and accept that this has > platform-dependent behaviour. Means we'll have to drop the test for > this, but that's the lesser evil from my point of view. As long as %p works everywhere and with stable output, that is the most appropriate solution, I would think. After all, this is used only for "oops, the test expects these two pointers are pointing at the same address, but they differ; they point at these places...". To test such a test, wouldn't it be sufficient to perform "does it give a bit of output or not?" check in isolation? ``` ## rsbecker@nexbridge.com, 2025-09-18 15:20 Subject: RE: [Change] Git build issue on NonStop Message-ID: <020001dc28af$bfec1420$3fc43c60$@nexbridge.com> URL: https://gitlist.dev/e/020001dc28af%24bfec1420%243fc43c60%24%40nexbridge.com In-Reply-To: ``` On September 18, 2025 10:48 AM, Junio C Hamano wrote: >Patrick Steinhardt writes: > >> One thing I missed: `uintmax_t` doesn't work on 32 bit systems: >> >> ::error file=clar.c,line=879::clar.c:879:8: cast from pointer to integer of different >size [-Werror=pointer-to-int-cast] >> 879 | (uintmax_t)p1, (uintmax_t)p2); >> | ^ >> >> I'm inclined to just use "%p" instead and accept that this has >> platform-dependent behaviour. Means we'll have to drop the test for >> this, but that's the lesser evil from my point of view. > >As long as %p works everywhere and with stable output, that is the most >appropriate solution, I would think. After all, this is used only for "oops, the test >expects these two pointers are pointing at the same address, but they differ; they >point at these places...". > >To test such a test, wouldn't it be sufficient to perform "does it give a bit of output >or not?" check in isolation? Surprisingly, this actually works on NonStop x86 in both memory models despite warnings to the contrary in their man page.. Thanks :) ```