commit: refactored existing code to decode commits from streams - performance is slightly better

git.cmd: added method to provide access to the content stream directly. This is more efficient if large objects are handled, if it is actually used
test.helpers: removed unnecessary code

Author: Byron

Diff for: lib/git/cmd.py

@@ -9,12 +9,14 @@
 import git.stats as stats
 from git.actor import Actor
 from tree import Tree
+from cStringIO import StringIO
 import base
 import utils
 import time
 import os
 
-class Commit(base.Object, Iterable, diff.Diffable, utils.Traversable):
+
+class Commit(base.Object, Iterable, diff.Diffable, utils.Traversable, utils.Serializable):
 	"""
 	Wraps a git Commit object.
 	
@@ -91,7 +93,8 @@ def __init__(self, repo, sha, tree=None, author=None, authored_date=None, author
 		self._set_self_from_args_(locals())
 
 		if parents is not None:
-			self.parents = tuple( self.__class__(repo, p) for p in parents )
+			cls = type(self)
+			self.parents = tuple(cls(repo, p) for p in parents if not isinstance(p, cls))
 		# END for each parent to convert
 
 		if self.sha and tree is not None:
@@ -109,20 +112,9 @@ def _set_cache_(self, attr):
 		We set all values at once.
 		"""
 		if attr in Commit.__slots__:
-			# prepare our data lines to match rev-list
-			data_lines = self.data.splitlines()
-			data_lines.insert(0, "commit %s" % self.sha)
-			temp = self._iter_from_process_or_stream(self.repo, iter(data_lines), False).next()
-			self.parents = temp.parents
-			self.tree = temp.tree
-			self.author = temp.author
-			self.authored_date = temp.authored_date
-			self.author_tz_offset = temp.author_tz_offset
-			self.committer = temp.committer
-			self.committed_date = temp.committed_date
-			self.committer_tz_offset = temp.committer_tz_offset
-			self.message = temp.message
-			self.encoding = temp.encoding
+			# read the data in a chunk, its faster - then provide a file wrapper
+			hexsha, typename, size, data = self.repo.git.get_object_data(self)
+			self._deserialize(StringIO(data))
 		else:
 			super(Commit, self)._set_cache_(attr)
 
