git no longer builds on SunOS 5.10, a report

git no longer builds on SunOS 5.10, a report

[Alejandro R. Sedeño]

Hi all,

I've spent the entire day trying to fix the build for SunOS 5.10, as
I've done a few times over the years out of sheer stubbornness, but
this time I'm throwing in the towel. I figured I would at least relay
what I found though, in case anyone else came looking.

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.

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.

Including git-compat-util.h in clar/sandbox.h (bringing in mkdtemp,
and replacing strncpy with strlcpy) and clar/summary.h (replacing
localtime with localtime_r) leads to our next issue: a redefinition of
_FILE_OFFSET_BITS, which is defined unconditionally in
git-compat-util.h, because clar.c imports system headers that define
it first. git-compat-util.h is meant to be included first, so, I added
git-compat-util.h to the top of clar.c. That caused system includes
via <wchar.h> to no longer compile due to syntax errors. This is where
I gave up.

I'm sad that I can no longer build git on this old version of SunOS,
and that it's the newly-imported unit-testing framework and not git
itself that is preventing me from building it. Given the talk of
adding rust to git, and of bumping the perl requirements to 5.26.0 (I
have a system 5.8.3, and a 5.10.1 for building openssl), this moment
was inevitable. On the plus side, this was the slowest platform I
built git on, so perhaps I should be happy about my new free time.

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.

The last successful build for me was 2.45.2. I've built or tried to
build most versions since 1.6.6. Some of my build infrastructure was
unavailable since sometime after 2.45.2, so I have not tried 2.46.x,
but it lacks clar, so I expect it would build fine.

Cheers,
-Alejandro

Re: git no longer builds on SunOS 5.10, a report

[Patrick Steinhardt]

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.

> 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

[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!

Patrick

diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
index cef0f023c24..76557df3040 100644
--- a/t/unit-tests/clar/clar.c
+++ b/t/unit-tests/clar/clar.c
@@ -4,6 +4,10 @@
  * This file is part of clar, distributed under the ISC license.
  * For full terms see the included COPYING file.
  */
+
+#define _DARWIN_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+
 #include <assert.h>
 #include <setjmp.h>
 #include <stdlib.h>
@@ -271,9 +275,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
 
 static void clar_time_now(clar_time *out)
 {
-	struct timezone tz;
-
-	gettimeofday(out, &tz);
+	gettimeofday(out, NULL);
 }
 
 static double clar_time_diff(clar_time *start, clar_time *end)

Re: git no longer builds on SunOS 5.10, a report

[Alejandro R. Sedeño]

On Sat, Oct 12, 2024 at 4:20 AM Patrick Steinhardt <ps@pks.im> 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

[PATCH] Makefile: adjust sed command for generating "clar-decls.h"

[Alejandro R. Sedeño]

This moves the end-of-line marker out of the captured group, matching
the start-of-line marker and for some reason fixing generation of
"clar-decls.h" on some older, more esoteric platforms.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Alejandro R. Sedeño <asedeno@google.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
-- 
2.39.5

Re: git no longer builds on SunOS 5.10, a report

[Alejandro R. Sedeño]

On Sat, Oct 12, 2024 at 10:34 AM Alejandro R. Sedeño <asedeno@mit.edu>
wrote and gmail mangled:
> diff --git a/Makefile b/Makefile
> index 2dde1fd2b8..87c1f9e220 100644
> --- a/Makefile
> +++ b/Makefile
<snip>

resent as a whole patch via git send-email, since that was just sad.

-Alejandro

Re: git no longer builds on SunOS 5.10, a report

[Patrick Steinhardt]

On Sat, Oct 12, 2024 at 10:34:18AM -0400, Alejandro R. Sedeño wrote:
> On Sat, Oct 12, 2024 at 4:20 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Fri, Oct 11, 2024 at 10:10:26PM -0400, Alejandro R. Sedeño wrote:
> 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

Thanks!

> > > 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.

Okay. I assume that both mktemp and mkdir are available though, right?
If so, does the below patch work? The last bit is new, where we now use
the same mkdtemp implementation as we use on NonStop in clar.

Patrick

diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
index cef0f023c2..064ca5c2ea 100644
--- a/t/unit-tests/clar/clar.c
+++ b/t/unit-tests/clar/clar.c
@@ -4,6 +4,10 @@
  * This file is part of clar, distributed under the ISC license.
  * For full terms see the included COPYING file.
  */
+
+#define _DARWIN_C_SOURCE
+#define _POSIX_C_SOURCE=200809L
+
 #include <assert.h>
 #include <setjmp.h>
 #include <stdlib.h>
@@ -271,9 +275,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
 
 static void clar_time_now(clar_time *out)
 {
-	struct timezone tz;
-
-	gettimeofday(out, &tz);
+	gettimeofday(out, NULL);
 }
 
 static double clar_time_diff(clar_time *start, clar_time *end)
diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h
index e25057b7c4..b499d2e1e6 100644
--- a/t/unit-tests/clar/clar/sandbox.h
+++ b/t/unit-tests/clar/clar/sandbox.h
@@ -122,7 +122,7 @@ static int build_sandbox_path(void)
 
 	if (mkdir(_clar_path, 0700) != 0)
 		return -1;
-#elif defined(__TANDEM)
+#elif defined(__sunos) || defined(__TANDEM)
 	if (mktemp(_clar_path) == NULL)
 		return -1;
 

Re: git no longer builds on SunOS 5.10, a report

[Alejandro R. Sedeño]

On Sun, Oct 13, 2024 at 3:57 PM Patrick Steinhardt <ps@pks.im> wrote:
> diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
> index cef0f023c2..064ca5c2ea 100644
> --- a/t/unit-tests/clar/clar.c
> +++ b/t/unit-tests/clar/clar.c
> @@ -4,6 +4,10 @@
>   * This file is part of clar, distributed under the ISC license.
>   * For full terms see the included COPYING file.
>   */
> +
> +#define _DARWIN_C_SOURCE
> +#define _POSIX_C_SOURCE=200809L

token "=" is not valid in preprocessor expressions.

2008 postdates my available compiler by many years, so trying to define this
is not going to get you everything you might expect here.

Fixing the #define to use a space and not = results in

/usr/include/sys/feature_tests.h:332:2: #error "Compiler or options
invalid for pre-UNIX 03 X/Open applications and pre-2001 POSIX
applications"

The relevant bits of the header:

#if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) && !defined(_XPG6))
#error "Compiler or options invalid for pre-UNIX 03 X/Open applications \
        and pre-2001 POSIX applications"
#elif !defined(_STDC_C99) && \
        (defined(__XOPEN_OR_POSIX) && defined(_XPG6))
#error "Compiler or options invalid; UNIX 03 and POSIX.1-2001 applications \
        require the use of c99"
#endif

Removing `#define _POSIX_C_SOURCE 200809L` results in successful compilation.

> +
>  #include <assert.h>
>  #include <setjmp.h>
>  #include <stdlib.h>
> @@ -271,9 +275,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
>
>  static void clar_time_now(clar_time *out)
>  {
> -       struct timezone tz;
> -
> -       gettimeofday(out, &tz);
> +       gettimeofday(out, NULL);
>  }
>
>  static double clar_time_diff(clar_time *start, clar_time *end)
> diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h
> index e25057b7c4..b499d2e1e6 100644
> --- a/t/unit-tests/clar/clar/sandbox.h
> +++ b/t/unit-tests/clar/clar/sandbox.h
> @@ -122,7 +122,7 @@ static int build_sandbox_path(void)
>
>         if (mkdir(_clar_path, 0700) != 0)
>                 return -1;
> -#elif defined(__TANDEM)
> +#elif defined(__sunos) || defined(__TANDEM)

I think we want __sun here, not __sunos.

>         if (mktemp(_clar_path) == NULL)
>                 return -1;
>

-Alejandro

Re: git no longer builds on SunOS 5.10, a report

[Patrick Steinhardt]

On Sun, Oct 13, 2024 at 06:50:09PM -0400, Alejandro R. Sedeño wrote:
> On Sun, Oct 13, 2024 at 3:57 PM Patrick Steinhardt <ps@pks.im> wrote:
> > diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
> > index cef0f023c2..064ca5c2ea 100644
> > --- a/t/unit-tests/clar/clar.c
> > +++ b/t/unit-tests/clar/clar.c
> > @@ -4,6 +4,10 @@
> >   * This file is part of clar, distributed under the ISC license.
> >   * For full terms see the included COPYING file.
> >   */
> > +
> > +#define _DARWIN_C_SOURCE
> > +#define _POSIX_C_SOURCE=200809L
> 
> token "=" is not valid in preprocessor expressions.

Yeah, typoed this one.

> 2008 postdates my available compiler by many years, so trying to define this
> is not going to get you everything you might expect here.
> 
> Fixing the #define to use a space and not = results in
> 
> /usr/include/sys/feature_tests.h:332:2: #error "Compiler or options
> invalid for pre-UNIX 03 X/Open applications and pre-2001 POSIX
> applications"
> 
> The relevant bits of the header:
> 
> #if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) && !defined(_XPG6))
> #error "Compiler or options invalid for pre-UNIX 03 X/Open applications \
>         and pre-2001 POSIX applications"
> #elif !defined(_STDC_C99) && \
>         (defined(__XOPEN_OR_POSIX) && defined(_XPG6))
> #error "Compiler or options invalid; UNIX 03 and POSIX.1-2001 applications \
>         require the use of c99"
> #endif
> 
> Removing `#define _POSIX_C_SOURCE 200809L` results in successful compilation.

Ah, I didn't even know that headers would bail out in case they don't
support the standard, but it makes sense in retrospect. My current
version is:

#define _BSD_SOURCE
#define _DEFAULT_SOURCE
#define _DARWIN_C_SOURCE

I hope that should work fine on all platforms. In any case, I have
created [1] upstream now.

[1]: https://github.com/clar-test/clar/pull/107

> > +
> >  #include <assert.h>
> >  #include <setjmp.h>
> >  #include <stdlib.h>
> > @@ -271,9 +275,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
> >
> >  static void clar_time_now(clar_time *out)
> >  {
> > -       struct timezone tz;
> > -
> > -       gettimeofday(out, &tz);
> > +       gettimeofday(out, NULL);
> >  }
> >
> >  static double clar_time_diff(clar_time *start, clar_time *end)
> > diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h
> > index e25057b7c4..b499d2e1e6 100644
> > --- a/t/unit-tests/clar/clar/sandbox.h
> > +++ b/t/unit-tests/clar/clar/sandbox.h
> > @@ -122,7 +122,7 @@ static int build_sandbox_path(void)
> >
> >         if (mkdir(_clar_path, 0700) != 0)
> >                 return -1;
> > -#elif defined(__TANDEM)
> > +#elif defined(__sunos) || defined(__TANDEM)
> 
> I think we want __sun here, not __sunos.

And typoed that one, as well :)

