diff mbox series

[1/3] Makefile: extract script to generate clar declarations

Message ID 346aa2f08307fb9c501a4b9ac3322beb44dc9d5d.1728914219.git.ps@pks.im (mailing list archive)
State New
Headers show
Series [1/3] Makefile: extract script to generate clar declarations | expand

Commit Message

Patrick Steinhardt Oct. 14, 2024, 2:06 p.m. UTC
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 | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100755 t/unit-tests/generate-clar-decls.sh

Comments

Taylor Blau Oct. 14, 2024, 9:42 p.m. UTC | #1
On Mon, Oct 14, 2024 at 04:06:41PM +0200, Patrick Steinhardt wrote:
> 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.

Makes sense.

> +while test "$#" -ne 0
> +do
> +	suite="$1"
> +	shift

I'm perhaps nit-picking here, but I find:

    for suite in "$@"
    do
        ...
    done

to be much more readable than 'while test "$#" -ne 0' and 'shift'
above, since the for-loop variant does not need to mangle its arguments
with shift.

> +	sed -ne "s/^\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\)/extern \1;/p" "$suite" ||

This part is a faithful translation of the original Makefile. Looking
good.

Thanks,
Taylor
Patrick Steinhardt Oct. 15, 2024, 9:08 a.m. UTC | #2
On Mon, Oct 14, 2024 at 05:42:39PM -0400, Taylor Blau wrote:
> On Mon, Oct 14, 2024 at 04:06:41PM +0200, Patrick Steinhardt wrote:
> > +while test "$#" -ne 0
> > +do
> > +	suite="$1"
> > +	shift
> 
> I'm perhaps nit-picking here, but I find:
> 
>     for suite in "$@"
>     do
>         ...
>     done
> 
> to be much more readable than 'while test "$#" -ne 0' and 'shift'
> above, since the for-loop variant does not need to mangle its arguments
> with shift.

It certainly is, yeah. I know that there was a reason to write it in
this convoluted way... was it compatibility with non-Bash shells when
there were spaces in the paths? I don't know, and I cannot find a case
right now where it breaks.

Will adapt accordingly.

Patrick
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index feeed6f9321..87b86c5be1a 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 "$@" $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES))
 $(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..6646a90f711
--- /dev/null
+++ b/t/unit-tests/generate-clar-decls.sh
@@ -0,0 +1,18 @@ 
+#!/bin/sh
+
+if test $# -lt 2
+then
+	echo "USAGE: $0 <OUTPUT> <SUITE>..." 2>&1
+	exit 1
+fi
+
+OUTPUT="$1"
+shift
+
+while test "$#" -ne 0
+do
+	suite="$1"
+	shift
+	sed -ne "s/^\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\)/extern \1;/p" "$suite" ||
+	exit 1
+done >"$OUTPUT"