Re: [Bug] Compat objects not added to CLAR_TEST_PROG

From: Jeff King <peff@peff.net>
Date: Sep 5, 2025, 21:37 UTC
Message-ID: <20250905213708.GB612697@coredump.intra.peff.net>
In-Reply-To: <d8d4266e-838c-488e-9aaf-4a1be0169795@innomotics.com>

[+cc pks for clar portability]

On Fri, Sep 05, 2025 at 03:19:50PM +0200, Osipov, Michael (IN IT IN) wrote:

> I am building Git 2.51.0 on HP-UX 11.31, previous releases went smoothly.

Neat, today I learned HP-UX is still alive and kicking. :)

Half of your patch makes sense to me, but I'm puzzled by the other half. This:

Show 12 quoted lines

> diff -u -ur t/unit-tests/clar/clar/sandbox.h git-2.51.0.patched/t/unit-tests/clar/clar/sandbox.h
> --- t/unit-tests/clar/clar/sandbox.h	2025-08-18 02:35:38 +0200
> +++ t/unit-tests/clar/clar/sandbox.h	2025-09-05 14:10:52 +0200
> @@ -2,6 +2,8 @@
>  #include <sys/syslimits.h>
>  #endif
>  
> +#include "../../../../compat/posix.h"
> +
>  static char _clar_path[4096 + 1];
>  
>  static int

...seems like an obvious improvement. If we are compiling any C code, we'd want our compatibility macros, etc. Although it does get a little funny, as the contents of clar/ are imported from elsewhere, and now we're modifying that.

It looks like clar tries to handle portability on its own, so I guess another route is for it to add its own mkdtemp wrapper, and we'd import that fixed version. But it really feels like we're duplicating effort.

The other half of your patch is the linking side:

Show 12 quoted lines

> diff -u -ur Makefile Makefile
> --- Makefile	2025-08-18 02:35:38 +0200
> +++ Makefile	2025-09-05 14:34:43 +0200
> @@ -3933,7 +3933,7 @@
>  $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
>  $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
>  $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
> -$(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-LDFLAGS
> +$(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(COMPAT_OBJS) $(GITLIBS) GIT-LDFLAGS
>  	$(call mkdir_p_parent_template)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>

but I'm not sure that should be necessary. The compat objects are included in libgit.a, and we should be linking against that when we build the unit-test executable. At any rate, building with NO_MKDTEMP=1 for me on Linux does successfully find gitmkdtemp(). Are you sure this half of the patch was needed?

-Peff