From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 18 Sep 2025 05:37:34 GMT
Subject: Re: [Change] Git build issue on NonStop
Message-ID: <aMuankGhjxXNKErO@pks.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