Bug in b4 shazam?

Bug in b4 shazam?

[Ricardo Ribalda]

Hi Konstantin

Our CI bot has encounter some issues with this series:
93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl

it complains that the patch is corrupted:
error: corrupt patch at line 27

But it looks like b4 generated the corruption.



b4 mbox 93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl' gives a valid
patch as well.

But 'b4 shazam' messes up the patch: it deletes the empty line before
the '--' at the
end of the patch:

I.e., after running b4 shazam I run: 'git am
--show-current-patch=diff' to see what the
patch is it tries to apply, and that gives me:

 .. _v4l2-alpha-component:
--
2.45.2

instead of:

 .. _v4l2-alpha-component:

--
2.45.2

Is this a known bug?

PS: kudos to Hans to debug the root cause

-- 
Ricardo Ribalda
Software Engineer
ribalda@google.com

Re: Bug in b4 shazam?

[Konstantin Ryabitsev]

On Thu, Oct 31, 2024 at 03:06:56PM +0100, Ricardo Ribalda wrote:
> Hi Konstantin
> 
> Our CI bot has encounter some issues with this series:
> 93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl
> 
> it complains that the patch is corrupted:
> error: corrupt patch at line 27
> 
> But it looks like b4 generated the corruption.

It's a bit more complicated. It looks like the patch was copy-pasted into
Thunderbird, which is actually what introduced corruption. A well-formatted
patch has no blank lines in it -- the line has to either start with "+", "-",
or " ". If you view that commit with "git show --format=email", you will see
that both lines between .. +v4l2-alpha-component: have a space character
(replaced here with _ for clarity):

    +    necessary for hardware to work. This control is required for stateful
    +    encoders.
    _
    _.. _v4l2-alpha-component:
    _

However, when that patch was copy-pasted into Thunderbird, these blank spaces
were trimmed so, strictly speaking, this is no longer a well-formatted patch.

That said, git deals with it correctly, so I'll see if we can recover from
this corruption as well -- or at least not make it worse.

bugspray tag me

-K

Re: Bug in b4 shazam?

[Hans Verkuil]

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

On 31/10/2024 17:38, Konstantin Ryabitsev wrote:
> On Thu, Oct 31, 2024 at 03:06:56PM +0100, Ricardo Ribalda wrote:
>> Hi Konstantin
>>
>> Our CI bot has encounter some issues with this series:
>> 93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl
>>
>> it complains that the patch is corrupted:
>> error: corrupt patch at line 27
>>
>> But it looks like b4 generated the corruption.
> 
> It's a bit more complicated. It looks like the patch was copy-pasted into
> Thunderbird, which is actually what introduced corruption. A well-formatted
> patch has no blank lines in it -- the line has to either start with "+", "-",
> or " ". If you view that commit with "git show --format=email", you will see
> that both lines between .. +v4l2-alpha-component: have a space character
> (replaced here with _ for clarity):
> 
>     +    necessary for hardware to work. This control is required for stateful
>     +    encoders.
>     _
>     _.. _v4l2-alpha-component:
>     _
> 
> However, when that patch was copy-pasted into Thunderbird, these blank spaces
> were trimmed so, strictly speaking, this is no longer a well-formatted patch.
> 
> That said, git deals with it correctly, so I'll see if we can recover from
> this corruption as well -- or at least not make it worse.

It is true that the patch was copy-and-pasted in Thunderbird.

However, the patch itself as formatted by 'git format-patch' *also* has no spaces.
I've attached the git format-patch output to this email.

So the patch as pasted in Thunderbird is consistent with what git format-patch
gave me.

I think completely empty lines are handled a bit differently by diff and do not need
a space at the beginning.

Regards,

	Hans

> 
> bugspray tag me
> 
> -K