Patrick

[PATCH 0/2] t/unit-tests: improve clar platform compatibility

[Patrick Steinhardt]

Hi,

this small patch series addresses two recent reports about platform
compatibility with OpenSUSE Leap and SunOS.

Thanks!

Patrick

Alejandro R. Sedeño (1):
  Makefile: adjust sed command for generating "clar-decls.h"

Patrick Steinhardt (1):
  t/unit-tests: update clar to 0810a36

 Makefile                                   |   2 +-
 t/unit-tests/clar/.editorconfig            |  13 +++
 t/unit-tests/clar/.github/workflows/ci.yml |  20 +++-
 t/unit-tests/clar/.gitignore               |   1 +
 t/unit-tests/clar/CMakeLists.txt           |  28 +++++
 t/unit-tests/clar/clar.c                   | 115 +++++++++++----------
 t/unit-tests/clar/clar/print.h             |  11 +-
 t/unit-tests/clar/clar/sandbox.h           |  17 ++-
 t/unit-tests/clar/clar/summary.h           |  14 +--
 t/unit-tests/clar/test/.gitignore          |   4 -
 t/unit-tests/clar/test/CMakeLists.txt      |  39 +++++++
 t/unit-tests/clar/test/Makefile            |  39 -------
 12 files changed, 178 insertions(+), 125 deletions(-)
 create mode 100644 t/unit-tests/clar/.editorconfig
 create mode 100644 t/unit-tests/clar/.gitignore
 create mode 100644 t/unit-tests/clar/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/.gitignore
 create mode 100644 t/unit-tests/clar/test/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/Makefile

-- 
2.47.0.dirty

[PATCH 1/2] t/unit-tests: update clar to 0810a36

[Patrick Steinhardt]

Update clar from:

    - 1516124 (Merge pull request #97 from pks-t/pks-whitespace-fixes, 2024-08-15).

To:

    - 0810a36 (Merge pull request #107 from pks-t/pks-sunos-compatibility, 2024-10-14)

This update includes a bunch of fixes and improvements that we have
discussed in Git when initial support for clar was merged:

  - There is a ".editorconfig" file now.

  - Compatibility with Windows has been improved so that the clar
    compiles on this platform without an issue. This has been tested
    with Cygwin, MinGW and Microsoft Visual Studio.

  - clar now uses CMake. This does not impact us at all as we wire up
    the clar into our own build infrastructure anyway. This conversion
    was done such that we can easily run CI jobs against Windows.

  - Allocation failures are now checked for consistently.

  - We now define feature test macros in "clar.c", which fixes
    compilation on some platforms that didn't previously pull in
    non-standard functions like lstat(3p) or strdup(3p). This was
    reported by a user of OpenSUSE Leap.

  - We stop using `struct timezone`, which is undefined behaviour
    nowadays and results in a compilation error on some platforms.

  - We now use the combination of mktemp(3) and mkdir(3) on SunOS, same
    as we do on NonStop.

The most important bits here are the improved platform compatibility
with Windows, OpenSUSE and SunOS.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/unit-tests/clar/.editorconfig            |  13 +++
 t/unit-tests/clar/.github/workflows/ci.yml |  20 +++-
 t/unit-tests/clar/.gitignore               |   1 +
 t/unit-tests/clar/CMakeLists.txt           |  28 +++++
 t/unit-tests/clar/clar.c                   | 115 +++++++++++----------
 t/unit-tests/clar/clar/print.h             |  11 +-
 t/unit-tests/clar/clar/sandbox.h           |  17 ++-
 t/unit-tests/clar/clar/summary.h           |  14 +--
 t/unit-tests/clar/test/.gitignore          |   4 -
 t/unit-tests/clar/test/CMakeLists.txt      |  39 +++++++
 t/unit-tests/clar/test/Makefile            |  39 -------
 11 files changed, 177 insertions(+), 124 deletions(-)
 create mode 100644 t/unit-tests/clar/.editorconfig
 create mode 100644 t/unit-tests/clar/.gitignore
 create mode 100644 t/unit-tests/clar/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/.gitignore
 create mode 100644 t/unit-tests/clar/test/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/Makefile

diff --git a/t/unit-tests/clar/.editorconfig b/t/unit-tests/clar/.editorconfig
new file mode 100644
index 00000000000..aa343a42885
--- /dev/null
+++ b/t/unit-tests/clar/.editorconfig
@@ -0,0 +1,13 @@
+root = true
+
+[*]
+charset = utf-8
+insert_final_newline = true
+
+[*.{c,h}]
+indent_style = tab
+tab_width = 8
+
+[CMakeLists.txt]
+indent_style = tab
+tab_width = 8
diff --git a/t/unit-tests/clar/.github/workflows/ci.yml b/t/unit-tests/clar/.github/workflows/ci.yml
index b1ac2de460a..0065843d17a 100644
--- a/t/unit-tests/clar/.github/workflows/ci.yml
+++ b/t/unit-tests/clar/.github/workflows/ci.yml
@@ -10,14 +10,26 @@ jobs:
   build:
     strategy:
       matrix:
-        os: [ ubuntu-latest, macos-latest ]
+        platform:
+          - os: ubuntu-latest
+            generator: Unix Makefiles
+          - os: macos-latest
+            generator: Unix Makefiles
+          - os: windows-latest
+            generator: Visual Studio 17 2022
+          - os: windows-latest
+            generator: MSYS Makefiles
+          - os: windows-latest
+            generator: MinGW Makefiles
 
-    runs-on: ${{ matrix.os }}
+    runs-on: ${{ matrix.platform.os }}
 
     steps:
     - name: Check out
       uses: actions/checkout@v2
     - name: Build
       run: |
-        cd test
-        make
+        mkdir build
+        cd build
+        cmake .. -G "${{matrix.platform.generator}}"
+        cmake --build .
diff --git a/t/unit-tests/clar/.gitignore b/t/unit-tests/clar/.gitignore
new file mode 100644
index 00000000000..84c048a73cc
--- /dev/null
+++ b/t/unit-tests/clar/.gitignore
@@ -0,0 +1 @@
+/build/
diff --git a/t/unit-tests/clar/CMakeLists.txt b/t/unit-tests/clar/CMakeLists.txt
new file mode 100644
index 00000000000..12d4af114fe
--- /dev/null
+++ b/t/unit-tests/clar/CMakeLists.txt
@@ -0,0 +1,28 @@
+cmake_minimum_required(VERSION 3.16..3.29)
+
+project(clar LANGUAGES C)
+
+option(BUILD_TESTS "Build test executable" ON)
+
+add_library(clar INTERFACE)
+target_sources(clar INTERFACE
+	clar.c
+	clar.h
+	clar/fixtures.h
+	clar/fs.h
+	clar/print.h
+	clar/sandbox.h
+	clar/summary.h
+)
+set_target_properties(clar PROPERTIES
+	C_STANDARD 90
+	C_STANDARD_REQUIRED ON
+	C_EXTENSIONS OFF
+)
+
+if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
+	include(CTest)
+	if(BUILD_TESTING)
+		add_subdirectory(test)
+	endif()
+endif()
diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
index cef0f023c24..64879cf2bd5 100644
--- a/t/unit-tests/clar/clar.c
+++ b/t/unit-tests/clar/clar.c
@@ -4,7 +4,12 @@
  * This file is part of clar, distributed under the ISC license.
  * For full terms see the included COPYING file.
  */
-#include <assert.h>
+
+#define _BSD_SOURCE
+#define _DARWIN_C_SOURCE
+#define _DEFAULT_SOURCE
+
+#include <errno.h>
 #include <setjmp.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -13,6 +18,7 @@
 #include <stdarg.h>
 #include <wchar.h>
 #include <time.h>
+#include <inttypes.h>
 
 /* required for sandboxing */
 #include <sys/types.h>
@@ -28,6 +34,9 @@
 
 #	ifndef stat
 #		define stat(path, st) _stat(path, st)
+		typedef struct _stat STAT_T;
+#	else
+		typedef struct stat STAT_T;
 #	endif
 #	ifndef mkdir
 #		define mkdir(path, mode) _mkdir(path)
@@ -60,30 +69,11 @@
 #	else
 #		define p_snprintf snprintf
 #	endif
-
-#	ifndef PRIuZ
-#		define PRIuZ "Iu"
-#	endif
-#	ifndef PRIxZ
-#		define PRIxZ "Ix"
-#	endif
-
-#	if defined(_MSC_VER) || (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR))
-	typedef struct stat STAT_T;
-#	else
-	typedef struct _stat STAT_T;
-#	endif
 #else
 #	include <sys/wait.h> /* waitpid(2) */
 #	include <unistd.h>
 #	define _MAIN_CC
 #	define p_snprintf snprintf
-#	ifndef PRIuZ
-#		define PRIuZ "zu"
-#	endif
-#	ifndef PRIxZ
-#		define PRIxZ "zx"
-#	endif
 	typedef struct stat STAT_T;
 #endif
 
@@ -102,7 +92,7 @@ fixture_path(const char *base, const char *fixture_name);
 struct clar_error {
 	const char *file;
 	const char *function;
-	size_t line_number;
+	uintmax_t line_number;
 	const char *error_msg;
 	char *description;
 
@@ -195,11 +185,12 @@ static void clar_print_shutdown(int test_count, int suite_count, int error_count
 static void clar_print_error(int num, const struct clar_report *report, const struct clar_error *error);
 static void clar_print_ontest(const char *suite_name, const char *test_name, int test_number, enum cl_test_status failed);
 static void clar_print_onsuite(const char *suite_name, int suite_index);
+static void clar_print_onabortv(const char *msg, va_list argp);
 static void clar_print_onabort(const char *msg, ...);
 
 /* From clar_sandbox.c */
 static void clar_unsandbox(void);
-static int clar_sandbox(void);
+static void clar_sandbox(void);
 
 /* From summary.h */
 static struct clar_summary *clar_summary_init(const char *filename);
@@ -218,6 +209,15 @@ static int clar_summary_shutdown(struct clar_summary *fp);
 							   _clar.trace_payload);					\
 	} while (0)
 
+static void clar_abort(const char *msg, ...)
+{
+	va_list argp;
+	va_start(argp, msg);
+	clar_print_onabortv(msg, argp);
+	va_end(argp);
+	exit(-1);
+}
+
 void cl_trace_register(cl_trace_cb *cb, void *payload)
 {
 	_clar.pfn_trace_cb = cb;
@@ -271,9 +271,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
 
 static void clar_time_now(clar_time *out)
 {
-	struct timezone tz;
-
-	gettimeofday(out, &tz);
+	gettimeofday(out, NULL);
 }
 
 static double clar_time_diff(clar_time *start, clar_time *end)
@@ -386,7 +384,8 @@ clar_run_suite(const struct clar_suite *suite, const char *filter)
 
 		_clar.active_test = test[i].name;
 
-		report = calloc(1, sizeof(struct clar_report));
+		if ((report = calloc(1, sizeof(*report))) == NULL)
+			clar_abort("Failed to allocate report.\n");
 		report->suite = _clar.active_suite;
 		report->test = _clar.active_test;
 		report->test_number = _clar.tests_ran;
@@ -479,9 +478,10 @@ clar_parse_args(int argc, char **argv)
 
 					switch (action) {
 					case 's': {
-						struct clar_explicit *explicit =
-							calloc(1, sizeof(struct clar_explicit));
-						assert(explicit);
+						struct clar_explicit *explicit;
+
+						if ((explicit = calloc(1, sizeof(*explicit))) == NULL)
+							clar_abort("Failed to allocate explicit test.\n");
 
 						explicit->suite_idx = j;
 						explicit->filter = argument;
@@ -505,10 +505,8 @@ clar_parse_args(int argc, char **argv)
 				}
 			}
 
-			if (!found) {
-				clar_print_onabort("No suite matching '%s' found.\n", argument);
-				exit(-1);
-			}
+			if (!found)
+				clar_abort("No suite matching '%s' found.\n", argument);
 			break;
 		}
 
@@ -540,11 +538,17 @@ clar_parse_args(int argc, char **argv)
 		case 'r':
 			_clar.write_summary = 1;
 			free(_clar.summary_filename);
-			_clar.summary_filename = *(argument + 2) ? strdup(argument + 2) : NULL;
+			if (*(argument + 2)) {
+				if ((_clar.summary_filename = strdup(argument + 2)) == NULL)
+					clar_abort("Failed to allocate summary filename.\n");
+			} else {
+				_clar.summary_filename = NULL;
+			}
 			break;
 
 		default:
-			assert(!"Unexpected commandline argument!");
+			clar_abort("Unexpected commandline argument '%s'.\n",
+				   argument[1]);
 		}
 	}
 }
