git no longer builds on SunOS 5.10, a report

PATCH New Alejandro R. Sedeño 9 participants 30 messages Oct 12, 2024 – Nov 8, 2024 Kbuild

Share & tools

Alejandro R. Sedeño

author

2024-10-12 02:10 UTC #0 lore raw

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

Patrick Steinhardt

2024-10-12 08:20 UTC #1 lore raw

Quoted 8 lines · Alejandro R. Sedeño

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.

Quoted 6 lines

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]

Quoted 5 lines

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

t/unit-tests/clar/clar.c +5 -3

index cef0f023c24..76557df3040 100644
@@ -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)

Alejandro R. Sedeño

author

2024-10-12 14:34 UTC #2 lore raw

Quoted 11 lines · Patrick Steinhardt

Quoted 8 lines · Alejandro R. Sedeño
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.

Quoted 2 lines

I'd still appreciate it if you could show me the diff. From thereon I can handle the rest.

Makefile +0 -0

index 2dde1fd2b8..87c1f9e220 100644
@@ -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 $<

Quoted 1 line

$(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

Quoted 16 lines

Quoted 6 lines
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.

Quoted 5 lines

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.

Quoted 7 lines

[snip]
Quoted 5 lines
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.

Quoted 6 lines

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

Alejandro R. Sedeño

author

2024-10-12 14:41 UTC #3 lore raw

PATCH 1/1 Makefile: adjust sed command for generating "clar-decls.h"

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 <[email protected]>

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index 2dde1fd2b8..87c1f9e220 100644
@@ -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

Signature · 1 line

2.39.5

Alejandro R. Sedeño

author

2024-10-12 14:42 UTC #4 lore raw

In reply to Alejandro R. Sedeño

On Sat, Oct 12, 2024 at 10:34 AM Alejandro R. Sedeño <[email protected]> wrote and gmail mangled:

Makefile

Quoted 4 lines

Makefile +0 -0

index 2dde1fd2b8..87c1f9e220 100644

<snip>

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

-Alejandro

Patrick Steinhardt

2024-10-13 19:57 UTC #5 lore raw

In reply to Alejandro R. Sedeño

Quoted 16 lines · Alejandro R. Sedeño

Quoted 1 line · Patrick Steinhardt
On Fri, Oct 11, 2024 at 10:10:26PM -0400, Alejandro R. Sedeño wrote:
Makefile +0 -0
index 2dde1fd2b8..87c1f9e220 100644
@@ -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"

Quoted 8 lines

$(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 $<
Quoted 1 line
$(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!

Quoted 19 lines

Quoted 19 lines
Quoted 6 lines
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.

Quoted 6 lines

Quoted 2 lines
[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

t/unit-tests/clar/clar.c +5 -3

index cef0f023c2..064ca5c2ea 100644
@@ -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)

t/unit-tests/clar/clar/sandbox.h +1 -1

index e25057b7c4..b499d2e1e6 100644
@@ -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;

Alejandro R. Sedeño

author

2024-10-13 22:50 UTC #6 lore raw

t/unit-tests/clar/clar.c line 9

Quoted 11 lines · Patrick Steinhardt

t/unit-tests/clar/clar.c +3 -0

index cef0f023c2..064ca5c2ea 100644
@@ -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.

Quoted 20 lines

t/unit-tests/clar/clar.c +2 -3

+
 #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)

t/unit-tests/clar/clar/sandbox.h +0 -0

index e25057b7c4..b499d2e1e6 100644

t/unit-tests/clar/clar/sandbox.h static int build_sandbox_path(void) · line 125

Quoted 5 lines

t/unit-tests/clar/clar/sandbox.h +1 -1

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

t/unit-tests/clar/clar/sandbox.h static int build_sandbox_path(void) · line 128

Quoted 3 lines

t/unit-tests/clar/clar/sandbox.h +0 -0

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

-Alejandro

Patrick Steinhardt

2024-10-14 06:20 UTC #7 lore raw

Quoted 14 lines · Alejandro R. Sedeño

t/unit-tests/clar/clar.c line 9
Quoted 11 lines · Patrick Steinhardt
t/unit-tests/clar/clar.c +3 -0
index cef0f023c2..064ca5c2ea 100644
@@ -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.

Quoted 14 lines

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"

Quoted 7 lines

#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

Quoted 21 lines

t/unit-tests/clar/clar.c +2 -3

+
 #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)

t/unit-tests/clar/clar/sandbox.h +0 -0

index e25057b7c4..b499d2e1e6 100644
@@ -122,7 +122,7 @@ static int build_sandbox_path(void)

Quoted 6 lines

t/unit-tests/clar/clar/sandbox.h static int build_sandbox_path(void) · line 125
Quoted 4 lines
t/unit-tests/clar/clar/sandbox.h +1 -1
        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

Patrick Steinhardt

2024-10-14 11:45 UTC #8 lore raw

cover t/unit-tests: improve clar platform compatibility

In reply to Alejandro R. Sedeño

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

Signature · 1 line

2.47.0.dirty

Patrick Steinhardt

2024-10-14 11:45 UTC #9 lore raw

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

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 <[email protected]>

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

t/unit-tests/clar/.editorconfig new file +13 -0

new file mode 100644
index 00000000000..aa343a42885
@@ -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

t/unit-tests/clar/.github/workflows/ci.yml +16 -4

index b1ac2de460a..0065843d17a 100644
@@ -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 .

t/unit-tests/clar/.gitignore new file +1 -0

new file mode 100644
index 00000000000..84c048a73cc
@@ -0,0 +1 @@
+/build/

t/unit-tests/clar/CMakeLists.txt new file +28 -0

new file mode 100644
index 00000000000..12d4af114fe
@@ -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()

t/unit-tests/clar/clar.c +59 ~~-56~~

240 lines not on this page · raw message

t/unit-tests/clar/clar/print.h +8 -3

index c17e2f693bd..69d0ee967e7 100644
@@ -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);
 }

t/unit-tests/clar/clar/sandbox.h +8 -9

index e25057b7c49..bc960f50e0f 100644
@@ -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)

