Re: STGIT: Deathpatch in linus tree

From

Junio C Hamano <gitster@pobox.com>

Date

Feb 7, 2012, 17:37 UTC

Message-ID

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

In-Reply-To

<4F3120D4.1050604@warmcat.com>

"Andy Green (林安廸)" <andy@warmcat.com> writes:

> [agreen@build linux-2.6]$ git reset --hard 105e518
> HEAD is now at 105e518 Merge tag 'hwmon-fixes-for-3.3-rc3' of
> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
> [agreen@build linux-2.6]$ stg push
> Error: Unhandled exception:
> Traceback (most recent call last):
>   File "/usr/lib/python2.7/site-packages/stgit/main.py", line 152, in _main
>     ret = command.func(parser, options, args)
>   File "/usr/lib/python2.7/site-packages/stgit/commands/push.py", line
> 68, in func
>     check_clean_iw = clean_iw)
>   File "/usr/lib/python2.7/site-packages/stgit/lib/transaction.py", line
> 95, in __init__
>     self.__current_tree = self.__stack.head.data.tree
>   File "/usr/lib/python2.7/site-packages/stgit/lib/git.py", line 426, in
> data
>     self.__repository.cat_object(self.sha1))
>   File "/usr/lib/python2.7/site-packages/stgit/lib/git.py", line 408, in
> parse
>     assert False
> AssertionError

@@ -404,8 +404,6 @@ class CommitData(Immutable, Repr):
                 cd = cd.set_author(Person.parse(value))
             elif key == 'committer':
                 cd = cd.set_committer(Person.parse(value))
-            else:
-                assert False
         assert False

The above code in StGit that tries to parse commit object header is
broken, and I am surprised that nobody caught it back in late 2006 when
the optional encoding header was added to the commit object, which the
above does not understand.

I am not sure if that removal of the extra assertion is enough, though.
It does not skip lines that it does not understand until the first blank
line, i.e. the end of the header, as it should.  Instead it has this
before starting to inspect if a line in the header is what it knows about:

            line = lines[i].strip()
            if not line:
                return cd.set_message(''.join(lines[i+1:]))
            key, value = line.split(None, 1)

I do not think this will work on a continuation line inside a header that
has only one token, and it will also ignore the leading whitespace that
protects the continuation line.

I think the loop should at least changed line this, until StGit starts
caring about multi-line headers with continuation lines.

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 56287f6..f2b284d 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -392,18 +392,20 @@ class CommitData(Immutable, Repr):
         cd = cls(parents = [])
         lines = list(s.splitlines(True))
         for i in xrange(len(lines)):
-            line = lines[i].strip()
+            line = lines[i]
             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))
+	    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))
         assert False
 
 class Commit(GitObject):