@@ -566,22 +570,18 @@ clar_test_init(int argc, char **argv)
 	if (!_clar.summary_filename &&
 	    (summary_env = getenv("CLAR_SUMMARY")) != NULL) {
 		_clar.write_summary = 1;
-		_clar.summary_filename = strdup(summary_env);
+		if ((_clar.summary_filename = strdup(summary_env)) == NULL)
+			clar_abort("Failed to allocate summary filename.\n");
 	}
 
 	if (_clar.write_summary && !_clar.summary_filename)
-		_clar.summary_filename = strdup("summary.xml");
+		if ((_clar.summary_filename = strdup("summary.xml")) == NULL)
+			clar_abort("Failed to allocate summary filename.\n");
 
-	if (_clar.write_summary &&
-	    !(_clar.summary = clar_summary_init(_clar.summary_filename))) {
-		clar_print_onabort("Failed to open the summary file\n");
-		exit(-1);
-	}
+	if (_clar.write_summary)
+	    _clar.summary = clar_summary_init(_clar.summary_filename);
 
-	if (clar_sandbox() < 0) {
-		clar_print_onabort("Failed to sandbox the test runner.\n");
-		exit(-1);
-	}
+	clar_sandbox();
 }
 
 int
@@ -615,10 +615,9 @@ clar_test_shutdown(void)
 
 	clar_unsandbox();
 