t/unit-tests/clar/clar/summary.h +5 -9

index 4dd352e28b8..0d0b646fe75 100644
@@ -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;

t/unit-tests/clar/test/.gitignore deleted +0 -4

deleted file mode 100644
index a477d0c40ca..00000000000
@@ -1,4 +0,0 @@
-clar.suite
-.clarcache
-clar_test
-*.o

t/unit-tests/clar/test/CMakeLists.txt new file +39 -0

new file mode 100644
index 00000000000..7f2c1dc17a9
@@ -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)

t/unit-tests/clar/test/Makefile deleted +0 ~~-39~~

deleted file mode 100644
index 93c6b2ad32c..00000000000
@@ -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

Signature · 1 line

2.47.0.dirty

Patrick Steinhardt

2024-10-14 11:45 UTC #10 lore raw

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

In reply to Patrick Steinhardt

From: Alejandro R. Sedeño <[email protected]>

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 <[email protected]>

Signed-off-by Patrick Steinhardt <[email protected]>

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index feeed6f9321..0101d349f38 100644
@@ -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

Signature · 1 line

2.47.0.dirty

Toon Claes

2024-10-18 15:46 UTC #11 lore raw

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

Quoted 27 lines · Patrick Steinhardt

From: Alejandro R. Sedeño <[email protected]>
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 <[email protected]>
Signed-off-by Patrick Steinhardt <[email protected]>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index feeed6f9321..0101d349f38 100644
@@ -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
Signature · 1 line
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/

Signature · 1 line

Toon

Taylor Blau

2024-10-18 21:14 UTC #12 lore raw

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

Quoted 5 lines · Toon Claes

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/[email protected]/

, no?

Thanks,
Taylor

Patrick Steinhardt

