diff --git a/git_footers.py b/git_footers.py index 8e9aaff9bc..31535c917c 100755 --- a/git_footers.py +++ b/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) - if not footer_lines or not all(footers): + footers = filter(None, map(parse_footer, footer_lines)) + if not footers: return message_lines, [], [] return message_lines[:-len(footer_lines)], footer_lines, footers diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py index 729f3337b3..baf8aeea1b 100755 --- a/recipes/recipe_modules/bot_update/resources/bot_update.py +++ b/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. - footers.clear() - return footers + # If any single line isn't valid, continue anyway for compatibility with + # Gerrit (which itself uses JGit for this). + continue footers[m.group(1)] = m.group(2).strip() return footers diff --git a/tests/git_footers_test.py b/tests/git_footers_test.py index 8bc06bb2c0..d7a4cfedd3 100755 --- a/tests/git_footers_test.py +++ b/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',