Re: [PATCH v4 2/3] checkpatch: warn when Reported-by: is not followed by Link:

[Thorsten Leemhuis]

On 02.03.23 10:11, Dmitry Vyukov wrote:
> On Thu, 2 Mar 2023 at 10:04, Thorsten Leemhuis <linux@xxxxxxxxxxxxx> wrote:
>> On 02.03.23 09:27, Dmitry Vyukov wrote:
>>> On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>>>> On Thu, 2 Mar 2023 06:17:22 +0100 Thorsten Leemhuis wrote:
>>>>> On 02.03.23 05:46, Jakub Kicinski wrote:
>>>>>> On Fri, 20 Jan 2023 13:35:19 +0100 Thorsten Leemhuis wrote:
>>>>>>> Encourage patch authors to link to reports by issuing a warning, if
>>>>>>> a Reported-by: is not accompanied by a link to the report. Those links
>>>>>>> are often extremely useful for any code archaeologist that wants to know
>>>>>>> more about the backstory of a change than the commit message provides.
>>>>>>> That includes maintainers higher up in the patch-flow hierarchy, which
>>>>>>> is why Linus asks developers to add such links [1, 2, 3]. To quote [1]:
>>>>>>
>>>>>> Is it okay if we exclude syzbot reports from this rule?
>>>>>> If full syzbot report ID is provided - it's as good as a link.
>>>>>
>>>>> Hmmm. Not sure. Every special case makes things harder for humans and
>>>>> software that looks at a commits downstream. Clicking on a link also
>>>>> makes things easy for code archaeologists that might look into the issue
>>>>> months or years later (which might not even know how to find the report
>>>>> and potential discussions on lore from the syzbot report ID).
>>>>
>>>> No other system comes close to syzbot in terms of reporting meaningful
>>>> bugs, IMHO special casing it doesn't risk creep.
>>>>
>>>> Interestingly other bots attach links which are 100% pointless noise:
>>>>
>>>> Reported-by: Abaci Robot <abaci@xxxxxxxxxxxxxxxxx>
>>>> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4174
>>>>
>>>> Oh, eh. Let's see how noisy this check is once the merge window is over.
>>>>
>>>>> Hence, wouldn't it be better to ask the syzbot folks to change their
>>>>> reporting slightly and suggest something like this instead in their
>>>>> reports (the last line is the new one):
>>>>>
>>>>> ```
>>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>>>> Reported-by: syzbot+bba886ab504fcafecafe@xxxxxxxxxxxxxxxxxxxxxxxxx
>>>>> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@xxxxxxxxxx/
>>>>> ```
>>>>>
>>>>> This might not be to hard if they known the message-id in advance. Maybe
>>>>> they could even use the syzbot report ID as msg-id to make things even
>>>>> easier. And for developers not much would change afaics, they just need
>>>>> to copy and paste two lines instead of one.
>>>>
>>>> Dmitry, WDYT?
>>>
>>> Adding a Link to syzbot reports should be relatively trivial.
>>
>> Sounds good.
>>
>>> Ted proposed to use Link _instead_ of Reported-by:
>>> https://github.com/google/syzkaller/issues/3596
>>>> in fact, it might be nice if we could encourage upstream developers
>>>> put in the commit trailer:
>>>> Link: https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>>>> in addition to, or better yet, instead of:
>>>> Reported-by: syzbot+15cd994e273307bf5cfa@xxxxxxxxxxxxxxxxxxxxxxxxx
>>>
>>> We could also use a link in the Reported-by tag, e.g.:
>>>
>>> Reported-by: https://syzkaller.appspot.com/b/5266d464285a03cee9db
>>>
>>> Some folks parse Reported-by to collect stats.
>>>
>>> What is better?
>>
>> Here are my thoughts:
>>
>> * we should definitely have a "Link:" to the report in lore, as that's
>> the long-term archive under our own control and also where discussions
>> happen after the report was posted; but I'm biased here, as such a tag
>> would make tracking with regzbot a no-brainer ;)
>>
>> * "Reported-by:" IMHO should stay for the hat tip and stats aspects; I
>> don't care if it includes the syzbot report ID or not (omitting it might
>> be good for the stats aspects and is more friendly to the eyes, but
>> those are just details)
>>
>> * a Link: to the syzkaller web ui might be nice, too -- and likely is
>> the easiest thing to look out for on the syzbot server side
>>
>> IOW something like this maybe:
>>
>> Reported-by: syzbot+cafecafecaca0cafecafe@xxxxxxxxxxxxxxxxxxxxxxxxx
>> Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@xxxxxxxxxx/
>> Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe
>>
>> Something like the following would look more normal, but of course is
>> only possible if syzbot starts out to look for such Link: tags (not sure
>> if the msgid is valid here, but you get the idea):
>>
>> Reported-by: syzbot@xxxxxxxxxxxxxxxxxxxxxxxxx
>> Link:
>> https://lore.kernel.org/r/syzbot+cafecafecaca0cafecafe-syzkaller-appspotmail-com@xxxxxxxxxx/
> 
> Oh, you mean lore link.
> 
> We can parse out our hash from any tag, but the problem is that the
> current email api we use, does not allow to specify Message-ID before
> sending, so we don't know it when generating the text.
> We don't even know it after sending, the API is super simple:
> https://pkg.go.dev/google.golang.org/appengine/mail
> So we don't know what the lore link will be...

That's... unfortunate, as from my understanding of things that would be
the most important "Link:" to have in any patches that fix issues report
by syzbot. But well, that's how it is for now. In that case I'd vote for
this:

Reported-by: syzbot@xxxxxxxxxxxxxxxxxxxxxxxxx
Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe

Regzbot can handle this, as long as somebody tells it about that URL.
IOW: it creates a little extra work for some human. But that is not much
of a problem, especially as of now, as I only track syzbot reports that
for one reason or another make me go "I should better track this".

Ciao, Thorsten