Re: [Ksummit-discuss] bug-introducing patches

[Sasha Levin]

On Thu, May 03, 2018 at 07:20:39PM +0100, Al Viro wrote:
>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*.

Right, -stable shouldn't affect how/when you merge your patches in.

Also, in scenarios such as this we have enough tools in place to figure
out the dependency chain, and we could address it when backporting to
stable.

I don't want to create additional work that you wouldn't do anyways.
Keep in mind that the Fixes: tag will also be useful for yourself later
on if you have to go back to this patch a few years later.

As to the question you actually asked, assuming patch (3) is important 
for stable as well (you didn't state it explicitly), the correct 
tagging would be:

For patch (1):
	Fixes: AAA ("Commit from several years ago that introduced the hole")
	Cc: stable@xxxxxxxxxxxxxxx

For patch (2):
	No tags

For patch (3):
	Fixes: AAA ("Commit from several years ago that introduced the hole")
	Cc: stable@xxxxxxxxxxxxxxx # commit-id-of-(2)