2024-10-21 07:00 UTC #13 lore raw

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

Quoted 5 lines · Taylor Blau

Quoted 4 lines · Toon Claes
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.

Quoted 7 lines

Quoted 1 line
[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/[email protected]/
, 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

Patrick Steinhardt

2024-10-21 10:56 UTC #14 lore raw

v2 cover t/unit-tests: improve clar platform compatibility

In reply to Alejandro R. Sedeño

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]: <[email protected]>

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 <[email protected]>

## 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 <[email protected]>

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

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Patrick Steinhardt

2024-10-21 10:56 UTC #15 lore raw

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

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 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 <[email protected]>

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

t/unit-tests/clar/.editorconfig new file +13 -0

new file mode 100644
index 00000000000..aa343a42885
@@ -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

t/unit-tests/clar/.github/workflows/ci.yml +16 -4

index b1ac2de460a..0065843d17a 100644
@@ -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 .

t/unit-tests/clar/.gitignore new file +1 -0

new file mode 100644
index 00000000000..84c048a73cc
@@ -0,0 +1 @@
+/build/

t/unit-tests/clar/CMakeLists.txt new file +28 -0

new file mode 100644
index 00000000000..12d4af114fe
@@ -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()

t/unit-tests/clar/clar.c +71 ~~-56~~

263 lines not on this page · raw message

t/unit-tests/clar/clar/print.h +8 -3

index c17e2f693bd..69d0ee967e7 100644
@@ -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);
 }

t/unit-tests/clar/clar/sandbox.h +8 -9

index e25057b7c49..bc960f50e0f 100644
@@ -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)

t/unit-tests/clar/clar/summary.h +5 -9

index 4dd352e28b8..0d0b646fe75 100644
@@ -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;

t/unit-tests/clar/test/.gitignore deleted +0 -4

deleted file mode 100644
index a477d0c40ca..00000000000
@@ -1,4 +0,0 @@
-clar.suite
-.clarcache
-clar_test
-*.o

t/unit-tests/clar/test/CMakeLists.txt new file +39 -0

new file mode 100644
index 00000000000..7f2c1dc17a9
@@ -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)

t/unit-tests/clar/test/Makefile deleted +0 ~~-39~~

deleted file mode 100644
index 93c6b2ad32c..00000000000
@@ -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

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Patrick Steinhardt

2024-10-21 10:56 UTC #16 lore raw

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

In reply to Patrick Steinhardt

From: Alejandro R. Sedeño <[email protected]>

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 <[email protected]>

Signed-off-by Patrick Steinhardt <[email protected]>

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index feeed6f9321..0101d349f38 100644
@@ -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

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Kristoffer Haugsbakk

2024-10-21 11:07 UTC #17 lore raw

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

Quoted 8 lines · Patrick Steinhardt

From: Alejandro R. Sedeño <[email protected]>
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 <[email protected]>
Signed-off-by Patrick Steinhardt <[email protected]>

Alejandro used two signoffs in his original:[1]

Signed-off-by Alejandro R. Sedeño <[email protected]>

I don’t know if this matters?

🔗 1: https://lore.kernel.org/git/[email protected]/

Patrick Steinhardt

2024-10-21 11:35 UTC #18 lore raw

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

Quoted 12 lines · Kristoffer Haugsbakk

Quoted 8 lines · Patrick Steinhardt
From: Alejandro R. Sedeño <[email protected]>
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 <[email protected]>
Signed-off-by Patrick Steinhardt <[email protected]>
Alejandro used two signoffs in his original:[1]

Quoted 8 lines

Signed-off-by Alejandro R. Sedeño <[email protected]>
Signed-off-by Alejandro R. Sedeño <[email protected]>
I don’t know if this matters?
See also https://lore.kernel.org/git/[email protected]/
🔗 1: https://lore.kernel.org/git/[email protected]/

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

Patrick Steinhardt

2024-10-21 10:56 UTC #19 lore raw

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