@@ -260,59 +252,18 @@ def _iter_from_process_or_stream(cls, repo, proc_or_stream, from_rev_list):
 			iterator returning Commit objects
 		"""
 		stream = proc_or_stream
-		if not hasattr(stream,'next'):
+		if not hasattr(stream,'readline'):
 			stream = proc_or_stream.stdout
 
-		for line in stream:
-			commit_tokens = line.split() 
+		while True:
+			line = stream.readline()
+			if not line:
+				break
+			commit_tokens = line.split()
 			id = commit_tokens[1]
 			assert commit_tokens[0] == "commit"
-			tree = stream.next().split()[1]
-
-			parents = []
-			next_line = None
-			for parent_line in stream:
-				if not parent_line.startswith('parent'):
-					next_line = parent_line
-					break
-				# END abort reading parents
-				parents.append(parent_line.split()[-1])
-			# END for each parent line
-			
-			author, authored_date, author_tz_offset = utils.parse_actor_and_date(next_line)
-			committer, committed_date, committer_tz_offset = utils.parse_actor_and_date(stream.next())
-			
 
-			# empty line
-			encoding = stream.next()
-			encoding.strip()
-			if encoding:
-				encoding = encoding[encoding.find(' ')+1:]
-			# END parse encoding
-			
-			message_lines = list()
-			if from_rev_list:
-				for msg_line in stream:
-					if not msg_line.startswith('    '):
-						# and forget about this empty marker
-						break
-					# END abort message reading 
-					# strip leading 4 spaces
-					message_lines.append(msg_line[4:])
-				# END while there are message lines
-			else:
-				# a stream from our data simply gives us the plain message
-				for msg_line in stream:
-					message_lines.append(msg_line)
-			# END message parsing
-			message = '\n'.join(message_lines)
-			
-			
-			yield Commit(repo, id, tree,  
-						 author, authored_date, author_tz_offset,
-						 committer, committed_date, committer_tz_offset,
-						 message, tuple(parents), 
-						 encoding or cls.default_encoding)
+			yield Commit(repo, id)._deserialize(stream, from_rev_list) 
 		# END for each line in stream
 
 
@@ -393,7 +344,7 @@ def create_from_tree(cls, repo, tree, message, parent_commits=None, head=False):
 
 		# assume utf8 encoding
 		enc_section, enc_option = cls.conf_encoding.split('.')
-		conf_encoding = cr.get_value(enc_section, enc_option, default_encoding)
+		conf_encoding = cr.get_value(enc_section, enc_option, cls.default_encoding)
 
 		author = Actor(author_name, author_email)
 		committer = Actor(committer_name, committer_email)
@@ -429,3 +380,61 @@ def __str__(self):
 	def __repr__(self):
 		return '<git.Commit "%s">' % self.sha
 
+	#{ Serializable Implementation
+	
+	def _serialize(self, stream):
+		# for now, this is very inefficient and in fact shouldn't be used like this
+		return super(Commit, self)._serialize(stream)
+	
+	def _deserialize(self, stream, from_rev_list=False):
+		""":param from_rev_list: if true, the stream format is coming from the rev-list command
+		Otherwise it is assumed to be a plain data stream from our object"""
+		self.tree = Tree(self.repo, stream.readline().split()[1], 0, '')
+
+		self.parents = list()
+		next_line = None
+		while True:
+			parent_line = stream.readline()
+			if not parent_line.startswith('parent'):
+				next_line = parent_line
+				break
+			# END abort reading parents
+			self.parents.append(type(self)(self.repo, parent_line.split()[-1]))
+		# END for each parent line
+		self.parents = tuple(self.parents)
+		
+		self.author, self.authored_date, self.author_tz_offset = utils.parse_actor_and_date(next_line)
+		self.committer, self.committed_date, self.committer_tz_offset = utils.parse_actor_and_date(stream.readline())
+		
+		
+		# empty line
+		self.encoding = self.default_encoding
+		enc = stream.readline()
+		enc.strip()
+		if enc:
+			self.encoding = enc[enc.find(' ')+1:]
+		# END parse encoding
+		
+		message_lines = list()
+		if from_rev_list:
+			while True:
+				msg_line = stream.readline()
+				if not msg_line.startswith('    '):
+					# and forget about this empty marker
+					# cut the last newline to get rid of the artificial newline added
+					# by rev-list command. Lets hope its just linux style \n
+					message_lines[-1] = message_lines[-1][:-1]
+					break
+				# END abort message reading 
+				# strip leading 4 spaces
+				message_lines.append(msg_line[4:])
+			# END while there are message lines
+			self.message = ''.join(message_lines)
+		else:
+			# a stream from our data simply gives us the plain message
+			# The end of our message stream is marked with a newline that we strip
+			self.message = stream.read()[:-1]
+		# END message parsing
+		return self
+		
+	#} END serializable implementation

Diff for: lib/git/objects/base.py

@@ -209,7 +209,7 @@ def traverse( self, predicate = lambda i,d: True,
                            visit_once = False, ignore_self=1 ):
         """For documentation, see utils.Traversable.traverse
         
-        Trees are set to visist_once = False to gain more performance in the traversal"""
+        Trees are set to visit_once = False to gain more performance in the traversal"""
         return super(Tree, self).traverse(predicate, prune, depth, branch_first, visit_once, ignore_self)
 
     # List protocol

Diff for: lib/git/objects/commit.py

@@ -280,3 +280,20 @@ def addToStack( stack, item, branch_first, depth ):
 
 			addToStack( stack, item, branch_first, nd )
 		# END for each item on work stack
+		
+
+class Serializable(object):
+	"""Defines methods to serialize and deserialize objects from and into a data stream"""
+	
+	def _serialize(self, stream):
+		"""Serialize the data of this object into the given data stream
+		:note: a serialized object would ``_deserialize`` into the same objet
+		:param stream: a file-like object
+		:return: self"""
+		raise NotImplementedError("To be implemented in subclass")
+		
+	def _deserialize(self, stream):
+		"""Deserialize all information regarding this object from the stream
+		:param stream: a file-like object
+		:return: self"""
+		raise NotImplementedError("To be implemented in subclass")

Diff for: lib/git/objects/tree.py

@@ -129,7 +129,7 @@ def test_rev_list_bisect_all(self, git):
                                       bisect_all=True)
         assert_true(git.called)
 
