Re: [Ksummit-discuss] bug-introducing patches

From: Al Viro
Date: Thu May 03 2018 - 14:20:52 EST


On Thu, May 03, 2018 at 05:34:24PM +0000, Sasha Levin wrote:

> >Moreover, what the hell do you suggest in situation when
> > * foofs_barf() is b0rken in quite a few ways. There's an
> >easily triggerable memory corruptor that can be fixed locally as well
> >as something else that needs a change of e.g. ->mkdir() calling
> >conventions to take care of. The change is mechanical and fairly
> >simple, but it's already -rc4.
>
> I'm not advocating to forcefully block people from submitting patches
> after -rc4 (that was Ted's suggesting).

I am, though - change of a method signature when we have several dozens
of instances does *not* belong in -rc5; if nothing else, it guarantees
a nightmare pile of conflicts with individual filesystem trees.

> I'm just saying that as a maintainer, you should use your brain and
> figure out how critical the bug is, how good is the fix and how well was
> it tested, and decide if you want to merge it in or not.
>
> If it fixed the bug and didn't introduce a regression, great! If it
> messed something else, you'd have some input on how to address it better
> in the future.
>
> I'm trying to come up with a tool/system to help maintainers with
> this task because right now it's not working too well. I'm not trying to
> introduce arbitrary rules to make your life miserable.

And I am asking you what kind of rules do you want/expect/would prefer
for Fixes: pseudo-header. *I* do not give a flying fuck for its
contents; I can put it in, if there is a good reason, though. And
the obvious consumers of that thing are -stable maintainers. Including
yourself. Which is why I am asking you what should go in there in
situation described above. And no, that's not a rhetorical question;
I really want to know.

Let me describe it again:
* a bunch of holes is found in a function; all of them go back
several years
* a clean fix for the whole pile is a composition of
1) local fix of trivially triggered memory corruptor
2) tree-wide mechanical change of method signature + matching modifications
of callers of that method (say, all five of them).
3) further changes in the function in question and its caller (which happens
to be an instance of the method modified by (2).
* dependencies between parts: (1) is standalone, (3) has a hard
dependency on (2), (1) can be reordered past (2)+(modified 3), but modifications
needed in (1) and (3) are not trivial.
* the crap fixed by (1) is much more severe than that fixed by (3)
(and (2) is an equivalent transformation which does not affect behaviour of
anything).
* too late in the cycle for tree-wide patches like (2).

As far as I'm concerned (and if it makes -stable folks' lives unpleasant,
too fucking bad) the merge order is: (1) as soon as it's sufficiently
reviewed and tested, (2) and (3) - next merge window. The only question
is what kind of metadata should go into commit messages to minimize the
PITA for -stable folks, *given* *that* *merge* *timing*.