Reland of Relax git_footers parsing to match that of Gerrit (JGit).
Previously landed as:
> Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
> Reviewed-on: https://chromium-review.googlesource.com/501849
R=agable@chromium.org,machenbach@chromium.org
Bug: 717504
Change-Id: Iede5c29ff4c317b68d8c2bca319edec140a176f5
Reviewed-on: https://chromium-review.googlesource.com/502908
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
diff --git a/git_footers.py b/git_footers.py
index 8e9aaff..e03c71c 100755
--- a/git_footers.py
+++ b/git_footers.py
@@ -42,12 +42,25 @@
return footer_map
+def matches_footer_key(line, key):
+ """Returns whether line is a valid footer whose key matches a given one.
+
+ Keys are compared in normalized form.
+ """
+ r = parse_footer(line)
+ if r is None:
+ return None
+ return normalize_name(r[0]) == normalize_name(key)
+
+
def split_footers(message):
"""Returns (non_footer_lines, footer_lines, parsed footers).
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 +74,8 @@
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
@@ -111,11 +124,11 @@
after_keys = set(map(normalize_name, after_keys or []))
after_indices = [
footer_lines.index(x) for x in footer_lines for k in after_keys
- if normalize_name(parse_footer(x)[0]) == k]
+ if matches_footer_key(x, k)]
before_keys = set(map(normalize_name, before_keys or []))
before_indices = [
footer_lines.index(x) for x in footer_lines for k in before_keys
- if normalize_name(parse_footer(x)[0]) == k]
+ if matches_footer_key(x, k)]
if after_indices:
# after_keys takes precedence, even if there's a conflict.
insert_idx = max(after_indices) + 1
diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py
index 729f333..baf8aee 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 @@
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 8bc06bb..1d537ce 100755
--- a/tests/git_footers_test.py
+++ b/tests/git_footers_test.py
@@ -59,6 +59,33 @@
{ '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',
@@ -101,6 +128,10 @@
git_footers.add_footer_change_id('header: like footer', 'Ixxx'),
'header: like footer\n\nChange-Id: Ixxx')
+ self.assertEqual(
+ git_footers.add_footer_change_id('Header.\n\nBug: v8\nN=t\nT=z', 'Ix'),
+ 'Header.\n\nBug: v8\nChange-Id: Ix\nN=t\nT=z')
+
def testAddFooter(self):
self.assertEqual(
git_footers.add_footer('', 'Key', 'Value'),