On Sat, Oct 12, 2024 at 4:20 AM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> On Fri, Oct 11, 2024 at 10:10:26PM -0400, Alejandro R. Sedeño wrote:
> > First, clar.suite was generated as broken because clar-decls.h was
> > generated as empty. Tweaking the sed one-liner in Makefile that is
> > used to generate clar-decls.h fixed that (move the end-of-line marker
> > outside of the capture group, `$$\)` -> `\)$$`), which I would submit
> > as a patch, but (a) that only fixed part of the problem and (b) I'm
> > not entirely sure why it helped. If someone else wants to apply this
> > change, which would align the end-of-line marker placement with the
> > start-of-line marker placement, have at it.
>
> I'd still appreciate it if you could show me the diff. From thereon I
> can handle the rest.
diff --git a/Makefile b/Makefile
index 2dde1fd2b8..87c1f9e220 100644
--- a/Makefile
+++ b/Makefile
@@ -3906,7 +3906,7 @@ GIT-TEST-SUITES: FORCE
$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst
%,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \
- sed -ne "s/^\(void
test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\)/extern \1;/p"
$(UNIT_TEST_DIR)/$$suite.c; \
+ sed -ne "s/^\(void
test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$$/extern \1;/p"
$(UNIT_TEST_DIR)/$$suite.c; \
done >$@
$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $<
>$(UNIT_TEST_DIR)/clar.suite
Or feel free to grab the entire commit from here:
https://asedeno.scripts.mit.edu/gitweb/?p=git.git;a=shortlog;h=refs/heads/clar_sed_tweak
> > The next issue was that clar/sandbox.h uses mkdtemp, which I don't
> > have here. Git has solved this in compat/mkdtemp.c via
> > git-compat-util.h, but clar is not using it. Adding git-compat-util.h
> > to clar/sandbox.h feels weird, but does get us further along. That
> > change introduced banned.h into clar, which exposed the use of strncpy
> > and localtime, both otherwise banned in git.
>
> Yeah, we don't want to pull in that header. The clar is from upstream,
> so ideally we shouldn't have to modify it with non-upstreamable bits.
>
> In any case, I've got a similar report yesterday where some functions
> weren't available. The root cause is that we don't set `_POSIX_C_SOURCE`
> in "clar.c", so with the below patch things started to work. Does that
> patch work for you, too? At least I think it should, as [1] mentions
> that the function is available on SunOS when those defines exist.
>
> In any case, the patch has already been merged upstream [2], and I'll
> send a patch early next week that updates our bundled version of clar.
>
> [1]: https://www.unix.com/man-page/sunos/3/MKDTEMP/
> [2]: https://github.com/clar-test/clar/pull/106
The listed man page is from the Linux Programmer's Manual, regardless of
the url path. It won't be enough here as mkdtemp is nowhere to be found
in /usr/include or any other /usr/**/include.
For what it's worth, the compat objects are being linked in, so perhaps a
smaller compat shim for clar that brings in definitions for mkdtemp, mkdir,
and whatever else might be handy without the weight of git-compat-util.h
would be a reasonable compromise. Maybe not. I don't know.
> [snip]
> > Please note that this should not be read as opposition to the new
> > unit-testing framework in any way. Building git (and curl, and gmake,
> > and zlib, and openssl, and perl, all for git) for SunOS was a hobby
> > for me, and not anything I personally need, and besides, it's not like
> > my previous builds have disappeared.
>
> Sure. But if the fix is easy enough I don't see a reason why we
> shouldn't try to support your platform. It would be great to get earlier
> feedback such that we can fix issues like this before we create the
> release (see also our Documentation/technical/platform-support.txt,
> which we have released recently.). But I'll take what I can get, so
> thanks a lot for sending the report in the first place!
I appreciate that. Builds here are very slow, which is why I usually only
build releases. The effort that went into this report would have taken
maybe an hour or two on any other platform I use.
Oh, and for the sake of future readers of the thread, git v2.46.2 did build.
Thanks,
-Alejandro
