b4 inserts Reviewed-by tag from unrelated thread/patch

b4 inserts Reviewed-by tag from unrelated thread/patch

[Andrew Cooper]

Hello,

This is partly my fault for not double checking well enough, but it
turns out I've committed a false Reviewed-by tag.  (Anthony does
typically review patches of this nature, so I thought nothing of it when
b4 told me that he'd reviewed one of the patches.)

I've minimised a repro to:

b4 shazam
cover.7da1777882774486a13e6f39ff4a2096f6b7901e.1744028549.git-series.marmarek@invisiblethingslab.com
-P4

wherein the analysis somehow decides that "Re: [PATCH 07/12] libxl:
Allow stubdomain to control interupts of PCI device" is related, and has
inserted the R-by tag from that thread.

Unfortunately, I can't make much sense of what -d is telling me there. 
This is with b4 master.

~Andrew

Re: b4 inserts Reviewed-by tag from unrelated thread/patch

[Konstantin Ryabitsev]

On Tue, Apr 08, 2025 at 02:18:42PM +0100, Andrew Cooper wrote:
> Hello,
> 
> This is partly my fault for not double checking well enough, but it
> turns out I've committed a false Reviewed-by tag.  (Anthony does
> typically review patches of this nature, so I thought nothing of it when
> b4 told me that he'd reviewed one of the patches.)
> 
> I've minimised a repro to:
> 
> b4 shazam
> cover.7da1777882774486a13e6f39ff4a2096f6b7901e.1744028549.git-series.marmarek@invisiblethingslab.com
> -P4

Thank you, this is very helpful.

> wherein the analysis somehow decides that "Re: [PATCH 07/12] libxl:
> Allow stubdomain to control interupts of PCI device" is related, and has
> inserted the R-by tag from that thread.

This triggers a bug that I suspect has to do with a "/" in one of the
message-id's. Basically:

- [PATCH 4/12] in your series is [PATCH 1/12] in that other series, which is
  how this other series is getting pulled in.

- Antony sent a reviewed-by to a different patch in that other series (7/12),
  but when b4 walks up the references, it somehow messes up and misattributes
  that reviewed-by as if it was sent to the cover letter, not just to 7/12.
  *This is the bug that I need to fix.*

- When a tag is sent to a cover, we propagate it to all patches in that
  series, which is how your patch 4/12 gets Anthony's reviewed-by.

I'll work on getting this fixed.

-K

bugspray tag me

Re: b4 inserts Reviewed-by tag from unrelated thread/patch

[Bugspray Bot]

Hello:

This conversation is now tracked by Kernel.org Bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=219993

There is no need to do anything else, just keep talking.
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (bugspray 0.1-dev)

Re: b4 inserts Reviewed-by tag from unrelated thread/patch

[Bugspray Bot]

Konstantin Ryabitsev writes in commit 048adc880245925f7a62639fc0cc58c9e086b3f1:

Fix trailer attribution bug due to incorrect thread traversing

When we were walking the thread looking for where the trailer belongs,
we were incorrectly attributing trailers sent to a single patch as if it
were sent to the cover letter. This only happened in the following
situation

 [PATCH 0/X] Cover Letter
 |
 |---> [PATCH Y/X] Patch Y
 |     (patch being reviewed)
 |
 |-----> Re: [PATCH Y/X] Patch Y
 |       (follow-up that *doesn't* contain a trailer)
 |
 |-------> Re: [PATCH Y/X] Patch Y
 |         (follow-up that *does* contains a trailer)

In this case we had a bug in traversing the references where we first
looked at the immediate parent, and then straight at the top of the
thread, becaues the references were not reversed when we had parsed
them.

This patch fixes both the incorrect order of references AND it doesn't
trust them to be in the correct order -- it will now always rely on the
in-reply-to header to figure out what the parent of the message is.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=219993
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

(via https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=048adc880245)
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (bugspray 0.1-dev)