-	if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0) {
-		clar_print_onabort("Failed to write the summary file\n");
-		exit(-1);
-	}
+	if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0)
+		clar_abort("Failed to write the summary file '%s: %s.\n",
+			   _clar.summary_filename, strerror(errno));
 
 	for (explicit = _clar.explicit; explicit; explicit = explicit_next) {
 		explicit_next = explicit->next;
@@ -649,7 +648,7 @@ static void abort_test(void)
 {
 	if (!_clar.trampoline_enabled) {
 		clar_print_onabort(
-				"Fatal error: a cleanup method raised an exception.");
+				"Fatal error: a cleanup method raised an exception.\n");
 		clar_report_errors(_clar.last_report);
 		exit(-1);
 	}
@@ -673,7 +672,10 @@ void clar__fail(
 	const char *description,
 	int should_abort)
 {
-	struct clar_error *error = calloc(1, sizeof(struct clar_error));
+	struct clar_error *error;
+
+	if ((error = calloc(1, sizeof(*error))) == NULL)
+		clar_abort("Failed to allocate error.\n");
 
 	if (_clar.last_report->errors == NULL)
 		_clar.last_report->errors = error;
@@ -688,8 +690,9 @@ void clar__fail(
 	error->line_number = line;
 	error->error_msg = error_msg;
 
-	if (description != NULL)
-		error->description = strdup(description);
+	if (description != NULL &&
+	    (error->description = strdup(description)) == NULL)
+		clar_abort("Failed to allocate description.\n");
 
 	_clar.total_errors++;
 	_clar.last_report->status = CL_TEST_FAILURE;
@@ -798,8 +801,8 @@ void clar__assert_equal(
 			}
 		}
 	}
-	else if (!strcmp("%"PRIuZ, fmt) || !strcmp("%"PRIxZ, fmt)) {
-		size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t);
+	else if (!strcmp("%"PRIuMAX, fmt) || !strcmp("%"PRIxMAX, fmt)) {
+		uintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t);
 		is_equal = (sz1 == sz2);
 		if (!is_equal) {
 			int offset = p_snprintf(buf, sizeof(buf), fmt, sz1);
diff --git a/t/unit-tests/clar/clar/print.h b/t/unit-tests/clar/clar/print.h
index c17e2f693bd..69d0ee967e7 100644
--- a/t/unit-tests/clar/clar/print.h
+++ b/t/unit-tests/clar/clar/print.h
@@ -21,7 +21,7 @@ static void clar_print_clap_error(int num, const struct clar_report *report, con
 {
 	printf("  %d) Failure:\n", num);
 
-	printf("%s::%s [%s:%"PRIuZ"]\n",
+	printf("%s::%s [%s:%"PRIuMAX"]\n",
 		report->suite,
 		report->test,
 		error->file,
@@ -136,7 +136,7 @@ static void clar_print_tap_ontest(const char *suite_name, const char *test_name,
 
 		printf("    at:\n");
 		printf("      file: '"); print_escaped(error->file); printf("'\n");
-		printf("      line: %" PRIuZ "\n", error->line_number);
+		printf("      line: %" PRIuMAX "\n", error->line_number);
 		printf("      function: '%s'\n", error->function);
 		printf("    ---\n");
 
@@ -202,10 +202,15 @@ static void clar_print_onsuite(const char *suite_name, int suite_index)
 	PRINT(onsuite, suite_name, suite_index);
 }
 
+static void clar_print_onabortv(const char *msg, va_list argp)
+{
+	PRINT(onabort, msg, argp);
+}
+
 static void clar_print_onabort(const char *msg, ...)
 {
 	va_list argp;
 	va_start(argp, msg);
-	PRINT(onabort, msg, argp);
+	clar_print_onabortv(msg, argp);
 	va_end(argp);
 }
diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h
index e25057b7c49..bc960f50e0f 100644
--- a/t/unit-tests/clar/clar/sandbox.h
+++ b/t/unit-tests/clar/clar/sandbox.h
@@ -122,14 +122,14 @@ static int build_sandbox_path(void)
 
 	if (mkdir(_clar_path, 0700) != 0)
 		return -1;
-#elif defined(__TANDEM)
-	if (mktemp(_clar_path) == NULL)
+#elif defined(_WIN32)
+	if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0)
 		return -1;
 
 	if (mkdir(_clar_path, 0700) != 0)
 		return -1;
-#elif defined(_WIN32)
-	if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0)
+#elif defined(__sun) || defined(__TANDEM)
+	if (mktemp(_clar_path) == NULL)
 		return -1;
 
 	if (mkdir(_clar_path, 0700) != 0)
@@ -142,15 +142,14 @@ static int build_sandbox_path(void)
 	return 0;
 }
 
-static int clar_sandbox(void)
+static void clar_sandbox(void)
 {
 	if (_clar_path[0] == '\0' && build_sandbox_path() < 0)
-		return -1;
+		clar_abort("Failed to build sandbox path.\n");
 
 	if (chdir(_clar_path) != 0)
-		return -1;
-
-	return 0;
+		clar_abort("Failed to change into sandbox directory '%s': %s.\n",
+			   _clar_path, strerror(errno));
 }
 
 const char *clar_sandbox_path(void)
diff --git a/t/unit-tests/clar/clar/summary.h b/t/unit-tests/clar/clar/summary.h
index 4dd352e28b8..0d0b646fe75 100644
--- a/t/unit-tests/clar/clar/summary.h
+++ b/t/unit-tests/clar/clar/summary.h
@@ -66,16 +66,12 @@ struct clar_summary *clar_summary_init(const char *filename)
 	struct clar_summary *summary;
 	FILE *fp;
 
-	if ((fp = fopen(filename, "w")) == NULL) {
-		perror("fopen");
-		return NULL;
-	}
+	if ((fp = fopen(filename, "w")) == NULL)
+		clar_abort("Failed to open the summary file '%s': %s.\n",
+			   filename, strerror(errno));
 
-	if ((summary = malloc(sizeof(struct clar_summary))) == NULL) {
-		perror("malloc");
-		fclose(fp);
-		return NULL;
-	}
+	if ((summary = malloc(sizeof(struct clar_summary))) == NULL)
+		clar_abort("Failed to allocate summary.\n");
 
 	summary->filename = filename;
 	summary->fp = fp;
diff --git a/t/unit-tests/clar/test/.gitignore b/t/unit-tests/clar/test/.gitignore
deleted file mode 100644
index a477d0c40ca..00000000000
--- a/t/unit-tests/clar/test/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-clar.suite
-.clarcache
-clar_test
-*.o
diff --git a/t/unit-tests/clar/test/CMakeLists.txt b/t/unit-tests/clar/test/CMakeLists.txt
new file mode 100644
index 00000000000..7f2c1dc17a9
--- /dev/null
+++ b/t/unit-tests/clar/test/CMakeLists.txt
@@ -0,0 +1,39 @@
+find_package(Python COMPONENTS Interpreter REQUIRED)
+
+add_custom_command(OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/clar.suite"
+	COMMAND "${Python_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/generate.py" --output "${CMAKE_CURRENT_BINARY_DIR}"
+	DEPENDS main.c sample.c clar_test.h
+	WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+)
+
+add_executable(clar_test)
+set_target_properties(clar_test PROPERTIES
+	C_STANDARD 90
+	C_STANDARD_REQUIRED ON
+	C_EXTENSIONS OFF
+)
+
+# MSVC generates all kinds of warnings. We may want to fix these in the future
+# and then unconditionally treat warnings as errors.
+if(NOT MSVC)
+	set_target_properties(clar_test PROPERTIES
+		COMPILE_WARNING_AS_ERROR ON
+	)
+endif()
+
+target_sources(clar_test PRIVATE
+	main.c
+	sample.c
+	"${CMAKE_CURRENT_BINARY_DIR}/clar.suite"
+)
+target_compile_definitions(clar_test PRIVATE
+	CLAR_FIXTURE_PATH="${CMAKE_CURRENT_SOURCE_DIR}/resources/"
+)
+target_compile_options(clar_test PRIVATE
+	$<IF:$<CXX_COMPILER_ID:MSVC>,/W4,-Wall>
+)
+target_include_directories(clar_test PRIVATE
+	"${CMAKE_SOURCE_DIR}"
+	"${CMAKE_CURRENT_BINARY_DIR}"
+)
+target_link_libraries(clar_test clar)
diff --git a/t/unit-tests/clar/test/Makefile b/t/unit-tests/clar/test/Makefile
deleted file mode 100644
index 93c6b2ad32c..00000000000
--- a/t/unit-tests/clar/test/Makefile
+++ /dev/null
@@ -1,39 +0,0 @@
-#
-# Copyright (c) Vicent Marti. All rights reserved.
-#
-# This file is part of clar, distributed under the ISC license.
-# For full terms see the included COPYING file.
-#
-
-#
-# Set up the path to the clar sources and to the fixtures directory
-#
-# The fixture path needs to be an absolute path so it can be used
-# even after we have chdir'ed into the test directory while testing.
-#
-CURRENT_MAKEFILE  := $(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST))
-TEST_DIRECTORY    := $(abspath $(dir $(CURRENT_MAKEFILE)))
-CLAR_PATH         := $(dir $(TEST_DIRECTORY))
-CLAR_FIXTURE_PATH := $(TEST_DIRECTORY)/resources/
-
-CFLAGS=-g -I.. -I. -Wall -DCLAR_FIXTURE_PATH=\"$(CLAR_FIXTURE_PATH)\"
-
-.PHONY: clean
-
-# list the objects that go into our test
-objects = main.o sample.o
-
-# build the test executable itself
-clar_test: $(objects) clar_test.h clar.suite $(CLAR_PATH)clar.c
-	$(CC) $(CFLAGS) -o $@ "$(CLAR_PATH)clar.c" $(objects)
-
-# test object files depend on clar macros
-$(objects) : $(CLAR_PATH)clar.h
-
-# build the clar.suite file of test metadata
-clar.suite:
-	python "$(CLAR_PATH)generate.py" .
-
-# remove all generated files
-clean:
-	$(RM) -rf *.o clar.suite .clarcache clar_test clar_test.dSYM
-- 
2.47.0.dirty

[PATCH 2/2] Makefile: adjust sed command for generating "clar-decls.h"

[Patrick Steinhardt]

From: Alejandro R. Sedeño <asedeno@mit.edu>

This moves the end-of-line marker out of the captured group, matching
the start-of-line marker and for some reason fixing generation of
"clar-decls.h" on some older, more esoteric platforms.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index feeed6f9321..0101d349f38 100644
--- a/Makefile
+++ b/Makefile
@@ -3905,7 +3905,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
-- 
2.47.0.dirty

Re: [PATCH 2/2] Makefile: adjust sed command for generating "clar-decls.h"

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> From: Alejandro R. Sedeño <asedeno@mit.edu>
>
> This moves the end-of-line marker out of the captured group, matching
> the start-of-line marker and for some reason fixing generation of
> "clar-decls.h" on some older, more esoteric platforms.
>
> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index feeed6f9321..0101d349f38 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3905,7 +3905,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
> -- 
> 2.47.0.dirty

You're most likely aware, but this change needs to move when the patch
"Makefile: extract script to generate clar declarations" [1] is merged,
because this line then lives in t/unit-tests/generate-clar-decls.sh.

[1]: https://lore.kernel.org/git/7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im/

-- 
Toon

Re: [PATCH 2/2] Makefile: adjust sed command for generating "clar-decls.h"

[Taylor Blau]

On Fri, Oct 18, 2024 at 05:45:58PM +0200, Toon Claes wrote:
> You're most likely aware, but this change needs to move when the patch
> "Makefile: extract script to generate clar declarations" [1] is merged,
> because this line then lives in t/unit-tests/generate-clar-decls.sh.
>
> [1]: https://lore.kernel.org/git/7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im/

I believe that is what Patrick suggested in:

  https://lore.kernel.org/git/cover.1728914219.git.ps@pks.im/

, no?

Thanks,
Taylor

Re: [PATCH 2/2] Makefile: adjust sed command for generating "clar-decls.h"

[Patrick Steinhardt]

On Fri, Oct 18, 2024 at 05:14:40PM -0400, Taylor Blau wrote:
> On Fri, Oct 18, 2024 at 05:45:58PM +0200, Toon Claes wrote:
> > You're most likely aware, but this change needs to move when the patch
> > "Makefile: extract script to generate clar declarations" [1] is merged,
> > because this line then lives in t/unit-tests/generate-clar-decls.sh.
> >
> > [1]: https://lore.kernel.org/git/7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im/
> 
> I believe that is what Patrick suggested in:
> 
>   https://lore.kernel.org/git/cover.1728914219.git.ps@pks.im/
> 
> , no?

Yup. I think it makes everyones life easier if I were to just merge
these two series together. I would've done so right from the start if I
noticed that they interact with each other, but only saw this at a later
point in time.

I'll send this out later today or early tomorrow.

Patrick

[PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[Patrick Steinhardt]

Hi,

this is the second version of my patch series that addresses some
platform compatibility issues with clar. Changes compared to v1:

  - I've merged the CMake fixes at [1] into this patch series to avoid
    conflicts. @Taylor, please drop that other series, which is
    "ps/cmake-clar".

  - I've fixed up the "generate-clar-decls.h" script.

  - I've updated the clar such that it includes upstreamed changes for
    improved uClibc support when we lack support for `wchar_t`.

Thanks!

Patrick

[1]: <cover.1728914219.git.ps@pks.im>

Alejandro R. Sedeño (1):
  Makefile: adjust sed command for generating "clar-decls.h"

Patrick Steinhardt (4):
  t/unit-tests: update clar to 206accb
  Makefile: extract script to generate clar declarations
  cmake: fix compilation of clar-based unit tests
  cmake: set up proper dependencies for generated clar headers

 Makefile                                   |   4 +-
 contrib/buildsystems/CMakeLists.txt        |  52 +++------
 t/unit-tests/clar/.editorconfig            |  13 +++
 t/unit-tests/clar/.github/workflows/ci.yml |  20 +++-
 t/unit-tests/clar/.gitignore               |   1 +
 t/unit-tests/clar/CMakeLists.txt           |  28 +++++
 t/unit-tests/clar/clar.c                   | 127 ++++++++++++---------
 t/unit-tests/clar/clar/print.h             |  11 +-
 t/unit-tests/clar/clar/sandbox.h           |  17 ++-
 t/unit-tests/clar/clar/summary.h           |  14 +--
 t/unit-tests/clar/test/.gitignore          |   4 -
 t/unit-tests/clar/test/CMakeLists.txt      |  39 +++++++
 t/unit-tests/clar/test/Makefile            |  39 -------
 t/unit-tests/generate-clar-decls.sh        |  16 +++
 14 files changed, 219 insertions(+), 166 deletions(-)
 create mode 100644 t/unit-tests/clar/.editorconfig
 create mode 100644 t/unit-tests/clar/.gitignore
 create mode 100644 t/unit-tests/clar/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/.gitignore
 create mode 100644 t/unit-tests/clar/test/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/Makefile
 create mode 100755 t/unit-tests/generate-clar-decls.sh

Range-diff against v1:
1:  a96fbdbb5f9 ! 1:  06145a141dd t/unit-tests: update clar to 0810a36
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t/unit-tests: update clar to 0810a36
    +    t/unit-tests: update clar to 206accb
     
         Update clar from:
     
    @@ Commit message
     
         To:
     
    -        - 0810a36 (Merge pull request #107 from pks-t/pks-sunos-compatibility, 2024-10-14)
    +        - 206accb (Merge pull request #108 from pks-t/pks-uclibc-without-wchar, 2024-10-21)
     
         This update includes a bunch of fixes and improvements that we have
         discussed in Git when initial support for clar was merged:
    @@ Commit message
           - We now use the combination of mktemp(3) and mkdir(3) on SunOS, same
             as we do on NonStop.
     
    +      - We now support uClibc without support for <wchar.h>.
    +
         The most important bits here are the improved platform compatibility
    -    with Windows, OpenSUSE and SunOS.
    +    with Windows, OpenSUSE, SunOS and uClibc.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ t/unit-tests/clar/clar.c
      
      /* required for sandboxing */
      #include <sys/types.h>
    + #include <sys/stat.h>
    + 
    ++#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)
    ++	/*
    ++	 * uClibc can optionally be built without wchar support, in which case
    ++	 * the installed <wchar.h> is a stub that only defines the `whar_t`
    ++	 * type but none of the functions typically declared by it.
    ++	 */
    ++#else
    ++#	define CLAR_HAVE_WCHAR
    ++#endif
    ++
    + #ifdef _WIN32
    + #	define WIN32_LEAN_AND_MEAN
    + #	include <windows.h>
     @@
      
      #	ifndef stat
    @@ t/unit-tests/clar/clar.c: void clar__assert_equal(
      			}
      		}
      	}
    ++#ifdef CLAR_HAVE_WCHAR
    + 	else if (!strcmp("%ls", fmt)) {
    + 		const wchar_t *wcs1 = va_arg(args, const wchar_t *);
    + 		const wchar_t *wcs2 = va_arg(args, const wchar_t *);
    +@@ t/unit-tests/clar/clar.c: void clar__assert_equal(
    + 			}
    + 		}
    + 	}
     -	else if (!strcmp("%"PRIuZ, fmt) || !strcmp("%"PRIxZ, fmt)) {
     -		size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t);
    ++#endif /* CLAR_HAVE_WCHAR */
     +	else if (!strcmp("%"PRIuMAX, fmt) || !strcmp("%"PRIxMAX, fmt)) {
     +		uintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t);
      		is_equal = (sz1 == sz2);
2:  dda9b8e033c = 2:  17d77f36d41 Makefile: adjust sed command for generating "clar-decls.h"
-:  ----------- > 3:  c2e3fbcd853 Makefile: extract script to generate clar declarations
-:  ----------- > 4:  a30017a4d89 cmake: fix compilation of clar-based unit tests
-:  ----------- > 5:  bb005979e7e cmake: set up proper dependencies for generated clar headers

base-commit: 3a0677f8601d8937562ba14665d773fd8f2d71da
-- 
2.47.0.72.gef8ce8f3d4.dirty

[PATCH v2 1/5] t/unit-tests: update clar to 206accb

[Patrick Steinhardt]

Update clar from:

    - 1516124 (Merge pull request #97 from pks-t/pks-whitespace-fixes, 2024-08-15).

To:

    - 206accb (Merge pull request #108 from pks-t/pks-uclibc-without-wchar, 2024-10-21)

This update includes a bunch of fixes and improvements that we have
discussed in Git when initial support for clar was merged:

  - There is a ".editorconfig" file now.

  - Compatibility with Windows has been improved so that the clar
    compiles on this platform without an issue. This has been tested
    with Cygwin, MinGW and Microsoft Visual Studio.

  - clar now uses CMake. This does not impact us at all as we wire up
    the clar into our own build infrastructure anyway. This conversion
    was done such that we can easily run CI jobs against Windows.

  - Allocation failures are now checked for consistently.

  - We now define feature test macros in "clar.c", which fixes
    compilation on some platforms that didn't previously pull in
    non-standard functions like lstat(3p) or strdup(3p). This was
    reported by a user of OpenSUSE Leap.

  - We stop using `struct timezone`, which is undefined behaviour
    nowadays and results in a compilation error on some platforms.

  - We now use the combination of mktemp(3) and mkdir(3) on SunOS, same
    as we do on NonStop.

  - We now support uClibc without support for <wchar.h>.

The most important bits here are the improved platform compatibility
with Windows, OpenSUSE, SunOS and uClibc.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/unit-tests/clar/.editorconfig            |  13 +++
 t/unit-tests/clar/.github/workflows/ci.yml |  20 +++-
 t/unit-tests/clar/.gitignore               |   1 +
 t/unit-tests/clar/CMakeLists.txt           |  28 +++++
 t/unit-tests/clar/clar.c                   | 127 ++++++++++++---------
 t/unit-tests/clar/clar/print.h             |  11 +-
 t/unit-tests/clar/clar/sandbox.h           |  17 ++-
 t/unit-tests/clar/clar/summary.h           |  14 +--
 t/unit-tests/clar/test/.gitignore          |   4 -
 t/unit-tests/clar/test/CMakeLists.txt      |  39 +++++++
 t/unit-tests/clar/test/Makefile            |  39 -------
 11 files changed, 189 insertions(+), 124 deletions(-)
 create mode 100644 t/unit-tests/clar/.editorconfig
 create mode 100644 t/unit-tests/clar/.gitignore
 create mode 100644 t/unit-tests/clar/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/.gitignore
 create mode 100644 t/unit-tests/clar/test/CMakeLists.txt
 delete mode 100644 t/unit-tests/clar/test/Makefile

diff --git a/t/unit-tests/clar/.editorconfig b/t/unit-tests/clar/.editorconfig
new file mode 100644
index 00000000000..aa343a42885
--- /dev/null
+++ b/t/unit-tests/clar/.editorconfig
@@ -0,0 +1,13 @@
+root = true
+
+[*]
+charset = utf-8
+insert_final_newline = true
+
+[*.{c,h}]
+indent_style = tab
+tab_width = 8
+
+[CMakeLists.txt]
+indent_style = tab
+tab_width = 8
diff --git a/t/unit-tests/clar/.github/workflows/ci.yml b/t/unit-tests/clar/.github/workflows/ci.yml
index b1ac2de460a..0065843d17a 100644
--- a/t/unit-tests/clar/.github/workflows/ci.yml
+++ b/t/unit-tests/clar/.github/workflows/ci.yml
@@ -10,14 +10,26 @@ jobs:
   build:
     strategy:
       matrix:
-        os: [ ubuntu-latest, macos-latest ]
+        platform:
+          - os: ubuntu-latest
+            generator: Unix Makefiles
+          - os: macos-latest
+            generator: Unix Makefiles
+          - os: windows-latest
+            generator: Visual Studio 17 2022
+          - os: windows-latest
+            generator: MSYS Makefiles
+          - os: windows-latest
+            generator: MinGW Makefiles
 
-    runs-on: ${{ matrix.os }}
+    runs-on: ${{ matrix.platform.os }}
 
     steps:
     - name: Check out
       uses: actions/checkout@v2
     - name: Build
       run: |
-        cd test
-        make
+        mkdir build
+        cd build
+        cmake .. -G "${{matrix.platform.generator}}"
+        cmake --build .
diff --git a/t/unit-tests/clar/.gitignore b/t/unit-tests/clar/.gitignore
new file mode 100644
index 00000000000..84c048a73cc
--- /dev/null
+++ b/t/unit-tests/clar/.gitignore
@@ -0,0 +1 @@
+/build/
diff --git a/t/unit-tests/clar/CMakeLists.txt b/t/unit-tests/clar/CMakeLists.txt
new file mode 100644
index 00000000000..12d4af114fe
--- /dev/null
+++ b/t/unit-tests/clar/CMakeLists.txt
@@ -0,0 +1,28 @@
+cmake_minimum_required(VERSION 3.16..3.29)
+
+project(clar LANGUAGES C)
+
+option(BUILD_TESTS "Build test executable" ON)
+
+add_library(clar INTERFACE)
+target_sources(clar INTERFACE
+	clar.c
+	clar.h
+	clar/fixtures.h
+	clar/fs.h
+	clar/print.h
+	clar/sandbox.h
+	clar/summary.h
+)
+set_target_properties(clar PROPERTIES
+	C_STANDARD 90
+	C_STANDARD_REQUIRED ON
+	C_EXTENSIONS OFF
+)
+
+if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
+	include(CTest)
+	if(BUILD_TESTING)
+		add_subdirectory(test)
+	endif()
+endif()
diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
index cef0f023c24..d54e4553674 100644
--- a/t/unit-tests/clar/clar.c
+++ b/t/unit-tests/clar/clar.c
@@ -4,7 +4,12 @@
  * This file is part of clar, distributed under the ISC license.
  * For full terms see the included COPYING file.
  */
-#include <assert.h>
+
+#define _BSD_SOURCE
+#define _DARWIN_C_SOURCE
+#define _DEFAULT_SOURCE
+
+#include <errno.h>
 #include <setjmp.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -13,11 +18,22 @@
 #include <stdarg.h>
 #include <wchar.h>
 #include <time.h>
+#include <inttypes.h>
 
 /* required for sandboxing */
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__)
+	/*
+	 * uClibc can optionally be built without wchar support, in which case
+	 * the installed <wchar.h> is a stub that only defines the `whar_t`
+	 * type but none of the functions typically declared by it.
+	 */
+#else
+#	define CLAR_HAVE_WCHAR
+#endif
+
 #ifdef _WIN32
 #	define WIN32_LEAN_AND_MEAN
 #	include <windows.h>
@@ -28,6 +44,9 @@
 
 #	ifndef stat
 #		define stat(path, st) _stat(path, st)
+		typedef struct _stat STAT_T;
+#	else
+		typedef struct stat STAT_T;
 #	endif
 #	ifndef mkdir
 #		define mkdir(path, mode) _mkdir(path)
@@ -60,30 +79,11 @@
 #	else
 #		define p_snprintf snprintf
 #	endif
-
-#	ifndef PRIuZ
-#		define PRIuZ "Iu"
-#	endif
-#	ifndef PRIxZ
-#		define PRIxZ "Ix"
-#	endif
-
-#	if defined(_MSC_VER) || (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR))
-	typedef struct stat STAT_T;
-#	else
-	typedef struct _stat STAT_T;
-#	endif
 #else
 #	include <sys/wait.h> /* waitpid(2) */
 #	include <unistd.h>
 #	define _MAIN_CC
 #	define p_snprintf snprintf
-#	ifndef PRIuZ
-#		define PRIuZ "zu"
-#	endif
-#	ifndef PRIxZ
-#		define PRIxZ "zx"
-#	endif
 	typedef struct stat STAT_T;
 #endif
 
@@ -102,7 +102,7 @@ fixture_path(const char *base, const char *fixture_name);
 struct clar_error {
 	const char *file;
 	const char *function;
-	size_t line_number;
+	uintmax_t line_number;
 	const char *error_msg;
 	char *description;
 
@@ -195,11 +195,12 @@ static void clar_print_shutdown(int test_count, int suite_count, int error_count
 static void clar_print_error(int num, const struct clar_report *report, const struct clar_error *error);
 static void clar_print_ontest(const char *suite_name, const char *test_name, int test_number, enum cl_test_status failed);
 static void clar_print_onsuite(const char *suite_name, int suite_index);
+static void clar_print_onabortv(const char *msg, va_list argp);
 static void clar_print_onabort(const char *msg, ...);
 
 /* From clar_sandbox.c */
 static void clar_unsandbox(void);
-static int clar_sandbox(void);
+static void clar_sandbox(void);
 
 /* From summary.h */
 static struct clar_summary *clar_summary_init(const char *filename);
@@ -218,6 +219,15 @@ static int clar_summary_shutdown(struct clar_summary *fp);
 							   _clar.trace_payload);					\
 	} while (0)
 
+static void clar_abort(const char *msg, ...)
+{
+	va_list argp;
+	va_start(argp, msg);
+	clar_print_onabortv(msg, argp);
+	va_end(argp);
+	exit(-1);
+}
+
 void cl_trace_register(cl_trace_cb *cb, void *payload)
 {
 	_clar.pfn_trace_cb = cb;
@@ -271,9 +281,7 @@ static double clar_time_diff(clar_time *start, clar_time *end)
 
 static void clar_time_now(clar_time *out)
 {
-	struct timezone tz;
-
-	gettimeofday(out, &tz);
+	gettimeofday(out, NULL);
 }
 
 static double clar_time_diff(clar_time *start, clar_time *end)
@@ -386,7 +394,8 @@ clar_run_suite(const struct clar_suite *suite, const char *filter)
 
 		_clar.active_test = test[i].name;
 
-		report = calloc(1, sizeof(struct clar_report));
+		if ((report = calloc(1, sizeof(*report))) == NULL)
+			clar_abort("Failed to allocate report.\n");
 		report->suite = _clar.active_suite;
 		report->test = _clar.active_test;
 		report->test_number = _clar.tests_ran;
@@ -479,9 +488,10 @@ clar_parse_args(int argc, char **argv)
 
 					switch (action) {
 					case 's': {
-						struct clar_explicit *explicit =
-							calloc(1, sizeof(struct clar_explicit));
-						assert(explicit);
+						struct clar_explicit *explicit;
+
+						if ((explicit = calloc(1, sizeof(*explicit))) == NULL)
+							clar_abort("Failed to allocate explicit test.\n");
 
 						explicit->suite_idx = j;
 						explicit->filter = argument;
@@ -505,10 +515,8 @@ clar_parse_args(int argc, char **argv)
 				}
 			}
 
-			if (!found) {
-				clar_print_onabort("No suite matching '%s' found.\n", argument);
-				exit(-1);
-			}
+			if (!found)
+				clar_abort("No suite matching '%s' found.\n", argument);
 			break;
 		}
 
@@ -540,11 +548,17 @@ clar_parse_args(int argc, char **argv)
 		case 'r':
 			_clar.write_summary = 1;
 			free(_clar.summary_filename);
-			_clar.summary_filename = *(argument + 2) ? strdup(argument + 2) : NULL;
+			if (*(argument + 2)) {
+				if ((_clar.summary_filename = strdup(argument + 2)) == NULL)
+					clar_abort("Failed to allocate summary filename.\n");
+			} else {
+				_clar.summary_filename = NULL;
+			}
 			break;
 
 		default:
-			assert(!"Unexpected commandline argument!");
+			clar_abort("Unexpected commandline argument '%s'.\n",
+				   argument[1]);
 		}
 	}
 }
