[StGit PATCH] Parse commit object header correctly

From

Junio C Hamano <gitster@pobox.com>

Date

Feb 8, 2012, 07:33 UTC

Message-ID

<7vd39pzsmq.fsf_-_@alter.siamese.dyndns.org>

In-Reply-To

<7vvcni1r5u.fsf@alter.siamese.dyndns.org>

 (1) the header ends with an empty line (just an LF, not "a blank line"),
 (2) unknown lines can be skipped, and
 (3) a header "field" begins with the field name, followed by a single SP
     followed by the value.

 - It used "strip()" to lose whitespaces from both ends, risking a line
   with only whitespaces to be mistaken as the end of the header.

 - It used "k, v = line.split(None, 1)", blindly assuming that all header
   lines (including the ones that the version of StGit may not understand)
   can safely be split without raising an exception, which is not true if
   there is no SP on the line.

This patch changes the parsing logic so that it:

 (1) detects end of the hedaer correctly by treating only an empty line as
     such;
 (2) handles multi-line fields (a header line that begins with a single SP
     is appended to the previous line after removing that leading SP but
     retaining the LF between the line and the previous line) correctly;
 (3) splits a line at the first SP to find the field name, but only does
     so when there actually is SP on the line; and
 (4) ignores lines that cannot be understood without barfing.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Earlier I sent a minimum parser fix that ignores multi-line fields, as
   the fields StGit cares about are all single line.  This patch also
   teaches multi-line fields to the parser, so that later versions of
   StGit can parse and use them if they choose to.

   Python is not my primary language, so please take this with a grain of
   salt.

   Thanks.

 stgit/lib/git.py |   41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 56287f6..f19371b 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -390,21 +390,34 @@ class CommitData(Immutable, Repr):
         @return: A new L{CommitData} object
         @rtype: L{CommitData}"""
         cd = cls(parents = [])
-        lines = list(s.splitlines(True))
+        raw_lines = list(s.splitlines(True))
+        lines = []
+        # Collapse multi-line header lines
+        for i in xrange(len(raw_lines)):
+            line = raw_lines[i]
+            if line == '\n':
+                cd.set_message(''.join(raw_lines[i+1:]))
+                break
+            if line[0] == ' ':
+                # continuation line
+                lines[-1] += '\n' + line[1:]
+            else:
+                lines.append(line)
         for i in xrange(len(lines)):
-            line = lines[i].strip()
-            if not line:
-                return cd.set_message(''.join(lines[i+1:]))
-            key, value = line.split(None, 1)
-            if key == 'tree':
-                cd = cd.set_tree(repository.get_tree(value))
-            elif key == 'parent':
-                cd = cd.add_parent(repository.get_commit(value))
-            elif key == 'author':
-                cd = cd.set_author(Person.parse(value))
-            elif key == 'committer':
-                cd = cd.set_committer(Person.parse(value))
-        assert False
+            line = lines[i].rstrip('\n')
+            ix = line.find(' ')
+            if 0 <= ix:
+                key, value = line[0:ix], line[ix+1:]
+                if key == 'tree':
+                    cd = cd.set_tree(repository.get_tree(value))
+                elif key == 'parent':
+                    cd = cd.add_parent(repository.get_commit(value))
+                elif key == 'author':
+                    cd = cd.set_author(Person.parse(value))
+                elif key == 'committer':
+                    cd = cd.set_committer(Person.parse(value))
+        return cd
+
 
 class Commit(GitObject):
     """Represents a git commit object. All the actual data contents of the