From fb22533e80b2677a228c42081b56cb811ed985d8 Mon Sep 17 00:00:00 2001
From: Connor Sheehan <cosheehan@mozilla.com>
Date: Tue, 23 Jan 2024 14:49:31 -0500
Subject: [PATCH] git: scan end of patch for `--` instead of assuming last two
 lines (Bug 1856590) (#373)

Some patches sent with Git will include whitespace at the end of the
patch, meaning the Git version info is the last 3 lines instead of
2. Add a function which scans the lines in reverse and looks for
the first line beginning with `--` instead of using a naive slice
that removes the last two lines of output.

Update the Git parsing test to include extra lines of whitespace
to account for this change in the main tests for `GitPatchParser`.
Also add a simple test for the new parsing function.
---
 landoapi/hgexports.py   | 23 +++++++++++++++++++++--
 tests/test_hgexports.py | 20 +++++++++++++++++++-
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/landoapi/hgexports.py b/landoapi/hgexports.py
index 55ae1b90..c981430f 100644
--- a/landoapi/hgexports.py
+++ b/landoapi/hgexports.py
@@ -290,6 +290,23 @@ def get_header(self, name: bytes | str) -> Optional[str]:
         # `EmailMessage` will return `None` if the header isn't found.
         return self.message[name]
 
+    @classmethod
+    def strip_git_version_info_lines(cls, patch_lines: list[str]) -> list[str]:
+        """Strip the Git version info lines from the end of the given patch lines.
+
+        Assumes the `patch_lines` is the remaining content of a `git format-patch`
+        style patch with Git version info at the base of the patch. Moves backward
+        through the patch to find the `--` barrier between the patch and the version
+        info and strips the version info.
+        """
+        # Collect the lines with the enumerated line numbers in a list, then
+        # iterate through them in reverse order.
+        for i, line in reversed(list(enumerate(patch_lines))):
+            if line.startswith("--"):
+                return patch_lines[:i]
+
+        raise ValueError("Malformed patch: could not find Git version info.")
+
     def parse_email_body(self, content: str) -> tuple[str, str]:
         """Parse the patch email's body, returning the commit message and diff.
 
@@ -340,8 +357,10 @@ def parse_email_body(self, content: str) -> tuple[str, str]:
             raise ValueError("Patch is malformed, could not find start of patch diff.")
 
         # The diff is the remainder of the patch, except the last two lines of Git version info.
-        remaining_lines = list(line_iterator)
-        diff_lines += list(remaining_lines[:-2])
+        remaining_lines = GitPatchHelper.strip_git_version_info_lines(
+            list(line_iterator)
+        )
+        diff_lines += remaining_lines
         diff = "\n".join(diff_lines)
 
         return commit_message, diff
diff --git a/tests/test_hgexports.py b/tests/test_hgexports.py
index 59ba58fa..5805c0d8 100644
--- a/tests/test_hgexports.py
+++ b/tests/test_hgexports.py
@@ -96,7 +96,9 @@
          500,
 --
 2.31.1
-""".strip()
+
+
+""".lstrip()
 
 GIT_PATCH_ONLY_DIFF = """diff --git a/landoui/errorhandlers.py b/landoui/errorhandlers.py
 index f56ba1c..33391ea 100644
@@ -413,3 +415,19 @@ def test_git_formatpatch_helper_empty_commit():
         "unavailable at the moment and is not broken."
     ), "`commit_description()` should return full commit message."
     assert patch.get_diff() == "", "`get_diff()` should return an empty string."
+
+
+def test_strip_git_version_info_lines():
+    lines = [
+        "blah",
+        "blah",
+        "--",
+        "git version info",
+        "",
+        "",
+    ]
+
+    assert GitPatchHelper.strip_git_version_info_lines(lines) == [
+        "blah",
+        "blah",
+    ]