@@ -566,22 +580,18 @@ clar_test_init(int argc, char **argv)
 	if (!_clar.summary_filename &&
 	    (summary_env = getenv("CLAR_SUMMARY")) != NULL) {
 		_clar.write_summary = 1;
-		_clar.summary_filename = strdup(summary_env);
+		if ((_clar.summary_filename = strdup(summary_env)) == NULL)
+			clar_abort("Failed to allocate summary filename.\n");
 	}
 
 	if (_clar.write_summary && !_clar.summary_filename)
-		_clar.summary_filename = strdup("summary.xml");
+		if ((_clar.summary_filename = strdup("summary.xml")) == NULL)
+			clar_abort("Failed to allocate summary filename.\n");
 
-	if (_clar.write_summary &&
-	    !(_clar.summary = clar_summary_init(_clar.summary_filename))) {
-		clar_print_onabort("Failed to open the summary file\n");
-		exit(-1);
-	}
+	if (_clar.write_summary)
+	    _clar.summary = clar_summary_init(_clar.summary_filename);
 
-	if (clar_sandbox() < 0) {
-		clar_print_onabort("Failed to sandbox the test runner.\n");
-		exit(-1);
-	}
+	clar_sandbox();
 }
 
 int
@@ -615,10 +625,9 @@ clar_test_shutdown(void)
 
 	clar_unsandbox();
 