In reply to 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 <[email protected]>

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

Makefile +1 -3

index 0101d349f38..6318ec0271b 100644
@@ -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

t/unit-tests/generate-clar-decls.sh new file +16 -0

new file mode 100755
index 00000000000..688e0885f4f
@@ -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"

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Patrick Steinhardt

2024-10-21 10:56 UTC #20 lore raw

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

In reply to 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 <[email protected]>

Signed-off-by Patrick Steinhardt <[email protected]>

---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

contrib/buildsystems/CMakeLists.txt +1 -1

index 62af7b33d2f..093852ad9d6 100644
@@ -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

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Patrick Steinhardt

2024-10-21 10:56 UTC #21 lore raw

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

In reply to 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 <[email protected]>

---
 contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
 1 file changed, 12 insertions(+), 38 deletions(-)

contrib/buildsystems/CMakeLists.txt +12 ~~-38~~

index 093852ad9d6..9f80ab92656 100644
@@ -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)

Signature · 1 line

2.47.0.72.gef8ce8f3d4.dirty

Johannes Schindelin

2024-11-05 19:55 UTC #22 lore raw

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

Hi Patrick,

Quoted 4 lines · 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.

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

Quoted 73 lines

Signed-off-by Patrick Steinhardt <[email protected]>

---
 contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
 1 file changed, 12 insertions(+), 38 deletions(-)

contrib/buildsystems/CMakeLists.txt +12 ~~-39~~

index 093852ad9d6..9f80ab92656 100644
@@ -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

Phillip Wood

2024-11-06 10:59 UTC #23 lore raw

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

Hi Johannes

Quoted 15 lines · Johannes Schindelin

Hi Patrick,
Quoted 4 lines · 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.
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.

Quoted 6 lines

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

Quoted 77 lines

Ciao,
Johannes

Quoted 73 lines

Signed-off-by Patrick Steinhardt <[email protected]>

---
  contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
  1 file changed, 12 insertions(+), 38 deletions(-)

contrib/buildsystems/CMakeLists.txt +12 ~~-39~~

index 093852ad9d6..9f80ab92656 100644
@@ -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

Patrick Steinhardt

2024-11-08 12:59 UTC #24 lore raw

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

Quoted 23 lines · Phillip Wood

Hi Johannes
Quoted 20 lines · Johannes Schindelin
Hi Patrick,
Quoted 4 lines · 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.
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.

Quoted 7 lines

Quoted 1 line
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

Taylor Blau

2024-10-21 20:52 UTC #25 lore raw

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

In reply to Patrick Steinhardt

Quoted 5 lines · Patrick Steinhardt

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

Quoted 8 lines

- 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

karthik nayak

2024-10-25 12:17 UTC #26 lore raw

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

In reply to Patrick Steinhardt

Quoted 9 lines · 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".

Quoted 7 lines

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

Bagas Sanjaya

Tested-by on the v2 series

2024-10-26 05:01 UTC #27 lore raw

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

In reply to Patrick Steinhardt

Quoted 5 lines · Patrick Steinhardt

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

Quoted 9 lines

- 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 <[email protected]>

Signature · 1 line

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

Patrick Steinhardt

2024-10-27 13:01 UTC #28 lore raw

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

Quoted 12 lines · Bagas Sanjaya

Quoted 11 lines · 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.

Quoted 8 lines

Quoted 3 lines
- 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 <[email protected]>

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

Patrick

Taylor Blau

2024-10-27 23:56 UTC #29 lore raw

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

Quoted 17 lines · Patrick Steinhardt

Quoted 16 lines · Bagas Sanjaya
Quoted 14 lines · 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.

Quoted 6 lines

Quoted 4 lines
Git builds successfully on Buildroot (aarch64 uClibc with and without wchar, aarch64 glibc).
Tested-by Bagas Sanjaya <[email protected]>
Great, thanks a lot for verifying whether this fixes your issues!

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

Thanks,
Taylor