[U-Boot] [PATCH] patman: Check commit_match before stripping leading whitespace

[Simon Glass]

On 3 October 2014 20:40, Simon Glass <sjg at chromium.org> wrote:
> Hi Scott,
>
> On 2 October 2014 18:54, Scott Wood <scottwood at freescale.com> wrote:
>> On Tue, 2014-09-30 at 20:25 -0600, Simon Glass wrote:
>>> Hi Scott,
>>>
>>> On 29 September 2014 10:22, Scott Wood <scottwood at freescale.com> wrote:
>>> > On Sun, 2014-09-28 at 12:04 -0600, Simon Glass wrote:
>>> >> Hi Scott,
>>> >>
>>> >> On 25 September 2014 13:30, Scott Wood <scottwood at freescale.com> wrote:
>>> >> > True commit lines start at column zero.  Anything that is indented
>>> >> > is part of the commit message instead.  I noticed this by trying to
>>> >> > run buildman with commit e3a4facdfc07179ebe017a07b8de6224a935a9f3
>>> >> > as master, which contained a reference to a Linux commit inside
>>> >> > the commit message.  ProcessLine saw that as a genuite commit
>>> >> > line, and thus buildman tried to build it, and died with an
>>> >> > exception because that SHA is not present in the U-Boot tree.
>>> >> >
>>> >> > Signed-off-by: Scott Wood <scottwood at freescale.com>
>>> >> > ---
>>> >> >  tools/patman/patchstream.py | 4 +++-
>>> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
>>> >> >
>>> >> > diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
>>> >> > index d630157..68e98b9 100644
>>> >> > --- a/tools/patman/patchstream.py
>>> >> > +++ b/tools/patman/patchstream.py
>>> >> > @@ -139,6 +139,9 @@ class PatchStream:
>>> >> >          # Initially we have no output. Prepare the input line string
>>> >> >          out = []
>>> >> >          line = line.rstrip('\n')
>>> >> > +
>>> >> > +        commit_match = re_commit.match(line) if self.is_log else None
>>> >> > +
>>> >> >          if self.is_log:
>>> >> >              if line[:4] == '    ':
>>> >> >                  line = line[4:]
>>> >> > @@ -146,7 +149,6 @@ class PatchStream:
>>> >> >          # Handle state transition and skipping blank lines
>>> >> >          series_tag_match = re_series_tag.match(line)
>>> >> >          commit_tag_match = re_commit_tag.match(line)
>>> >> > -        commit_match = re_commit.match(line) if self.is_log else None
>>> >> >          cover_cc_match = re_cover_cc.match(line)
>>> >> >          signoff_match = re_signoff.match(line)
>>> >> >          tag_match = None
>>> >> > --
>>> >> > 1.9.1
>>> >> >
>>> >>
>>> >> Thanks for finding this bug.
>>> >>
>>> >> This could use a test in tools/patman/test.py
>>> >>
>>> >> The problem is that you are breaking the patch-processing part of this
>>> >> code. It operates in two modes - see the comment at the top of
>>> >> ProcessLine(). With your change it will not process patches correctly,
>>> >> e.g. to add Commit-notes: to the patch.
>>> >
>>> > How would this patch would make any difference in the "self.is_log ==
>>> > False" case?  The whitespace removal that this patch reorders around
>>> > won't be executed, and commit_match will be None regardless.
>>>
>>> You are changing it so that commit_match will always be None for patches.
>>
>> If by "for patches" you mean when self.is_log == False, then it was
>> always None.
>>
>>> This means that the commit message in a patch will never be found, so
>>> the state machine that tracks where it is in the patch will not
>>> function.
>>>
>>> Try adding something like:
>>>
>>> Commit-notes:
>>> this is a note
>>> more stuff
>>> END
>>
>> Commit-notes is detected by commit_tag_match, not commit_match.
>>
>>> to a commit and then run patman. You will see that the commit notes
>>> are not parsed.
>>
>> I just tried it and the commit notes work fine.
>
> Sorry you are right, I must have typed it incorrectly when I tried it.
> I just tried it again and it is fine.
>
> Acked-by: Simon Glass <sjg at chromium.org>

Applied to u-boot-x86/patman, thanks!

>
>>
>> BTW, tools/patman/README says "For most cases of using patman for U-Boot
>> development, patman will locate and use the file 'doc/git-mailrc' in
>> your U-Boot directory. This contains most of the aliases you will need."
>> But, this did not work -- patman threw an exception at me saying alias
>> not found -- until I ran "git config sendemail.aliasesfile
>> doc/git-mailrc" which the README doesn't mention.
>
> OK thanks the report, I sent a patch.
>
> Regards,
> Simon