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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Oct 18, 2024 at 05:21:17PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
> > new file mode 100755
> > index 00000000000..81da732917a
> > --- /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_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\)/extern \1;/p" "$suite" ||
> 
> In the Makefile the first `suite` was wrapped in curly braces. And I
> think we need to keep them in this script as well. I noticed because I
> was reviewing this code in my editor I've noticed it highlights
> "source__" as the variable name. You can see what happens if you add
> `set -x` to the top of the script:
> 
>     $ make t/unit-tests/clar-decls.h V=1
>     /bin/sh t/unit-tests/generate-clar-decls.sh "t/unit-tests/clar-decls.h" t/unit-tests/ctype.c t/unit-tests/strvec.c
>     + test 3 -lt 2
>     + OUTPUT=t/unit-tests/clar-decls.h
>     + shift
>     + for suite in "$@"
>     + sed -ne 's/^\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\)/extern \1;/p' t/unit-tests/ctype.c
>     + for suite in "$@"
>     + sed -ne 's/^\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\)/extern \1;/p' t/unit-tests/strvec.c
> 
> So it seems the script currently works "by accident".

Oh, indeed.

> You should replace the first $suite with something like:
> 
>     $(basename $suite .c)
> 
> One other suggestion, and feel free to disagree. What do you think about
> replacing the `$(patsubst ...)` in the recipe to `$(filter %.c,$^)`?

Yup, both of these are good suggestions.

Thanks!

Patrick