b4 trailers --update picks up bogus trailers

b4 trailers --update picks up bogus trailers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

Hi,

With current git b4 if I run b4 trailers --update on

   https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#t

patch 2 picks up reviewed-by trailers which were sent in reply to the
prior versions of the series even though that patch is new in this
version so the update is clearly bogus.

Thanks,
Mark

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: b4 trailers --update picks up bogus trailers

[Konstantin Ryabitsev]

On Wed, Oct 02, 2024 at 12:51:50PM GMT, Mark Brown wrote:
> Hi,
> 
> With current git b4 if I run b4 trailers --update on
> 
>    https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#t
> 
> patch 2 picks up reviewed-by trailers which were sent in reply to the
> prior versions of the series even though that patch is new in this
> version so the update is clearly bogus.

When I try it, I see the following updates that it wants to pull in:

	Finding code-review trailers for 41 commits...
	Checking change-id "20230303-arm64-gcs-e311ab0d8729"
	Grabbing search results from lore.kernel.org
	---
	  + Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
		https://lore.kernel.org/all/878r3f99o1.fsf@linaro.org
	  + Reviewed-by: Deepak Gupta <debug@rivosinc.com>
		https://lore.kernel.org/all/ZvyCC7tJT7QoKO%2BD@debug.ba.rivosinc.com
	  + Acked-by: Yury Khrustalev <yury.khrustalev@arm.com>
		https://lore.kernel.org/all/ZtbQPBSl6Ym7g4QE@arm.com
	  + Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
		https://lore.kernel.org/all/ZschddrfehjVUMXw@arm.com

Are these different from what you see?

-K

Re: b4 trailers --update picks up bogus trailers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

On Wed, Oct 02, 2024 at 09:34:20AM -0400, Konstantin Ryabitsev wrote:

> When I try it, I see the following updates that it wants to pull in:

> 	Finding code-review trailers for 41 commits...
> 	Checking change-id "20230303-arm64-gcs-e311ab0d8729"
> 	Grabbing search results from lore.kernel.org
> 	---
> 	  + Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 		https://lore.kernel.org/all/878r3f99o1.fsf@linaro.org
> 	  + Reviewed-by: Deepak Gupta <debug@rivosinc.com>
> 		https://lore.kernel.org/all/ZvyCC7tJT7QoKO%2BD@debug.ba.rivosinc.com
> 	  + Acked-by: Yury Khrustalev <yury.khrustalev@arm.com>
> 		https://lore.kernel.org/all/ZtbQPBSl6Ym7g4QE@arm.com
> 	  + Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 		https://lore.kernel.org/all/ZschddrfehjVUMXw@arm.com

> Are these different from what you see?

That's what I see.  The biggest immediate problem is that the R-b from
Thiago was sent against v8 and then gets added to patch 2:

  mm: Define VM_HIGH_ARCH_6
    + Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org> (✗ DKIM/linaro.org)

which was not present in that version so the tagging is clearly
inappropriate.

Other than Deepak's R-b the tags were all for old versions and one way
or another were deliberately removed or are being applied to newly added
patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: b4 trailers --update picks up bogus trailers

[Konstantin Ryabitsev]

On Wed, Oct 02, 2024 at 02:52:02PM GMT, Mark Brown wrote:
> > 	Finding code-review trailers for 41 commits...
> > 	Checking change-id "20230303-arm64-gcs-e311ab0d8729"
> > 	Grabbing search results from lore.kernel.org
> > 	---
> > 	  + Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > 		https://lore.kernel.org/all/878r3f99o1.fsf@linaro.org
> > 	  + Reviewed-by: Deepak Gupta <debug@rivosinc.com>
> > 		https://lore.kernel.org/all/ZvyCC7tJT7QoKO%2BD@debug.ba.rivosinc.com
> > 	  + Acked-by: Yury Khrustalev <yury.khrustalev@arm.com>
> > 		https://lore.kernel.org/all/ZtbQPBSl6Ym7g4QE@arm.com
> > 	  + Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > 		https://lore.kernel.org/all/ZschddrfehjVUMXw@arm.com
> 
> > Are these different from what you see?
> 
> That's what I see.  The biggest immediate problem is that the R-b from
> Thiago was sent against v8 and then gets added to patch 2:
> 
>   mm: Define VM_HIGH_ARCH_6
>     + Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org> (✗ DKIM/linaro.org)
> 
> which was not present in that version so the tagging is clearly
> inappropriate.

Yes, you're right -- there's a logic bug that causes trailers sent to previous
revision cover letters to be applied to all patches in the newer revisions.
I'll work on a fix so it respects patch-id's.

bugspray tag me

-K

Re: b4 trailers --update picks up bogus trailers

[Bugspray Bot]

Hello:

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

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 trailers --update picks up bogus trailers

[Konstantin Ryabitsev]

On Wed, Oct 02, 2024 at 12:51:50PM GMT, Mark Brown wrote:
> With current git b4 if I run b4 trailers --update on
> 
>    https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#t
> 
> patch 2 picks up reviewed-by trailers which were sent in reply to the
> prior versions of the series even though that patch is new in this
> version so the update is clearly bogus.

Okay, this was a bit hairier than I was expecting, but we now do a proper
patch-id match for all trailers we have received and only apply them to the
patches that have not changed between what the reviewer tagged and what is in
the current tree.

Please try it out in the latest master.

-K

Re: b4 trailers --update picks up bogus trailers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

On Thu, Oct 03, 2024 at 11:13:03AM -0400, Konstantin Ryabitsev wrote:
> On Wed, Oct 02, 2024 at 12:51:50PM GMT, Mark Brown wrote:
> > With current git b4 if I run b4 trailers --update on

> >    https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#t

> > patch 2 picks up reviewed-by trailers which were sent in reply to the
> > prior versions of the series even though that patch is new in this
> > version so the update is clearly bogus.

> Okay, this was a bit hairier than I was expecting, but we now do a proper
> patch-id match for all trailers we have received and only apply them to the
> patches that have not changed between what the reviewer tagged and what is in
> the current tree.

> Please try it out in the latest master.

That seems to be working well, thanks - it excludes the surprising tags.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: b4 trailers --update picks up bogus trailers

[Bugspray Bot]

Konstantin Ryabitsev writes in commit 9b746db767927ead0cc10b54b668f7bf72bb4eae:

ez: do a patch-id match when pulling in trailer updates

Address two important limitations of trailers -u:

- trailer updates were applied even if the local patch has changed
- trailers sent to the cover letter were applied to all patches in the
current series, even if they were sent to a previous revision and the
commits no longer matched

Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219342
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

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