[-- Attachment #2: 0001-Documentation-media-improve-V4L2_CID_MIN_BUFFERS_FOR.patch --]
[-- Type: text/x-patch, Size: 1708 bytes --]

From 3897c9e39976c4c4d4d27d992917969aef211670 Mon Sep 17 00:00:00 2001
Message-ID: <3897c9e39976c4c4d4d27d992917969aef211670.1730393669.git.hverkuil@xs4all.nl>
From: Hans Verkuil <hverkuil@xs4all.nl>
Date: Wed, 30 Oct 2024 11:03:58 +0100
Subject: [PATCH 1/4] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*
 doc

Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are for stateful
codecs only.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
index 57893814a1e5..9253cc946f02 100644
--- a/Documentation/userspace-api/media/v4l/control.rst
+++ b/Documentation/userspace-api/media/v4l/control.rst
@@ -290,13 +290,15 @@ Control IDs
     This is a read-only control that can be read by the application and
     used as a hint to determine the number of CAPTURE buffers to pass to
     REQBUFS. The value is the minimum number of CAPTURE buffers that is
-    necessary for hardware to work.
+    necessary for hardware to work. This control is required for stateful
+    decoders.
 
 ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
     This is a read-only control that can be read by the application and
     used as a hint to determine the number of OUTPUT buffers to pass to
     REQBUFS. The value is the minimum number of OUTPUT buffers that is
-    necessary for hardware to work.
+    necessary for hardware to work. This control is required for stateful
+    encoders.
 
 .. _v4l2-alpha-component:
 
-- 
2.45.2

Re: Bug in b4 shazam?

[Hans Verkuil]

On 31/10/2024 17:59, Hans Verkuil wrote:
> On 31/10/2024 17:38, Konstantin Ryabitsev wrote:
>> On Thu, Oct 31, 2024 at 03:06:56PM +0100, Ricardo Ribalda wrote:
>>> Hi Konstantin
>>>
>>> Our CI bot has encounter some issues with this series:
>>> 93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl
>>>
>>> it complains that the patch is corrupted:
>>> error: corrupt patch at line 27
>>>
>>> But it looks like b4 generated the corruption.
>>
>> It's a bit more complicated. It looks like the patch was copy-pasted into
>> Thunderbird, which is actually what introduced corruption. A well-formatted
>> patch has no blank lines in it -- the line has to either start with "+", "-",
>> or " ". If you view that commit with "git show --format=email", you will see
>> that both lines between .. +v4l2-alpha-component: have a space character
>> (replaced here with _ for clarity):
>>
>>     +    necessary for hardware to work. This control is required for stateful
>>     +    encoders.
>>     _
>>     _.. _v4l2-alpha-component:
>>     _
>>
>> However, when that patch was copy-pasted into Thunderbird, these blank spaces
>> were trimmed so, strictly speaking, this is no longer a well-formatted patch.
>>
>> That said, git deals with it correctly, so I'll see if we can recover from
>> this corruption as well -- or at least not make it worse.
> 
> It is true that the patch was copy-and-pasted in Thunderbird.
> 
> However, the patch itself as formatted by 'git format-patch' *also* has no spaces.
> I've attached the git format-patch output to this email.
> 
> So the patch as pasted in Thunderbird is consistent with what git format-patch
> gave me.
> 
> I think completely empty lines are handled a bit differently by diff and do not need
> a space at the beginning.

Found it:

https://www.gnu.org/software/diffutils/manual/html_node/Trailing-Blanks.html

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>>
>> bugspray tag me
>>
>> -K

Re: Bug in b4 shazam?

[Konstantin Ryabitsev]

On Thu, Oct 31, 2024 at 05:59:52PM +0100, Hans Verkuil wrote:
> However, the patch itself as formatted by 'git format-patch' *also* has no spaces.
> I've attached the git format-patch output to this email.

OK, I see -- using "git show --format=email" definitely shows me the spaces.

bugspray track

-K

Re: Bug in b4 shazam?

[Bugspray Bot]

Hello:

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

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: Bug in b4 shazam?

[Bugspray Bot]

Konstantin Ryabitsev writes in commit 355e82083f0eba59abf31521977dda09fab8bff5:

Don't strip trailing blank lines in patches

When parsing patches, don't rstrip blank newlines in case it's a
(slightly) malformed but a valid patch that strips the leading space in
diffs.

Reported-by: Ricardo Ribalda <ribalda@google.com>
Link: https://msgid.link/CANiDSCvSAVkSsdWzfGFHK2YZXy_dOHVeJOA7V0pmyzHzSm6iEw@mail.gmail.com
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219449
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

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