public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Toon Claes <toon@iotcl•com>
To: Patrick Steinhardt <ps@pks•im>, git@vger•kernel.org
Cc: Ed Reel <edreel@gmail•com>,
	Johannes Schindelin <Johannes.Schindelin@gmx•de>,
	Taylor Blau <me@ttaylorr•com>
Subject: Re: [PATCH v2 1/3] Makefile: extract script to generate clar declarations
Date: Fri, 18 Oct 2024 17:21:17 +0200	[thread overview]
Message-ID: <871q0dcu8i.fsf@iotcl.com> (raw)
In-Reply-To: <7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im>

Patrick Steinhardt <ps@pks•im> writes:

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

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,$^)`?

--
Toon


  parent reply	other threads:[~2024-10-18 15:21 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-09  3:23 Bug report Ed Reel
2024-10-14  6:10 ` Johannes Schindelin
2024-10-14 14:06 ` [PATCH 0/3] cmake: fix autogenerated clar headers Patrick Steinhardt
2024-10-14 14:06   ` [PATCH 1/3] Makefile: extract script to generate clar declarations Patrick Steinhardt
2024-10-14 21:42     ` Taylor Blau
2024-10-15  9:08       ` Patrick Steinhardt
2024-10-14 14:06   ` [PATCH 2/3] cmake: fix compilation of clar-based unit tests Patrick Steinhardt
2024-10-14 21:46     ` Taylor Blau
2024-10-15  9:09       ` Patrick Steinhardt
2024-10-14 14:06   ` [PATCH 3/3] cmake: set up proper dependencies for generated clar headers Patrick Steinhardt
2024-10-14 21:47     ` Taylor Blau
2024-10-14 21:40   ` [PATCH 0/3] cmake: fix autogenerated " Taylor Blau
2024-10-15  9:46 ` [PATCH v2 " Patrick Steinhardt
2024-10-15  9:46   ` [PATCH v2 1/3] Makefile: extract script to generate clar declarations Patrick Steinhardt
2024-10-15 19:24     ` Taylor Blau
2024-10-18 15:21     ` Toon Claes [this message]
2024-10-21  6:59       ` Patrick Steinhardt
2024-10-15  9:46   ` [PATCH v2 2/3] cmake: fix compilation of clar-based unit tests Patrick Steinhardt
2024-10-15  9:46   ` [PATCH v2 3/3] cmake: set up proper dependencies for generated clar headers Patrick Steinhardt
2024-10-15 19:25   ` [PATCH v2 0/3] cmake: fix autogenerated " Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871q0dcu8i.fsf@iotcl.com \
    --to=toon@iotcl$(echo .)com \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=edreel@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=me@ttaylorr$(echo .)com \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox