Re: [PATCH] coccicheck: introduce 'pending' semantic patches

2018-11-13 Thread SZEDER Gábor
On Fri, Nov 09, 2018 at 04:10:52PM -0800, Stefan Beller wrote:
> From: SZEDER Gábor 
> 
> 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 

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 

> Signed-off-by: Stefan Beller 
> Signed-off-by: Junio C Hamano 
> ---
> 
> 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.



Re: [PATCH] coccicheck: introduce 'pending' semantic patches

2018-11-12 Thread Josh Steadmon
On 2018.11.09 16:10, Stefan Beller wrote:
> From: SZEDER Gábor 
> 
> 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 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Junio C Hamano 
> ---
> 
> 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 


Re: [PATCH] coccicheck: introduce 'pending' semantic patches

2018-11-10 Thread Martin Ågren
On Sat, 10 Nov 2018 at 01:10, Stefan Beller  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


[PATCH] coccicheck: introduce 'pending' semantic patches

2018-11-09 Thread Stefan Beller
From: SZEDER Gábor 

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 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---

I dialed back on the workflow, as we may want to explore it first
before writing it down.

Stefan

 Makefile  |  7 +--
 contrib/coccinelle/README | 41 +++
 2 files changed, 46 insertions(+), 2 deletions(-)

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.
-- 
2.19.1.930.g4563a0d9d0-goog