diff mbox series

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

Message ID bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info
State New
Headers
Series checkpatch.pl: warn about discouraged tags and missing Link: tags |

Commit Message

Thorsten Leemhuis Jan. 20, 2023, 12:35 p.m. UTC 
  From: Kai Wasserbäch <kai@dev.carbon-project.org>

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]:

> Again, the commit has a link to the patch *submission*, which is
> almost entirely useless. There's no link to the actual problem the
> patch fixes.
>
> [...]
>
> Put another way: I can see that
>
> Reported-by: Zhangfei Gao <zhangfei.gao@foxmail.com>
>
> in the commit, but I don't have a clue what the actual report was, and
> there really isn't enough information in the commit itself, except for
> a fairly handwavy "Device drivers might, for instance, still need to
> flush operations.."
>
> I don't want to know what device drivers _might_ do. I would want to
> have an actual pointer to what they do and where.

Another reason why these links are wanted: the ongoing regression
tracking efforts can only scale with them, as they allow the regression
tracking bot 'regzbot' to automatically connect tracked reports with
patches that are posted or committed to fix tracked regressions.

Link: https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/ [1]
Link: https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/ [2]
Link: https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/ [3]
Signed-off-by: Kai Wasserbäch <kai@dev.carbon-project.org>
Co-developed-by: Thorsten Leemhuis <linux@leemhuis.info>
Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
---
 scripts/checkpatch.pl | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jakub Kicinski March 2, 2023, 4:46 a.m. UTC | #1 
On Fri, 20 Jan 2023 13:35:19 +0100 Thorsten Leemhuis wrote:
> From: Kai Wasserbäch <kai@dev.carbon-project.org>
> 
> 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. 
And regression tracking doesn't seem to happen much on syzbot 
reports either.

I like the addition otherwise, it's already catching missing links 
in netdev land!
Thorsten Leemhuis March 2, 2023, 5:17 a.m. UTC | #2 
On 02.03.23 05:46, Jakub Kicinski wrote:
> On Fri, 20 Jan 2023 13:35:19 +0100 Thorsten Leemhuis wrote:
>> From: Kai Wasserbäch <kai@dev.carbon-project.org>
>>
>> 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).

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@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
```

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.

> And regression tracking doesn't seem to happen much on syzbot 
> reports either.

Yeah, right now I most of the time stay away from CI reports and leave
the tracking to the people that run the CI (unless it's something I
consider worth tracking), but I hope that might change over time to have
things in one place.

> I like the addition otherwise, it's already catching missing links 
> in netdev land!

Thx for saying this!

Ciao, Thorsten
Jakub Kicinski March 2, 2023, 5:40 a.m. UTC | #3 
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@linux.alibaba.com>
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@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
> ```
> 
> 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?
Dmitry Vyukov March 2, 2023, 8:27 a.m. UTC | #4 
On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@kernel.org> 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@linux.alibaba.com>
> 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@syzkaller.appspotmail.com
> > Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
> > ```
> >
> > 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?

Hi Jakub, Thorsten,

Adding a Link to syzbot reports should be relatively trivial.

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@syzkaller.appspotmail.com

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?
Thorsten Leemhuis March 2, 2023, 9:04 a.m. UTC | #5 
On 02.03.23 09:27, Dmitry Vyukov wrote:
> On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@kernel.org> 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@linux.alibaba.com>
>> 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@syzkaller.appspotmail.com
>>> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
>>> ```
>>>
>>> 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@syzkaller.appspotmail.com
> 
> 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@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/
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@syzkaller.appspotmail.com
Link:
https://lore.kernel.org/r/syzbot+cafecafecaca0cafecafe-syzkaller-appspotmail-com@google.com/

