Re: New semantic patches vs. in-flight topics [was: Re: [PATCH 00/19] Bring more repository handles into our code base]

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> I don't really like how this or the previous RFC patch series deal
> with semantic patches (or how some past patch series dealt with them,
> for that matter), for various reasons:
> ...
> How about introducing the concept of "pending" semantic patches,
> stored in 'contrib/coccinelle/<name>.pending.cocci' files, modifying
> 'make coccicheck' to skip them, and adding the new 'make
> coccicheck-pending' target to make it convenient to apply them, e.g.
> something like the simple patch at the end.
>
> So the process would go something like this:
>
>   - A new semantic patch should be added as "pending", e.g. to the
>     file 'the_repository.pending.cocci', together with the resulting
>     transformations in the same commit.
>
>     This way neither 'make coccicheck' nor the static analysis build
>     job would complain in the topic branch or in the two integration
>     branches.  And if they do complain, then we would know right away
>     that they complain because of a well-established semantic patch.
>     Yet, anyone interested could run 'make coccicheck-pending' to see
>     where are we heading.
>
>   - The author of the "pending" semanting patch should then keep an
>     eye on already cooking topics: whether any of them contain new
>     code that should be transformed, and how they progress to
>     'master', and sending followup patch(es) with the remaining
>     transformations when applicable.
>     
>     Futhermore, the author should also pay attention to any new topics
>     that branch off after the "pending" semantic patch, and whether
>     any of them introduce code to be transformed, warning their
>     authors as necessary.
>
>   - Finally, after all the dust settled, the dev should follow up with
>     a patch to:
>     
>       - promote the "penging" patch to '<name>.cocci', if its purpose
>         is to avoid undesirable code patterns in the future, or
>     
>       - remove the semantic patch, if it was used in a one-off
>         transformation.
> ...
> Thoughts?

I actually think this round does a far nicer job playing well with
other topics than any earlier series.  The pain you are observing I
think come primarily from my not making the best use of these
patches.

Steppng back a bit, I'd imagine in an ideal world where "make
coccicheck" can be done instantaneously _and_ the spatch machinery
is just as reliable as C compilers.  In such a world, I may rename
all the *.c files in my tree to *.c.in, make the very first step of
the "make all" to run coccicheck and transform *.c.in to *.c before
starting the compilation.  There is no need to have changes to
*.c.in that spatch would fix.  Such an idealized set-up has a few
nice property.

 - Mechanical conflict resolutions between topics in flight and a
   series that modifies or adds new .cocci rules would greatly be
   reduced.

 - Still, *.c files that are "compiled" from *.c.in file used by
   each build will by definition cocci-clean.

 - Bugs like the one that required 6afedba8 ("object_id.cocci: match
   only expressions of type 'struct object_id'", 2018-10-15) are
   still possible, but some of them may be caught by C compilers
   that inspect the result of spatch compilation from *.c.in to *.c

Now we do not live in that ideal world and there is no separate
"turn *.c.in into *.c" step in our build procedure.  In such a
"real" world, if we had a rule like "each individual commit must
pass 'make coccicheck' cleanly", anybody who does such a merge and
resolve huge conflics would essentially be wasting time on something
that the tool could do, and then the result of the merge would
further need to be amended for semantic conflicts (i.e. the other
branch may have introduced new instances of old pattern .cocci
patches in our branch would want to transform)).  By not insisting
on cocci-cleanness at each commit level, we can gain (or at least
sumulate gaining) some benefit that we would reap in the ideal
world, and Stefan's latest series already does so for the former.
If we insisted that these patches must be accompanied with the
result of the cocci transformations, such a series will have zero
chance of landing in 'pu', unless we freeze the world.

What I _could_ do (and what I did do only for one round of pushing
out 'pu') is to queue a coccinelle transformation at the tip of
integration branches.  If "make coccicheck" ran in subsecond, I
could even automate it in the script that is used to rebuild 'pu'
every day, so that after merging each and every topic, do the "make
coccicheck" and apply resulting .cocci.patch files and squash that
into the merge commit.

But with the current "make coccicheck" performance, that is not
realistic.

I am wondering if it is feasible to do it at the tip of 'pu' (which
is rebuilt two to three times a day), 'next' (which is updated once
or twice a week) and 'master'.

I find that your "pending" idea may be nicer, as it distributes the
load.  Whoever wants to change the world order by updating the .cocci
rules is primarily responsible for making it happen without breaking
the world during the transition.  That's more scalable.

But at some point in the ideal future, it may turn out that it is
making the process scalable by distributing a task that does not
have to be done by any human, not by the integrator or by individual
developer---but the new compilation step from *.c.in to *.c should
be able to do.  We obviously are not there yet, so...