-	if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0) {
-		clar_print_onabort("Failed to write the summary file\n");
-		exit(-1);
-	}
+	if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0)
+		clar_abort("Failed to write the summary file '%s: %s.\n",
+			   _clar.summary_filename, strerror(errno));
 
 	for (explicit = _clar.explicit; explicit; explicit = explicit_next) {
 		explicit_next = explicit->next;
@@ -649,7 +658,7 @@ static void abort_test(void)
 {
 	if (!_clar.trampoline_enabled) {
 		clar_print_onabort(
-				"Fatal error: a cleanup method raised an exception.");
+				"Fatal error: a cleanup method raised an exception.\n");
 		clar_report_errors(_clar.last_report);
 		exit(-1);
 	}
@@ -673,7 +682,10 @@ void clar__fail(
 	const char *description,
 	int should_abort)
 {
-	struct clar_error *error = calloc(1, sizeof(struct clar_error));
+	struct clar_error *error;
+
+	if ((error = calloc(1, sizeof(*error))) == NULL)
+		clar_abort("Failed to allocate error.\n");
 
 	if (_clar.last_report->errors == NULL)
 		_clar.last_report->errors = error;
@@ -688,8 +700,9 @@ void clar__fail(
 	error->line_number = line;
 	error->error_msg = error_msg;
 
-	if (description != NULL)
-		error->description = strdup(description);
+	if (description != NULL &&
+	    (error->description = strdup(description)) == NULL)
+		clar_abort("Failed to allocate description.\n");
 
 	_clar.total_errors++;
 	_clar.last_report->status = CL_TEST_FAILURE;
@@ -763,6 +776,7 @@ void clar__assert_equal(
 			}
 		}
 	}
+#ifdef CLAR_HAVE_WCHAR
 	else if (!strcmp("%ls", fmt)) {
 		const wchar_t *wcs1 = va_arg(args, const wchar_t *);
 		const wchar_t *wcs2 = va_arg(args, const wchar_t *);
@@ -798,8 +812,9 @@ void clar__assert_equal(
 			}
 		}
 	}
-	else if (!strcmp("%"PRIuZ, fmt) || !strcmp("%"PRIxZ, fmt)) {
-		size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t);
+#endif /* CLAR_HAVE_WCHAR */
+	else if (!strcmp("%"PRIuMAX, fmt) || !strcmp("%"PRIxMAX, fmt)) {
+		uintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t);
 		is_equal = (sz1 == sz2);
 		if (!is_equal) {
 			int offset = p_snprintf(buf, sizeof(buf), fmt, sz1);
diff --git a/t/unit-tests/clar/clar/print.h b/t/unit-tests/clar/clar/print.h
index c17e2f693bd..69d0ee967e7 100644
--- a/t/unit-tests/clar/clar/print.h
+++ b/t/unit-tests/clar/clar/print.h
@@ -21,7 +21,7 @@ static void clar_print_clap_error(int num, const struct clar_report *report, con
 {
 	printf("  %d) Failure:\n", num);
 
-	printf("%s::%s [%s:%"PRIuZ"]\n",
+	printf("%s::%s [%s:%"PRIuMAX"]\n",
 		report->suite,
 		report->test,
 		error->file,
@@ -136,7 +136,7 @@ static void clar_print_tap_ontest(const char *suite_name, const char *test_name,
 
 		printf("    at:\n");
 		printf("      file: '"); print_escaped(error->file); printf("'\n");
-		printf("      line: %" PRIuZ "\n", error->line_number);
+		printf("      line: %" PRIuMAX "\n", error->line_number);
 		printf("      function: '%s'\n", error->function);
 		printf("    ---\n");
 
@@ -202,10 +202,15 @@ static void clar_print_onsuite(const char *suite_name, int suite_index)
 	PRINT(onsuite, suite_name, suite_index);
 }
 
+static void clar_print_onabortv(const char *msg, va_list argp)
+{
+	PRINT(onabort, msg, argp);
+}
+
 static void clar_print_onabort(const char *msg, ...)
 {
 	va_list argp;
 	va_start(argp, msg);
-	PRINT(onabort, msg, argp);
+	clar_print_onabortv(msg, argp);
 	va_end(argp);
 }
diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h
index e25057b7c49..bc960f50e0f 100644
--- a/t/unit-tests/clar/clar/sandbox.h
+++ b/t/unit-tests/clar/clar/sandbox.h
@@ -122,14 +122,14 @@ static int build_sandbox_path(void)
 
 	if (mkdir(_clar_path, 0700) != 0)
 		return -1;
-#elif defined(__TANDEM)
-	if (mktemp(_clar_path) == NULL)
+#elif defined(_WIN32)
+	if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0)
 		return -1;
 
 	if (mkdir(_clar_path, 0700) != 0)
 		return -1;
-#elif defined(_WIN32)
-	if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0)
+#elif defined(__sun) || defined(__TANDEM)
+	if (mktemp(_clar_path) == NULL)
 		return -1;
 
 	if (mkdir(_clar_path, 0700) != 0)
@@ -142,15 +142,14 @@ static int build_sandbox_path(void)
 	return 0;
 }
 
-static int clar_sandbox(void)
+static void clar_sandbox(void)
 {
 	if (_clar_path[0] == '\0' && build_sandbox_path() < 0)
-		return -1;
+		clar_abort("Failed to build sandbox path.\n");
 
 	if (chdir(_clar_path) != 0)
-		return -1;
-
-	return 0;
+		clar_abort("Failed to change into sandbox directory '%s': %s.\n",
+			   _clar_path, strerror(errno));
 }
 
 const char *clar_sandbox_path(void)
diff --git a/t/unit-tests/clar/clar/summary.h b/t/unit-tests/clar/clar/summary.h
index 4dd352e28b8..0d0b646fe75 100644
--- a/t/unit-tests/clar/clar/summary.h
+++ b/t/unit-tests/clar/clar/summary.h
@@ -66,16 +66,12 @@ struct clar_summary *clar_summary_init(const char *filename)
 	struct clar_summary *summary;
 	FILE *fp;
 
-	if ((fp = fopen(filename, "w")) == NULL) {
-		perror("fopen");
-		return NULL;
-	}
+	if ((fp = fopen(filename, "w")) == NULL)
+		clar_abort("Failed to open the summary file '%s': %s.\n",
+			   filename, strerror(errno));
 
-	if ((summary = malloc(sizeof(struct clar_summary))) == NULL) {
-		perror("malloc");
-		fclose(fp);
-		return NULL;
-	}
+	if ((summary = malloc(sizeof(struct clar_summary))) == NULL)
+		clar_abort("Failed to allocate summary.\n");
 
 	summary->filename = filename;
 	summary->fp = fp;
diff --git a/t/unit-tests/clar/test/.gitignore b/t/unit-tests/clar/test/.gitignore
deleted file mode 100644
index a477d0c40ca..00000000000
--- a/t/unit-tests/clar/test/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-clar.suite
-.clarcache
-clar_test
-*.o
diff --git a/t/unit-tests/clar/test/CMakeLists.txt b/t/unit-tests/clar/test/CMakeLists.txt
new file mode 100644
index 00000000000..7f2c1dc17a9
--- /dev/null
+++ b/t/unit-tests/clar/test/CMakeLists.txt
@@ -0,0 +1,39 @@
+find_package(Python COMPONENTS Interpreter REQUIRED)
+
+add_custom_command(OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/clar.suite"
+	COMMAND "${Python_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/generate.py" --output "${CMAKE_CURRENT_BINARY_DIR}"
+	DEPENDS main.c sample.c clar_test.h
+	WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+)
+
+add_executable(clar_test)
+set_target_properties(clar_test PROPERTIES
+	C_STANDARD 90
+	C_STANDARD_REQUIRED ON
+	C_EXTENSIONS OFF
+)
+
+# MSVC generates all kinds of warnings. We may want to fix these in the future
+# and then unconditionally treat warnings as errors.
+if(NOT MSVC)
+	set_target_properties(clar_test PROPERTIES
+		COMPILE_WARNING_AS_ERROR ON
+	)
+endif()
+
+target_sources(clar_test PRIVATE
+	main.c
+	sample.c
+	"${CMAKE_CURRENT_BINARY_DIR}/clar.suite"
+)
+target_compile_definitions(clar_test PRIVATE
+	CLAR_FIXTURE_PATH="${CMAKE_CURRENT_SOURCE_DIR}/resources/"
+)
+target_compile_options(clar_test PRIVATE
+	$<IF:$<CXX_COMPILER_ID:MSVC>,/W4,-Wall>
+)
+target_include_directories(clar_test PRIVATE
+	"${CMAKE_SOURCE_DIR}"
+	"${CMAKE_CURRENT_BINARY_DIR}"
+)
+target_link_libraries(clar_test clar)
diff --git a/t/unit-tests/clar/test/Makefile b/t/unit-tests/clar/test/Makefile
deleted file mode 100644
index 93c6b2ad32c..00000000000
--- a/t/unit-tests/clar/test/Makefile
+++ /dev/null
@@ -1,39 +0,0 @@
-#
-# Copyright (c) Vicent Marti. All rights reserved.
-#
-# This file is part of clar, distributed under the ISC license.
-# For full terms see the included COPYING file.
-#
-
-#
-# Set up the path to the clar sources and to the fixtures directory
-#
-# The fixture path needs to be an absolute path so it can be used
-# even after we have chdir'ed into the test directory while testing.
-#
-CURRENT_MAKEFILE  := $(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST))
-TEST_DIRECTORY    := $(abspath $(dir $(CURRENT_MAKEFILE)))
-CLAR_PATH         := $(dir $(TEST_DIRECTORY))
-CLAR_FIXTURE_PATH := $(TEST_DIRECTORY)/resources/
-
-CFLAGS=-g -I.. -I. -Wall -DCLAR_FIXTURE_PATH=\"$(CLAR_FIXTURE_PATH)\"
-
-.PHONY: clean
-
-# list the objects that go into our test
-objects = main.o sample.o
-
-# build the test executable itself
-clar_test: $(objects) clar_test.h clar.suite $(CLAR_PATH)clar.c
-	$(CC) $(CFLAGS) -o $@ "$(CLAR_PATH)clar.c" $(objects)
-
-# test object files depend on clar macros
-$(objects) : $(CLAR_PATH)clar.h
-
-# build the clar.suite file of test metadata
-clar.suite:
-	python "$(CLAR_PATH)generate.py" .
-
-# remove all generated files
-clean:
-	$(RM) -rf *.o clar.suite .clarcache clar_test clar_test.dSYM
-- 
2.47.0.72.gef8ce8f3d4.dirty

[PATCH v2 2/5] Makefile: adjust sed command for generating "clar-decls.h"

[Patrick Steinhardt]

From: Alejandro R. Sedeño <asedeno@mit.edu>

This moves the end-of-line marker out of the captured group, matching
the start-of-line marker and for some reason fixing generation of
"clar-decls.h" on some older, more esoteric platforms.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index feeed6f9321..0101d349f38 100644
--- a/Makefile
+++ b/Makefile
@@ -3905,7 +3905,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
-- 
2.47.0.72.gef8ce8f3d4.dirty

[PATCH v2 3/5] Makefile: extract script to generate clar declarations

[Patrick Steinhardt]

Extract the script to generate function declarations for the clar unit
testing framework into a standalone script. This is done such that we
can reuse it in other build systems.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                            |  4 +---
 t/unit-tests/generate-clar-decls.sh | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)
 create mode 100755 t/unit-tests/generate-clar-decls.sh

