Let's have a look:
On 2025-09-05 23:37, Jeff King wrote:
[+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:
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.
I am open to improvements here to get in the compat prototypes...
The other half of your patch is the linking side:
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?
libgit.a does indeed contain the symbol:
root@deblndw002x:/var/tmp/ports/work/git-2.51.0.patched
# nm ./libgit.a | grep mkdtemp
[595] | 0| 0|FUNC |GLOB |0| UNDEF|gitmkdtemp
[204] | 0| 0|FUNC |GLOB |0| UNDEF|gitmkdtemp
[278] | 0| 0|FUNC |GLOB |0| UNDEF|gitmkdtemp
Symbols from ./libgit.a[mkdtemp.o]:
[1] | 0| 0|FILE |LOCAL|0| ABS|compat/mkdtemp.c
[110] | 0| 272|FUNC |GLOB |0| .text|gitmkdtemp
Let's try to revert the second half and see:
...and you are right. Since the include does properly replace the
missing symbol at compiliation time, linking now works expecte:
root@deblndw002x:/var/tmp/ports/work/git-2.51.0.patched
# nm t/unit-tests/bin/unit-tests | grep mkdtemp
[7332] | 0| 0|FILE |LOCAL|0| ABS|compat/mkdtemp.c
[16404] | 73286896| 272|FUNC |GLOB |0| .text|gitmkdtemp
We can drop one hunk from the patch, great!
Michael