-        commits = Commit._iter_from_process_or_stream(self.rorepo, ListProcessAdapter(revs), True)
+        commits = Commit._iter_from_process_or_stream(self.rorepo, StringProcessAdapter(revs), True)
         expected_ids = (
             'cf37099ea8d1d8c7fbf9b6d12d7ec0249d3acb8b',
             '33ebe7acec14b25c5f84f35a664803fcab2f7781',

Diff for: lib/git/objects/utils.py

@@ -20,15 +20,15 @@ def _assert_diff_format(self, diffs):
         return diffs
 
     def test_list_from_string_new_mode(self):
-        output = ListProcessAdapter(fixture('diff_new_mode'))
+        output = StringProcessAdapter(fixture('diff_new_mode'))
         diffs = Diff._index_from_patch_format(self.rorepo, output.stdout)
         self._assert_diff_format(diffs)
 
         assert_equal(1, len(diffs))
         assert_equal(10, len(diffs[0].diff.splitlines()))
 
     def test_diff_with_rename(self):
-        output = ListProcessAdapter(fixture('diff_rename'))
+        output = StringProcessAdapter(fixture('diff_rename'))
         diffs = Diff._index_from_patch_format(self.rorepo, output.stdout)
         self._assert_diff_format(diffs)
 
@@ -47,7 +47,7 @@ def test_diff_patch_format(self):
                     "diff_tree_numstat_root" )
 
         for fixture_name in fixtures:
-            diff_proc = ListProcessAdapter(fixture(fixture_name))
+            diff_proc = StringProcessAdapter(fixture(fixture_name))
             diffs = Diff._index_from_patch_format(self.rorepo, diff_proc.stdout)
         # END for each fixture
 

Diff for: test/git/test_commit.py

@@ -48,6 +48,7 @@ def test_heads_should_populate_head_data(self):
 
     def test_tree_from_revision(self):
         tree = self.rorepo.tree('0.1.6')
+        assert len(tree.sha) == 40 
         assert tree.type == "tree"
         assert self.rorepo.tree(tree) == tree
 
@@ -56,9 +57,9 @@ def test_tree_from_revision(self):
 
     @patch_object(Git, '_call_process')
     def test_commits(self, git):
-        git.return_value = ListProcessAdapter(fixture('rev_list'))
+        git.return_value = StringProcessAdapter(fixture('rev_list'))
 
-        commits = list( self.rorepo.iter_commits('master', max_count=10) )
+        commits = list(self.rorepo.iter_commits('master', max_count=10))
 
         c = commits[0]
         assert_equal('4c8124ffcf4039d292442eeccabdeca5af5c5017', c.sha)

Diff for: test/git/test_diff.py

@@ -9,6 +9,7 @@
 from unittest import TestCase
 import tempfile
 import shutil
+import cStringIO
 
 GIT_REPO = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
 
@@ -23,40 +24,13 @@ def absolute_project_path():
     return os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
 
 
-class ListProcessAdapter(object):
-    """Allows to use lists as Process object as returned by SubProcess.Popen.
+class StringProcessAdapter(object):
+    """Allows to use strings as Process object as returned by SubProcess.Popen.
     Its tailored to work with the test system only"""
 
-    class Stream(object):
-        """Simple stream emulater meant to work only with tests"""
-        def __init__(self, data):
-            self.data = data
-            self.cur_iter = None
-        
-        def __iter__(self):
-            dat = self.data
-            if isinstance(dat, basestring):
-                dat = dat.splitlines()
-            if self.cur_iter is None:
-                self.cur_iter = iter(dat)
-            return self.cur_iter
-            
-        def read(self):
-            dat = self.data
-            if isinstance(dat, (tuple,list)):
-                dat = "\n".join(dat)
-            return dat
-            
-        def next(self):
-            if self.cur_iter is None:
-                self.cur_iter = iter(self)
-            return self.cur_iter.next()
-            
-    # END stream 
-    
-    def __init__(self, input_list_or_string):
-        self.stdout = self.Stream(input_list_or_string)
-        self.stderr = self.Stream('')
+    def __init__(self, input_string):
+        self.stdout = cStringIO.StringIO(input_string)
+        self.stderr = cStringIO.StringIO()
 
     def wait(self):
         return 0