Skip to contentSkip to conversation

patman: Deal with 'git apply' failures correctly

PATCH Superseded Simon Glass 2 participants 4 messages Mar 22, 2014 – Apr 20, 2014

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 patman: Deal with 'git apply' failures correctly

This sort of failure is rare, but the code to deal with it is wrong. Fix it.

---

 tools/patman/gitutil.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/patman/gitutil.py +4 -2
index 5dcbaa3..3ea256d 100644
@@ -11,6 +11,7 @@ import subprocess
 import sys
 import terminal
 
+import checkpatch
 import settings
 
 
@@ -193,6 +194,7 @@ def ApplyPatch(verbose, fname):
     Args:
         fname: filename of patch file to apply
     """
+    col = terminal.Color()
     cmd = ['git', 'am', fname]
     pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE,
             stderr=subprocess.PIPE)
@@ -203,8 +205,8 @@ def ApplyPatch(verbose, fname):
             print line
         match = re_error.match(line)
         if match:
-            print GetWarningMsg('warning', match.group(1), int(match.group(2)),
-                    'Patch failed')
+            print checkpatch.GetWarningMsg(col, 'warning', match.group(1),
+                                           int(match.group(2)), 'Patch failed')
     return pipe.returncode == 0, stdout
 
 def ApplyPatches(verbose, args, start_point):
Signature · 1 line
1.9.1.423.g4596e3a
#1 lore raw

PATCH 2/2 patman: Avoid duplicate sign-offs

Collect all Signed-off-by tags in a commit and make sure that there are no duplicates.

---

 tools/patman/README         |  1 +
 tools/patman/commit.py      | 13 +++++++++++++
 tools/patman/patchstream.py | 10 ++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)
tools/patman/README +1 -0
index 59f1776..119c554 100644
@@ -192,6 +192,7 @@ END
 	A sign-off is added automatically to your patches (this is
 	probably a bug). If you put this tag in your patches, it will
 	override the default signoff that patman automatically adds.
+	Multiple duplicate signoffs will be removed.
 
  Tested-by: Their Name <email>
  Reviewed-by: Their Name <email>
tools/patman/commit.py +13 -0
index 89cce7f..6cc5a41 100644
@@ -29,6 +29,7 @@ class Commit:
         self.tags = []
         self.changes = {}
         self.cc_list = []
+        self.signoff_set = set()
         self.notes = []
 
     def AddChange(self, version, info):
@@ -72,3 +73,15 @@ class Commit:
             cc_list:    List of aliases or email addresses
         """
         self.cc_list += cc_list
+
+    def AddSignoff(self, signoff):
+        """Add a list of signoffs when we send this patch.
+
+        Args:
+            signoff:    Signoff line
+        """
+        self.signoff_set.add(signoff)
+
+    def FormatSignoffs(self):
+        """Return a formatted list of signoffs for this commit."""
+        return ['Signed-off-by: ' + s for s in self.signoff_set]
tools/patman/patchstream.py +8 -2
index 684204c..32ebc35 100644
@@ -21,7 +21,7 @@ re_remove = re.compile('^BUG=|^TEST=|^BRANCH=|^Change-Id:|^Review URL:'
 re_allowed_after_test = re.compile('^Signed-off-by:')
 
 # Signoffs
-re_signoff = re.compile('^Signed-off-by:')
+re_signoff = re.compile('^Signed-off-by: *(.*)')
 
 # The start of the cover letter
 re_cover = re.compile('^Cover-letter:')
@@ -159,6 +159,7 @@ class PatchStream:
         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
         if self.state == STATE_PATCH_HEADER:
             tag_match = re_tag.match(line)
@@ -223,7 +224,7 @@ class PatchStream:
             if is_blank:
                 # Blank line ends this change list
                 self.in_change = 0
-            elif line == '---' or re_signoff.match(line):
+            elif line == '---':
                 self.in_change = 0
                 out = self.ProcessLine(line)
             else:
@@ -272,6 +273,10 @@ class PatchStream:
             else:
                 self.tags.append(line);
 
+        # Collect signoffs to add later
+        elif signoff_match:
+            self.commit.AddSignoff(signoff_match.group(1))
+
         # Well that means this is an ordinary line
         else:
             pos = 1
@@ -305,6 +310,7 @@ class PatchStream:
                 out = []
                 log = self.series.MakeChangeLog(self.commit)
                 out += self.FormatTags(self.tags)
+                out += self.commit.FormatSignoffs()
                 out += [line] + self.commit.notes + [''] + log
             elif self.found_test:
                 if not re_allowed_after_test.match(line):
Signature · 1 line
1.9.1.423.g4596e3a
#2 lore raw

PATCH 2/2 patman: Avoid duplicate sign-offs

Hi Simon,

Quoted 4 lines

Collect all Signed-off-by tags in a commit and make sure that there are no duplicates.

I have a change request about this patch.

It is true this patch avoids duplication, but
it changes the order *-by: credits.

For example, assume something like this in my git-log

blah blah blah

Patman changes it to

blah blah blah

I want Patman to touch *-by: credits only when it is really necessary.
[1] when adding a missing Signed-off-by:
[2] when deleting redundant Signed-off-by:

Best Regards
Masahiro Yamada

#3 lore raw

PATCH 2/2 patman: Avoid duplicate sign-offs

Hi Masahiro,

Quoted 29 lines · Masahiro Yamada

Hi Simon,

Quoted 4 lines

Collect all Signed-off-by tags in a commit and make sure that there are no duplicates.

I have a change request about this patch.

It is true this patch avoids duplication, but
it changes the order *-by: credits.

For example, assume something like this in my git-log

blah blah blah

Patman changes it to

blah blah blah

Quoted 11 lines

I want Patman to touch *-by: credits only when it is really necessary.
[1] when adding a missing Signed-off-by:
[2] when deleting redundant Signed-off-by:

Yes I agree that is better - I will update the patch and resend as v2.

Regards,
Simon