Message ID | 20181110001052.101290-1-sbeller@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | coccicheck: introduce 'pending' semantic patches | expand |
On Sat, 10 Nov 2018 at 01:10, Stefan Beller <sbeller@google.com> wrote: > I dialed back on the workflow, as we may want to explore it first > before writing it down. Makes sense. FWIW, this iteration looks good to me. Martin
On 2018.11.09 16:10, Stefan Beller wrote: > From: SZEDER Gábor <szeder.dev@gmail.com> > > Teach `make coccicheck` to avoid patches named "*.pending.cocci" and > handle them separately in a new `make coccicheck-pending` instead. > This means that we can separate "critical" patches from "FYI" patches. > The former target can continue causing Travis to fail its static > analysis job, while the latter can let us keep an eye on ongoing > (pending) transitions without them causing too much fallout. > > Document the intended use-cases around these two targets. > As the process around the pending patches is not yet fully explored, > leave that out. > > Based-on-work-by: SZEDER Gábor <szeder.dev@gmail.com> > Signed-off-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > I dialed back on the workflow, as we may want to explore it first > before writing it down. > > Stefan Looks good to me. Reviewed-by: Josh Steadmon <steadmon@google.com>
On Fri, Nov 09, 2018 at 04:10:52PM -0800, Stefan Beller wrote: > From: SZEDER Gábor <szeder.dev@gmail.com> > > Teach `make coccicheck` to avoid patches named "*.pending.cocci" and > handle them separately in a new `make coccicheck-pending` instead. > This means that we can separate "critical" patches from "FYI" patches. > The former target can continue causing Travis to fail its static > analysis job, while the latter can let us keep an eye on ongoing > (pending) transitions without them causing too much fallout. > > Document the intended use-cases around these two targets. > As the process around the pending patches is not yet fully explored, > leave that out. > > Based-on-work-by: SZEDER Gábor <szeder.dev@gmail.com> Hm, do I need to sign off? Well, I sign off now anyway, and then Junio can take it if necessary or just ignore it if not. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > Signed-off-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > I dialed back on the workflow, as we may want to explore it first > before writing it down. Yeah, I too think that's better to wait with the workflow details until we gather some experience about how it works out in practice. Thanks for following it through.
diff --git a/Makefile b/Makefile index bbfbb4292d..6bc4654828 100644 --- a/Makefile +++ b/Makefile @@ -2740,9 +2740,12 @@ endif then \ echo ' ' SPATCH result: $@; \ fi -coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci)) +coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci))) -.PHONY: coccicheck +# See contrib/coccinelle/README +coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci)) + +.PHONY: coccicheck coccicheck-pending ### Installation rules diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README index 9c2f8879c2..f0e80bd7f0 100644 --- a/contrib/coccinelle/README +++ b/contrib/coccinelle/README @@ -1,2 +1,43 @@ This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/) semantic patches that might be useful to developers. + +There are two types of semantic patches: + + * Using the semantic transformation to check for bad patterns in the code; + The target 'make coccicheck' is designed to check for these patterns and + it is expected that any resulting patch indicates a regression. + The patches resulting from 'make coccicheck' are small and infrequent, + so once they are found, they can be sent to the mailing list as per usual. + + Example for introducing new patterns: + 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28) + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24) + + Example of fixes using this approach: + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to + a strbuf, 2018-03-25) + f919ffebed (Use MOVE_ARRAY, 2018-01-22) + + These types of semantic patches are usually part of testing, c.f. + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something + to transform, 2018-07-23) + + * Using semantic transformations in large scale refactorings throughout + the code base. + + When applying the semantic patch into a real patch, sending it to the + mailing list in the usual way, such a patch would be expected to have a + lot of textual and semantic conflicts as such large scale refactorings + change function signatures that are used widely in the code base. + A textual conflict would arise if surrounding code near any call of such + function changes. A semantic conflict arises when other patch series in + flight introduce calls to such functions. + + So to aid these large scale refactorings, semantic patches can be used. + However we do not want to store them in the same place as the checks for + bad patterns, as then automated builds would fail. + That is why semantic patches 'contrib/coccinelle/*.pending.cocci' + are ignored for checks, and can be applied using 'make coccicheck-pending'. + + This allows to expose plans of pending large scale refactorings without + impacting the bad pattern checks.