Ciao, Thorsten
Dmitry Vyukov March 2, 2023, 9:11 a.m. UTC | #6 
On Thu, 2 Mar 2023 at 10:04, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 02.03.23 09:27, Dmitry Vyukov wrote:
> > On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@kernel.org> 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@linux.alibaba.com>
> >> 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@syzkaller.appspotmail.com
> >>> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
> >>> ```
> >>>
> >>> 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@syzkaller.appspotmail.com
> >
> > 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@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/
> 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@syzkaller.appspotmail.com
> Link:
> https://lore.kernel.org/r/syzbot+cafecafecaca0cafecafe-syzkaller-appspotmail-com@google.com/

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...
Thorsten Leemhuis March 2, 2023, 9:48 a.m. UTC | #7 
On 02.03.23 10:11, Dmitry Vyukov wrote:
> On Thu, 2 Mar 2023 at 10:04, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> On 02.03.23 09:27, Dmitry Vyukov wrote:
>>> On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@kernel.org> 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@linux.alibaba.com>
>>>> 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@syzkaller.appspotmail.com
>>>>> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
>>>>> ```
>>>>>
>>>>> 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@syzkaller.appspotmail.com
>>>
>>> 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@syzkaller.appspotmail.com
>> Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/
>> 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@syzkaller.appspotmail.com
>> Link:
>> https://lore.kernel.org/r/syzbot+cafecafecaca0cafecafe-syzkaller-appspotmail-com@google.com/
> 
> 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@syzkaller.appspotmail.com
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
Andrew Morton March 3, 2023, 2:10 a.m. UTC | #8 
On Thu, 2 Mar 2023 10:48:22 +0100 Thorsten Leemhuis <linux@leemhuis.info> wrote:

> > 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@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe

As you previously mentioned, patch preparers should also include
the lore link so any followup discussion is easily located.
Dmitry Vyukov March 6, 2023, 8:53 a.m. UTC | #9 
On Fri, 3 Mar 2023 at 03:10, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 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@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe
>
> As you previously mentioned, patch preparers should also include
> the lore link so any followup discussion is easily located.

If the link we need to include is to lore, then we don't need to
change the current syzbot Reported-by, right? Instead of asking 3
tags, we can ask only for:

Reported-by: syzbot+df61b36319e045c00a08@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/

But as I mentioned we can't provide the lore link at the moment, we
can only add a text to ask to include it.

This also means that checkpatch does not need special casing for syzbot.
Thorsten Leemhuis March 7, 2023, 11:36 a.m. UTC | #10 
On 06.03.23 09:53, Dmitry Vyukov wrote:
> On Fri, 3 Mar 2023 at 03:10, Andrew Morton <akpm@linux-foundation.org> wrote:
>>>> 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@syzkaller.appspotmail.com
>>> Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe
>>
>> As you previously mentioned, patch preparers should also include
>> the lore link so any followup discussion is easily located.
> 
> If the link we need to include is to lore, then we don't need to
> change the current syzbot Reported-by, right? Instead of asking 3
> tags, we can ask only for:
> 
> Reported-by: syzbot+df61b36319e045c00a08@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/

Yeah, that's not perfect (see below), but at least better. As mentioned
earlier: if the Reported-by: includes the sysbot-id (e.g. the
df61b36319e045c00a08) is up to you.

> But as I mentioned we can't provide the lore link at the moment, we
> can only add a text to ask to include it.

Yeah, that would be good. Normally it's the oblation of the developer
anyway to add Link: tags to any report (which most of the time means: in
lore) when fixing things. Obviously the chance that they actually do it
is a lot bigger when syzbot would suggest it.

> This also means that checkpatch does not need special casing for syzbot.

Yup

Ciao, Thorsten
diff mbox series

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d739ce0909b1..b74d6002f773 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3155,8 +3155,20 @@  sub process {
 					     "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
 				}
 			}
+
+# check if Reported-by: is followed by a Link:
+			if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
+				if (!defined $lines[$linenr]) {
+					WARN("BAD_REPORTED_BY_LINK",
+					     "Reported-by: should be immediately followed by Link: to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+				} elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
+					WARN("BAD_REPORTED_BY_LINK",
+					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+				}
+			}
 		}
 
+
 # Check Fixes: styles is correct
 		if (!$in_header_lines &&
 		    $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {