Re: [PATCH v5 10/13] Makefile: copy contrib/coccinelle/*.cocci to build/

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH v5 10/13] Makefile: copy contrib/coccinelle/*.cocci to build/
Date: Wed, 9 Nov 2022 16:05:56 +0100	[thread overview]
Message-ID: <20221109150556.GE1731@szeder.dev> (raw)
In-Reply-To: <patch-v5-10.13-56ca8f5720a-20221101T222616Z-avarab@gmail.com>

On Tue, Nov 01, 2022 at 11:35:52PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Change the "coccinelle" rule so that we first copy the *.cocci source
> in e.g. "contrib/coccinelle/strbuf.cocci" to
> ".build/contrib/coccinelle/strbuf.cocci" before operating on it.

After this patch the output of 'make coccicheck' looks like this:

    CP contrib/coccinelle/hashmap.cocci .build/contrib/coccinelle/hashmap.cocci
    MKDIR -p .build/.build/contrib/coccinelle/hashmap.cocci.patch
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/unpack-trees.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/gpg-interface.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/linear-assignment.c

Notice how there is not one but two leading '.build' path components.
Surely one would be enough :)

This also breaks 'make cocciclean':

  $ make cocciclean
  rm -f GIT-SPATCH-DEFINES
  rm -f -r .build/contrib/coccinelle
  rm -f contrib/coccinelle/*.cocci.patch
  $ find .build/
  .build/
  .build/contrib
  .build/.build
  .build/.build/contrib
  .build/.build/contrib/coccinelle
  .build/.build/contrib/coccinelle/hashmap.cocci.patch
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/unpack-trees.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/gpg-interface.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/linear-assignment.c

> For now this serves as a rather pointless indirection, but prepares us
> for the subsequent commit where we'll be able to inject generated
> *.cocci files. Having the entire dependency tree live inside .build/*
> simplifies both the globbing we'd need to do, and any "clean" rules.
> 
> It will also help for future targets which will want to act on the
> generated patches or the logs, e.g. targets to alert if we can't parse
> certain files (or, less so than usual) with "spatch", and e.g. a
> replacement for "ci/run-static-analysis.sh". Such a replacement won't
> care about placing the patches in the in-tree, only whether they're
> "OK" (and about the diff).
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Makefile   | 27 +++++++++++++++++++++------
>  shared.mak |  1 +
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c7c96d284dd..44c906b65d5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3164,8 +3164,11 @@ check: $(GENERATED_H)
>  	fi
>  
>  COCCI_GLOB = $(wildcard contrib/coccinelle/*.cocci)
> -COCCI_RULES = $(COCCI_GLOB)
> -COCCI_NAMES = $(COCCI_RULES:contrib/coccinelle/%.cocci=%)
> +COCCI_RULES_TRACKED = $(COCCI_GLOB:%=.build/%)
> +COCCI_RULES =
> +COCCI_RULES += $(COCCI_RULES_TRACKED)
> +COCCI_NAMES =
> +COCCI_NAMES += $(COCCI_RULES:.build/contrib/coccinelle/%.cocci=%)
>  
>  COCCICHECK_PENDING = $(filter %.pending.cocci,$(COCCI_RULES))
>  COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_RULES))
> @@ -3173,6 +3176,9 @@ COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_RULES))
>  COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
>  COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
>  
> +COCCICHECK_PATCHES_INTREE = $(COCCICHECK_PATCHES:.build/%=%)
> +COCCICHECK_PATCHES_PENDING_INTREE = $(COCCICHECK_PATCHES_PENDING:.build/%=%)
> +
>  # It's expensive to compute the many=many rules below, only eval them
>  # on $(MAKECMDGOALS) that match these $(COCCI_RULES)
>  COCCI_RULES_GLOB =
> @@ -3180,10 +3186,16 @@ COCCI_RULES_GLOB += cocci%
>  COCCI_RULES_GLOB += .build/contrib/coccinelle/%
>  COCCI_RULES_GLOB += $(COCCICHECK_PATCHES)
>  COCCI_RULES_GLOB += $(COCCICHEC_PATCHES_PENDING)
> +COCCI_RULES_GLOB += $(COCCICHECK_PATCHES_INTREE)
> +COCCI_RULES_GLOB += $(COCCICHECK_PATCHES_PENDING_INTREE)
>  COCCI_GOALS = $(filter $(COCCI_RULES_GLOB),$(MAKECMDGOALS))
>  
>  COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
>  
> +$(COCCI_RULES_TRACKED): .build/% : %
> +	$(call mkdir_p_parent_template)
> +	$(QUIET_CP)cp $< $@
> +
>  .build/contrib/coccinelle/FOUND_H_SOURCES: $(FOUND_H_SOURCES)
>  	$(call mkdir_p_parent_template)
>  	$(QUIET_GEN) >$@
> @@ -3197,7 +3209,7 @@ define cocci-rule
>  # $(1) = e.g. "free.cocci"
>  # $(2) = e.g. "grep.c"
>  # $(3) = e.g. "grep.o"
> -COCCI_$(1:contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
> +COCCI_$(1:.build/contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
>  .build/$(1).patch/$(2): GIT-SPATCH-DEFINES
>  .build/$(1).patch/$(2): $(if $(and $(SPATCH_USE_O_DEPENDENCIES),$(wildcard $(3))),$(3),.build/contrib/coccinelle/FOUND_H_SOURCES)
>  .build/$(1).patch/$(2): $(1)
> @@ -3225,12 +3237,15 @@ endif
>  
>  define spatch-rule
>  
> -contrib/coccinelle/$(1).cocci.patch: $$(COCCI_$(1))
> +.build/contrib/coccinelle/$(1).cocci.patch: $$(COCCI_$(1))
>  	$$(QUIET_SPATCH_CAT)cat $$^ >$$@ && \
>  	if test -s $$@; \
>  	then \
>  		echo '    ' SPATCH result: $$@; \
>  	fi
> +contrib/coccinelle/$(1).cocci.patch: .build/contrib/coccinelle/$(1).cocci.patch
> +	$$(QUIET_CP)cp $$< $$@
> +
>  endef
>  
>  ifdef COCCI_GOALS
> @@ -3254,11 +3269,11 @@ $(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinell
>  coccicheck-test: $(COCCI_TEST_RES_GEN)
>  
>  coccicheck: coccicheck-test
> -coccicheck: $(COCCICHECK_PATCHES)
> +coccicheck: $(COCCICHECK_PATCHES_INTREE)
>  
>  # See contrib/coccinelle/README
>  coccicheck-pending: coccicheck-test
> -coccicheck-pending: $(COCCICHECK_PATCHES_PENDING)
> +coccicheck-pending: $(COCCICHECK_PATCHES_PENDING_INTREE)
>  
>  .PHONY: coccicheck coccicheck-pending
>  
> diff --git a/shared.mak b/shared.mak
> index f437073e48c..a34b66c926d 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -60,6 +60,7 @@ ifndef V
>  	QUIET_AR       = @echo '   ' AR $@;
>  	QUIET_LINK     = @echo '   ' LINK $@;
>  	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
> +	QUIET_CP       = @echo '   ' CP $< $@;
>  	QUIET_LNCP     = @echo '   ' LN/CP $@;
>  	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
>  	QUIET_MSGINIT  = @echo '   ' MSGINIT $@;
> -- 
> 2.38.0.1280.g8136eb6fab2
> 