[U-Boot] [PATCH] patman: Check commit_match before stripping leading whitespace
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@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
Hi Scott,
On 25 September 2014 13:30, Scott Wood scottwood@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@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 Noneif 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.
Regards, Simon
On Sun, 2014-09-28 at 12:04 -0600, Simon Glass wrote:
Hi Scott,
On 25 September 2014 13:30, Scott Wood scottwood@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@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 Noneif 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.
-Scott
Hi Scott,
On 29 September 2014 10:22, Scott Wood scottwood@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@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@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 Noneif 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.
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
to a commit and then run patman. You will see that the commit notes are not parsed.
Regards, Simon
On Tue, 2014-09-30 at 20:25 -0600, Simon Glass wrote:
Hi Scott,
On 29 September 2014 10:22, Scott Wood scottwood@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@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@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 Noneif 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.
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.
-Scott
Hi Scott,
On 2 October 2014 18:54, Scott Wood scottwood@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@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@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@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 Noneif 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@chromium.org
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
On 3 October 2014 20:40, Simon Glass sjg@chromium.org wrote:
Hi Scott,
On 2 October 2014 18:54, Scott Wood scottwood@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@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@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@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 Noneif 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@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
participants (2)
-
Scott Wood -
Simon Glass