From de37c01295cbc323956495017eeedcd9a3feced7 Mon Sep 17 00:00:00 2001
From: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu, 6 Jul 2017 21:06:50 +0200
Subject: [PATCH] Fix git-numberer swallowing of footers for Rietveld.

Due to relaxation of when last paragraph of commit message is
is consider as containing footers, `git cl land` started removing
non-canonic footer lines from last paragraph if Git Numberer is enabled
on a repo. This only manifests in manual lands of Rietveld CLs or
bypassing code review entirely.

R=agable@chromium.org
Bug: 736852
Change-Id: I3972c590c3959974157ada9de9891a3c08bd385a
Reviewed-on: https://chromium-review.googlesource.com/562278
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
---
 git_cl.py            | 16 ++++++++--------
 tests/git_cl_test.py | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/git_cl.py b/git_cl.py
index e7d952273..a2de0c448 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -3517,13 +3517,14 @@ class ChangeDescription(object):
         re.match(self.CHERRY_PICK_LINE, self._description_lines[-1])):
       cp_line = self._description_lines.pop()
 
-    top_lines, _, parsed_footers = git_footers.split_footers(self.description)
+    top_lines, footer_lines, _ = git_footers.split_footers(self.description)
 
     # Original-ify all Cr- footers, to avoid re-lands, cherry-picks, or just
     # user interference with actual footers we'd insert below.
-    for i, (k, v) in enumerate(parsed_footers):
-      if k.startswith('Cr-'):
-        parsed_footers[i] = (k.replace('Cr-', 'Cr-Original-'), v)
+    for i, line in enumerate(footer_lines):
+      k, v = git_footers.parse_footer(line) or (None, None)
+      if k and k.startswith('Cr-'):
+        footer_lines[i] = '%s: %s' % ('Cr-Original-' + k[len('Cr-'):], v)
 
     # Add Position and Lineage footers based on the parent.
     lineage = list(reversed(parent_footer_map.get('Cr-Branched-From', [])))
@@ -3535,16 +3536,15 @@ class ChangeDescription(object):
       lineage.insert(0, '%s-%s@{#%d}' % (parent_hash, parent_position[0],
                                          int(parent_position[1])))
 
-    parsed_footers.append(('Cr-Commit-Position',
-                           '%s@{#%d}' % (dest_ref, number)))
-    parsed_footers.extend(('Cr-Branched-From', v) for v in lineage)
+    footer_lines.append('Cr-Commit-Position: %s@{#%d}' % (dest_ref, number))
+    footer_lines.extend('Cr-Branched-From: %s' % v for v in lineage)
 
     self._description_lines = top_lines
     if cp_line:
       self._description_lines.append(cp_line)
     if self._description_lines[-1] != '':
       self._description_lines.append('')  # Ensure footer separator.
-    self._description_lines.extend('%s: %s' % kv for kv in parsed_footers)
+    self._description_lines.extend(footer_lines)
 
 
 def get_approving_reviewers(props, disapproval=False):
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 65eedb104..91dffcab2 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -278,6 +278,24 @@ class TestGitClBasic(unittest.TestCase):
         '\n'
         'Cr-Commit-Position: refs/heads/master@{#13}')
 
+  def test_git_number_same_branch_mixed_footers(self):
+    actual = self._test_git_number(
+        'Parent\n'
+        '\n'
+        'Cr-Commit-Position: refs/heads/master@{#12}',
+        dest_ref='refs/heads/master',
+        child_msg='Child\n'
+                  '\n'
+                  'Broken-by: design\n'
+                  'BUG=123')
+    self.assertEqualByLine(
+        actual,
+        'Child\n'
+        '\n'
+        'Broken-by: design\n'
+        'BUG=123\n'
+        'Cr-Commit-Position: refs/heads/master@{#13}')
+
   def test_git_number_same_branch_with_originals(self):
     actual = self._test_git_number(
         'Parent\n'