linux-kernel - Re: [PATCH v1 1/1] docs: process: submitting-patches: Clarify the Reported-by usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yfj0lb50vS3ssrsn@hovoldconsulting.com>
Date:   Tue, 1 Feb 2022 09:51:33 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Matthew Wilcox <willy@...radead.org>, kbuild-all@...ts.01.org,
        Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Florian Eckert <fe@....tdt.de>
Subject: Re: [PATCH v1 1/1] docs: process: submitting-patches: Clarify the
 Reported-by usage

On Mon, Jan 31, 2022 at 08:16:42PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 31, 2022 at 05:47:32PM +0100, Johan Hovold wrote:
> > On Mon, Jan 31, 2022 at 05:34:35PM +0200, Andy Shevchenko wrote:
> > > On Mon, Jan 31, 2022 at 04:18:30PM +0100, Johan Hovold wrote:
> > > > On Fri, Jan 28, 2022 at 01:44:20PM +0000, Matthew Wilcox wrote:
> > 
> > > > > I think this misunderstands the problem that Andy is trying to fix.
> > > > > 
> > > > > The situation: I write a patch.  I post it for review.  A bot does
> > > > > something and finds a bug (could be compile-error, could be boot
> > > > > problem).  That bot sends a bug report with a suggestion to add
> > > > > Reported-by:.  That suggestion is inappropriate because the bug never
> > > > > made it upstream, so it looks like the bot reported the "problem"
> > > > > that the patch "fixes".
> > > > > 
> > > > > It's not unique to "new feature" patches.  If I'm fixing a bug and
> > > > > my fix also contains a bug spotted by a bot, adding Reported-by
> > > > > makes it look like the bot spotted the original bug, rather than
> > > > > spotting a bug in the fix.
> > > > > 
> > > > > The best thing to do in this case is nothing.  Do not credit the bot.
> > > > > Maybe add a Checked-by:, but that would be a new trailer and I really
> > > > > don't think we need a new kind of trailer to get wrong.
> > > > 
> > > > It seems like the only way to fix this is to fix the bots. Adding more
> > > > documentation is unlikely to help in this case.
> > > 
> > > Links to the documentation at least may clarify the point in case of a
> > > review.
> > 
> > Sure.
> > 
> > > > Can't we file a bug to whoever is running the bots (Intel?) and ask them
> > > > to remove the suggestion to add a Reported-by when the bot is testing a
> > > > patch (as opposed to mainline or even -next)?
> > > 
> > > The granularity here is not a repo. It's a code itself and in some cases
> > > it might be easy to distinguish new feature from the code modifications,
> > > but when code is already there and feature is just an extension of the
> > > existing file(s), it's hard to tell. And it might be true or not.
> > 
> > Not sure I understand what you're saying here. Perhaps you and Matthew
> > are talking about different things after all.
> 
> I'm talking about your suggestion to fix the bots. It's not easy.
> The problem is the same as Matthew explained.

Perhaps I'm missing something, but if you re-read Mathews description
above, it still seems to me like the issue is that the bots are trying
to claim credit for finding things that haven't been merged yet.

Your suggestion is to document that the bots should be ignored. My
suggestion is to fix the bots.
 
> > But for Matthew's issue, the case where the bots are testing posted
> > patches ("Thank you for the patch! Yet something to improve:) should be
> > easy to fix by simply dropping or rephrasing the "kindly add following
> > tag as appropriate" suggestion.
> 
> Yes, but this is not "fixing the bots", it falls into category "working around"
> them, because even for a clear bug report the suggestion can be stronger.
> And doing that properly without kinda AI not easy.

A patch has (typically) not been merged yet when the bots find problems,
but when testing a branch it's harder to tell (e.g. unless it's
mainline).

The bot already knows it is testing a patch, so the "please give me
credit" suggestion can be dropped or rephrased in that case. That's
fixing the bot. And no need for AI.

> > When testing merged code, it may be harder to tell whether the branch in
> > question can be rebased or not (and an incremental fix with a
> > reported-by tag is warranted).

Johan