diff --git a/Makefile b/Makefile
index 0101d349f38..6318ec0271b 100644
--- a/Makefile
+++ b/Makefile
@@ -3904,9 +3904,7 @@ GIT-TEST-SUITES: FORCE
             fi
 
 $(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; \
-	done >$@
+	$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
 $(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
 $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
new file mode 100755
index 00000000000..688e0885f4f
--- /dev/null
+++ b/t/unit-tests/generate-clar-decls.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+if test $# -lt 2
+then
+	echo "USAGE: $0 <OUTPUT> <SUITE>..." 2>&1
+	exit 1
+fi
+
+OUTPUT="$1"
+shift
+
+for suite in "$@"
+do
+	sed -ne "s/^\(void test_$(basename "${suite%.c}")__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
+	exit 1
+done >"$OUTPUT"
-- 
2.47.0.72.gef8ce8f3d4.dirty

[PATCH v2 4/5] cmake: fix compilation of clar-based unit tests

[Patrick Steinhardt]

The compilation of clar-based unit tests is broken because we do not
add the binary directory into which we generate the "clar-decls.h" and
"clar.suite" files as include directories. Instead, we accidentally set
up the source directory as include directory.

Fix this by including the binary directory instead of the source
directory. Furthermore, set up the include directories as PUBLIC instead
of PRIVATE such that they propagate from "unit-tests.lib" to the
"unit-tests" executable, which needs to include the same directory.

Reported-by: Ed Reel <edreel@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 62af7b33d2f..093852ad9d6 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1042,7 +1042,7 @@ file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar
 list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
 list(TRANSFORM clar_test_SUITES APPEND ".c")
 add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c")
-target_include_directories(unit-tests-lib PRIVATE "${CMAKE_SOURCE_DIR}/t/unit-tests")
+target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests")
 add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c")
 target_link_libraries(unit-tests unit-tests-lib common-main)
 set_target_properties(unit-tests
-- 
2.47.0.72.gef8ce8f3d4.dirty

[PATCH v2 5/5] cmake: set up proper dependencies for generated clar headers

[Patrick Steinhardt]

The auto-generated headers used by clar are written at configure time
and thus do not get regenerated automatically. Refactor the build
recipes such that we use custom commands instead, which also has the
benefit that we can reuse the same infrastructure as our Makefile.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
 1 file changed, 12 insertions(+), 38 deletions(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 093852ad9d6..9f80ab92656 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})
 endforeach()
 
 parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
-
-set(clar_decls "")
-set(clar_cbs "")
-set(clar_cbs_count 0)
-set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
-list(LENGTH clar_test_SUITES clar_suites_count)
-foreach(suite ${clar_test_SUITES})
-	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
-		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
-
-	list(LENGTH decls decls_count)
-	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
-	string(JOIN "" cbs ${cbs})
-	list(TRANSFORM decls PREPEND "extern ")
-	string(JOIN ";\n" decls ${decls})
-
-	string(APPEND clar_decls "${decls};\n")
-	string(APPEND clar_cbs
-		"static const struct clar_func _clar_cb_${suite}[] = {\n"
-		${cbs}
-		"};\n")
-	string(APPEND clar_suites
-		"    {\n"
-		"        \"${suite}\",\n"
-		"        { NULL, NULL },\n"
-		"        { NULL, NULL },\n"
-		"        _clar_cb_${suite}, ${decls_count}, 1\n"
-		"    },\n")
-	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
-endforeach()
-string(APPEND clar_suites
-	"};\n"
-	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
-	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
-file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
-file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
-
 list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
 list(TRANSFORM clar_test_SUITES APPEND ".c")
-add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c")
+add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
+	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
+add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
+
+add_library(unit-tests-lib ${clar_test_SUITES}
+	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
+	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+)
 target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests")
 add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c")
 target_link_libraries(unit-tests unit-tests-lib common-main)
-- 
2.47.0.72.gef8ce8f3d4.dirty

Re: [PATCH v2 2/5] Makefile: adjust sed command for generating "clar-decls.h"

[Kristoffer Haugsbakk]

On Mon, Oct 21, 2024, at 12:56, Patrick Steinhardt wrote:
> From: Alejandro R. Sedeño <asedeno@mit.edu>
>
> This moves the end-of-line marker out of the captured group, matching
> the start-of-line marker and for some reason fixing generation of
> "clar-decls.h" on some older, more esoteric platforms.
>
> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

Alejandro used two signoffs in his original:[1]

    Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
    Signed-off-by: Alejandro R. Sedeño <asedeno@google.com>

I don’t know if this matters?

See also https://lore.kernel.org/git/xmqqilc571hf.fsf@gitster.g/

🔗 1: https://lore.kernel.org/git/20241012144027.2573690-1-asedeno@mit.edu/

Re: [PATCH v2 2/5] Makefile: adjust sed command for generating "clar-decls.h"

[Patrick Steinhardt]

On Mon, Oct 21, 2024 at 01:07:17PM +0200, Kristoffer Haugsbakk wrote:
> On Mon, Oct 21, 2024, at 12:56, Patrick Steinhardt wrote:
> > From: Alejandro R. Sedeño <asedeno@mit.edu>
> >
> > This moves the end-of-line marker out of the captured group, matching
> > the start-of-line marker and for some reason fixing generation of
> > "clar-decls.h" on some older, more esoteric platforms.
> >
> > Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> Alejandro used two signoffs in his original:[1]
> 
>     Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
>     Signed-off-by: Alejandro R. Sedeño <asedeno@google.com>
> 
> I don’t know if this matters?
> 
> See also https://lore.kernel.org/git/xmqqilc571hf.fsf@gitster.g/
> 
> 🔗 1: https://lore.kernel.org/git/20241012144027.2573690-1-asedeno@mit.edu/

It felt more like an accident than intent, but true, I should've asked.
Alejandro, please let me know whether I should restore the second SOB.

Thanks for reading this carefully!

Patrick

Re: [PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[Taylor Blau]

On Mon, Oct 21, 2024 at 12:56:30PM +0200, Patrick Steinhardt wrote:
> Hi,
>
> this is the second version of my patch series that addresses some
> platform compatibility issues with clar. Changes compared to v1:
>
>   - I've merged the CMake fixes at [1] into this patch series to avoid
>     conflicts. @Taylor, please drop that other series, which is
>     "ps/cmake-clar".
>
>   - I've fixed up the "generate-clar-decls.h" script.
>
>   - I've updated the clar such that it includes upstreamed changes for
>     improved uClibc support when we lack support for `wchar_t`.

Thanks (especially so for the suggestion to drop ps/cmake-clar), will
queue.

Thanks,
Taylor

Re: [PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[karthik nayak]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this is the second version of my patch series that addresses some
> platform compatibility issues with clar. Changes compared to v1:
>
>   - I've merged the CMake fixes at [1] into this patch series to avoid
>     conflicts. @Taylor, please drop that other series, which is
>     "ps/cmake-clar".
>
>   - I've fixed up the "generate-clar-decls.h" script.
>
>   - I've updated the clar such that it includes upstreamed changes for
>     improved uClibc support when we lack support for `wchar_t`.
>
> Thanks!
>

I went through the patches, played around with it locally too, couldn't
find any issues. So looks good to me!


[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Mon, Oct 21, 2024 at 12:56:30PM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> this is the second version of my patch series that addresses some
> platform compatibility issues with clar. Changes compared to v1:
> 
>   - I've merged the CMake fixes at [1] into this patch series to avoid
>     conflicts. @Taylor, please drop that other series, which is
>     "ps/cmake-clar".
> 
>   - I've fixed up the "generate-clar-decls.h" script.
> 
>   - I've updated the clar such that it includes upstreamed changes for
>     improved uClibc support when we lack support for `wchar_t`.
> 

Git builds successfully on Buildroot (aarch64 uClibc with and without wchar,
aarch64 glibc).

Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[Patrick Steinhardt]

On Sat, Oct 26, 2024 at 12:01:18PM +0700, Bagas Sanjaya wrote:
> On Mon, Oct 21, 2024 at 12:56:30PM +0200, Patrick Steinhardt wrote:
> > Hi,
> > 
> > this is the second version of my patch series that addresses some
> > platform compatibility issues with clar. Changes compared to v1:
> > 
> >   - I've merged the CMake fixes at [1] into this patch series to avoid
> >     conflicts. @Taylor, please drop that other series, which is
> >     "ps/cmake-clar".
> > 
> >   - I've fixed up the "generate-clar-decls.h" script.
> > 
> >   - I've updated the clar such that it includes upstreamed changes for
> >     improved uClibc support when we lack support for `wchar_t`.
> > 
> 
> Git builds successfully on Buildroot (aarch64 uClibc with and without wchar,
> aarch64 glibc).
> 
> Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>

Great, thanks a lot for verifying whether this fixes your issues!

Patrick

Re: [PATCH v2 0/5] t/unit-tests: improve clar platform compatibility

[Taylor Blau]

On Sun, Oct 27, 2024 at 02:01:17PM +0100, Patrick Steinhardt wrote:
> On Sat, Oct 26, 2024 at 12:01:18PM +0700, Bagas Sanjaya wrote:
> > On Mon, Oct 21, 2024 at 12:56:30PM +0200, Patrick Steinhardt wrote:
> > > Hi,
> > >
> > > this is the second version of my patch series that addresses some
> > > platform compatibility issues with clar. Changes compared to v1:
> > >
> > >   - I've merged the CMake fixes at [1] into this patch series to avoid
> > >     conflicts. @Taylor, please drop that other series, which is
> > >     "ps/cmake-clar".
> > >
> > >   - I've fixed up the "generate-clar-decls.h" script.
> > >
> > >   - I've updated the clar such that it includes upstreamed changes for
> > >     improved uClibc support when we lack support for `wchar_t`.
> > >
> >
> > Git builds successfully on Buildroot (aarch64 uClibc with and without wchar,
> > aarch64 glibc).
> >
> > Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>
>
> Great, thanks a lot for verifying whether this fixes your issues!

Thanks, both. Let's start merging this one down.

Thanks,
Taylor

Re: [PATCH v2 5/5] cmake: set up proper dependencies for generated clar headers

[Johannes Schindelin]

Hi Patrick,

On Mon, 21 Oct 2024, Patrick Steinhardt wrote:

> The auto-generated headers used by clar are written at configure time
> and thus do not get regenerated automatically. Refactor the build
> recipes such that we use custom commands instead, which also has the
> benefit that we can reuse the same infrastructure as our Makefile.

For the record: I did not use a shell script to generate the header for a
specific reason: Unix shell scripts are not native to Windows. Therefore
they cannot in general be run on Windows, however that was precisely the
idea for the CMake definition: to be run on a vanilla Windows with Visual
Studio installed.

Sadly, even Git's CI definition sets things up in a way that Git for
Windows' Bash can be used in the CMake definition, but in the intended use
case (opening a checkout of git/git in Visual Studio without any further
tools required) won't have a usable Bash.

Therefore I am unsure whether this patch is desirable.

Ciao,
Johannes

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
>  1 file changed, 12 insertions(+), 38 deletions(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 093852ad9d6..9f80ab92656 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})
>  endforeach()
>
>  parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
> -
> -set(clar_decls "")
> -set(clar_cbs "")
> -set(clar_cbs_count 0)
> -set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
> -list(LENGTH clar_test_SUITES clar_suites_count)
> -foreach(suite ${clar_test_SUITES})
> -	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
> -		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
> -
> -	list(LENGTH decls decls_count)
> -	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
> -	string(JOIN "" cbs ${cbs})
> -	list(TRANSFORM decls PREPEND "extern ")
> -	string(JOIN ";\n" decls ${decls})
> -
> -	string(APPEND clar_decls "${decls};\n")
> -	string(APPEND clar_cbs
> -		"static const struct clar_func _clar_cb_${suite}[] = {\n"
> -		${cbs}
> -		"};\n")
> -	string(APPEND clar_suites
> -		"    {\n"
> -		"        \"${suite}\",\n"
> -		"        { NULL, NULL },\n"
> -		"        { NULL, NULL },\n"
> -		"        _clar_cb_${suite}, ${decls_count}, 1\n"
> -		"    },\n")
> -	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
> -endforeach()
> -string(APPEND clar_suites
> -	"};\n"
> -	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
> -	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
> -
>  list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
>  list(TRANSFORM clar_test_SUITES APPEND ".c")
> -add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c")
> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> +	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
> +	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
> +
> +add_library(unit-tests-lib ${clar_test_SUITES}
> +	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +)
>  target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests")
>  add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c")
>  target_link_libraries(unit-tests unit-tests-lib common-main)
> --
> 2.47.0.72.gef8ce8f3d4.dirty
>
>

Re: [PATCH v2 5/5] cmake: set up proper dependencies for generated clar headers

[Phillip Wood]

Hi Johannes

On 05/11/2024 19:55, Johannes Schindelin wrote:
> Hi Patrick,
> 
> On Mon, 21 Oct 2024, Patrick Steinhardt wrote:
> 
>> The auto-generated headers used by clar are written at configure time
>> and thus do not get regenerated automatically. Refactor the build
>> recipes such that we use custom commands instead, which also has the
>> benefit that we can reuse the same infrastructure as our Makefile.
> 
> For the record: I did not use a shell script to generate the header for a
> specific reason: Unix shell scripts are not native to Windows. Therefore
> they cannot in general be run on Windows, however that was precisely the
> idea for the CMake definition: to be run on a vanilla Windows with Visual
> Studio installed.
> 
> Sadly, even Git's CI definition sets things up in a way that Git for
> Windows' Bash can be used in the CMake definition, but in the intended use
> case (opening a checkout of git/git in Visual Studio without any further
> tools required) won't have a usable Bash.
> 
> Therefore I am unsure whether this patch is desirable.

CMakeLists.txt tries to find sh.exe from git-for-windows and errors out 
if it cannot be found. It then uses that shell to run a number of 
scripts. Perhaps we should do the same in this patch? It would certainly 
be a worthwhile improvement to regenerate this file at build time if the 
source has changed.

Best Wishes

Phillip

> Ciao,
> Johannes
> 
>>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>>   contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
>>   1 file changed, 12 insertions(+), 38 deletions(-)
>>
>> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
>> index 093852ad9d6..9f80ab92656 100644
>> --- a/contrib/buildsystems/CMakeLists.txt
>> +++ b/contrib/buildsystems/CMakeLists.txt
>> @@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})
>>   endforeach()
>>
>>   parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
>> -
>> -set(clar_decls "")
>> -set(clar_cbs "")
>> -set(clar_cbs_count 0)
>> -set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
>> -list(LENGTH clar_test_SUITES clar_suites_count)
>> -foreach(suite ${clar_test_SUITES})
>> -	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
>> -		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
>> -
>> -	list(LENGTH decls decls_count)
>> -	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
>> -	string(JOIN "" cbs ${cbs})
>> -	list(TRANSFORM decls PREPEND "extern ")
>> -	string(JOIN ";\n" decls ${decls})
>> -
>> -	string(APPEND clar_decls "${decls};\n")
>> -	string(APPEND clar_cbs
>> -		"static const struct clar_func _clar_cb_${suite}[] = {\n"
>> -		${cbs}
>> -		"};\n")
>> -	string(APPEND clar_suites
>> -		"    {\n"
>> -		"        \"${suite}\",\n"
>> -		"        { NULL, NULL },\n"
>> -		"        { NULL, NULL },\n"
>> -		"        _clar_cb_${suite}, ${decls_count}, 1\n"
>> -		"    },\n")
>> -	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
>> -endforeach()
>> -string(APPEND clar_suites
>> -	"};\n"
>> -	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
>> -	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
>> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
>> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
>> -
>>   list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
>>   list(TRANSFORM clar_test_SUITES APPEND ".c")
>> -add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c")
>> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> +	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
>> +	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
>> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
>> +
>> +add_library(unit-tests-lib ${clar_test_SUITES}
>> +	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
>> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +)
>>   target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests")
>>   add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c")
>>   target_link_libraries(unit-tests unit-tests-lib common-main)
>> --
>> 2.47.0.72.gef8ce8f3d4.dirty
>>
>>
> 

Re: [PATCH v2 5/5] cmake: set up proper dependencies for generated clar headers

[Patrick Steinhardt]

On Wed, Nov 06, 2024 at 10:59:08AM +0000, Phillip Wood wrote:
> Hi Johannes
> 
> On 05/11/2024 19:55, Johannes Schindelin wrote:
> > Hi Patrick,
> > 
> > On Mon, 21 Oct 2024, Patrick Steinhardt wrote:
> > 
> > > The auto-generated headers used by clar are written at configure time
> > > and thus do not get regenerated automatically. Refactor the build
> > > recipes such that we use custom commands instead, which also has the
> > > benefit that we can reuse the same infrastructure as our Makefile.
> > 
> > For the record: I did not use a shell script to generate the header for a
> > specific reason: Unix shell scripts are not native to Windows. Therefore
> > they cannot in general be run on Windows, however that was precisely the
> > idea for the CMake definition: to be run on a vanilla Windows with Visual
> > Studio installed.
> > 
> > Sadly, even Git's CI definition sets things up in a way that Git for
> > Windows' Bash can be used in the CMake definition, but in the intended use
> > case (opening a checkout of git/git in Visual Studio without any further
> > tools required) won't have a usable Bash.
> > 
> > Therefore I am unsure whether this patch is desirable.
> 
> CMakeLists.txt tries to find sh.exe from git-for-windows and errors out if
> it cannot be found. It then uses that shell to run a number of scripts.
> Perhaps we should do the same in this patch? It would certainly be a
> worthwhile improvement to regenerate this file at build time if the source
> has changed.

Yeah, I think this solution makes most sense. I'll send a patch in a bit
to address this.

Thanks, both of you!

Patrick