| Message ID | patch-v5-10.13-56ca8f5720a-20221101T222616Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 202086b85c6591e99ee18e31277786d43f2804a2 |
| Headers | show |
| Series | cocci: make "incremental" possible + a ccache-like tool | expand |
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 >
On Wed, Nov 09 2022, SZEDER Gábor wrote: > 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 :) Oops, well spotted, I'll submit a patch on top soon to fix that... > 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 ...and that, i.e. the nested .build is clearly unintended... Aside: Now that "coccicheck" is a well-behaved (well, mostly, sans the above) target that knows its deps etc. I wonder if it makes sense to have it clean this at all, and just leave it for "make clean". I.e. it should clean the worktree litter, but we could just leave the ".build/contrib/coccinelle". Anyway, that's just a thought, and something to leave for some later "what should we clean in .build" topic, if any. I'll make sure it rm -rf's the right .build/ dir, and that we put stuff in it...
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 $@;
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. 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(-)