Relax git_footers parsing to match that of Gerrit (JGit).

R=agable@chromium.org

Bug: 717504
Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
Reviewed-on: https://chromium-review.googlesource.com/501849
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

git_footers.py

@@ -48,6 +48,8 @@ def split_footers(message):
   Guarantees that:
     (non_footer_lines + footer_lines) == message.splitlines().
     parsed_footers is parse_footer applied on each line of footer_lines.
+      There could be fewer parsed_footers than footer lines if some lines in
+      last paragraph are malformed.
   """
   message_lines = list(message.splitlines())
   footer_lines = []
@@ -61,8 +63,8 @@ def split_footers(message):
     footer_lines = []
 
   footer_lines.reverse()
-  footers = map(parse_footer, footer_lines)
+  footers = filter(None, map(parse_footer, footer_lines))
-  if not footer_lines or not all(footers):
+  if not footers:
     return message_lines, [], []
   return message_lines[:-len(footer_lines)], footer_lines, footers
 

recipes/recipe_modules/bot_update/resources/bot_update.py

@@ -411,9 +411,9 @@ def get_commit_message_footer_map(message):
   for line in lines:
     m = COMMIT_FOOTER_ENTRY_RE.match(line)
     if not m:
-      # If any single line isn't valid, the entire footer is invalid.
+      # If any single line isn't valid, continue anyway for compatibility with
-      footers.clear()
+      # Gerrit (which itself uses JGit for this).
-      return footers
+      continue
     footers[m.group(1)] = m.group(2).strip()
   return footers
 

tests/git_footers_test.py

@@ -59,6 +59,33 @@ My commit message is my best friend. It is my life. I must master it.
         { 'Bug': [''],
           'Cr-Commit-Position': [ self._position ] })
 
+  def testSkippingBadFooterLines(self):
+    message = ('Title.\n'
+               '\n'
+               'Last: paragraph starts\n'
+               'It-may: contain\n'
+               'bad lines, which should be skipped\n'
+               'For: example\n'
+               '(cherry picked from)\n'
+               'And-only-valid: footers taken')
+    self.assertEqual(git_footers.split_footers(message),
+                     (['Title.',
+                       ''],
+                      ['Last: paragraph starts',
+                       'It-may: contain',
+                       'bad lines, which should be skipped',
+                       'For: example',
+                       '(cherry picked from)',
+                       'And-only-valid: footers taken'],
+                      [('Last', 'paragraph starts'),
+                       ('It-may', 'contain'),
+                       ('For', 'example'),
+                       ('And-only-valid', 'footers taken')]))
+    self.assertEqual(git_footers.parse_footers(message),
+                     {'Last': ['paragraph starts'],
+                      'It-May': ['contain'],
+                      'For': ['example'],
+                      'And-Only-Valid': ['footers taken']})
 
   def testGetFooterChangeId(self):
     msg = '\n'.join(['whatever',