Actor: Moved it from git.objects.util to git.util, adjusted all imports accordingly. Added methods to Actor to retrieve the global committer and author information

Reflog: implemented and tested append_entry method

11 files changed, 210 insertions, 109 deletions

diff --git a/__init__.py b/__init__.py
index 7f275b44..483ac091 100644
--- a/__init__.py
+++ b/__init__.py
@@ -37,7 +37,8 @@ from git.index import *
 from git.util import (
 						LockFile, 
 						BlockingLockFile, 
-						Stats
+						Stats,
+						Actor
 						)
 
 #} END imports
diff --git a/objects/__init__.py b/objects/__init__.py
index e8e0ef39..65659cd1 100644
--- a/objects/__init__.py
+++ b/objects/__init__.py
@@ -15,7 +15,6 @@ from tag import *
 from blob import *
 from commit import *
 from tree import *
-from util import Actor
 
 __all__ = [ name for name, obj in locals().items()
             if not (name.startswith('_') or inspect.ismodule(obj)) ]
\ No newline at end of file
diff --git a/objects/commit.py b/objects/commit.py
index a2b6c554..9c7e66a3 100644
--- a/objects/commit.py
+++ b/objects/commit.py
@@ -4,7 +4,8 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
-from git.util import (
+from git.util import 		(
+							Actor,
 							Iterable,
 							Stats,
 						)
@@ -20,9 +21,7 @@ from gitdb.util import (
 from util import (
 						Traversable,
 						Serializable,
-						get_user_id,
 						parse_date,
-						Actor,
 						altz_to_utctz_str,
 						parse_actor_and_date
 					)
@@ -43,17 +42,10 @@ class Commit(base.Object, Iterable, Diffable, Traversable, Serializable):
 	
 	# ENVIRONMENT VARIABLES
 	# read when creating new commits
-	env_author_name = "GIT_AUTHOR_NAME"
-	env_author_email = "GIT_AUTHOR_EMAIL"
 	env_author_date = "GIT_AUTHOR_DATE"
-	env_committer_name = "GIT_COMMITTER_NAME"
-	env_committer_email = "GIT_COMMITTER_EMAIL"
 	env_committer_date = "GIT_COMMITTER_DATE"
-	env_email = "EMAIL"
 	
 	# CONFIGURATION KEYS
-	conf_name = 'name'
-	conf_email = 'email'
 	conf_encoding = 'i18n.commitencoding'
 	
 	# INVARIANTS
@@ -306,17 +298,9 @@ class Commit(base.Object, Iterable, Diffable, Traversable, Serializable):
 		# COMMITER AND AUTHOR INFO
 		cr = repo.config_reader()
 		env = os.environ
-		default_email = get_user_id()
-		default_name = default_email.split('@')[0]
 		
-		conf_name = cr.get_value('user', cls.conf_name, default_name)
-		conf_email = cr.get_value('user', cls.conf_email, default_email)
-		
-		author_name = env.get(cls.env_author_name, conf_name)
-		author_email = env.get(cls.env_author_email, conf_email)
-		
-		committer_name = env.get(cls.env_committer_name, conf_name)
-		committer_email = env.get(cls.env_committer_email, conf_email)
+		committer = Actor.committer(cr)
+		author = Actor.author(cr)
 		
 		# PARSE THE DATES
 		unix_time = int(time())
@@ -340,9 +324,6 @@ class Commit(base.Object, Iterable, Diffable, Traversable, Serializable):
 		enc_section, enc_option = cls.conf_encoding.split('.')
 		conf_encoding = cr.get_value(enc_section, enc_option, cls.default_encoding)
 		
-		author = Actor(author_name, author_email)
-		committer = Actor(committer_name, committer_email)
-		
 		
 		# if the tree is no object, make sure we create one - otherwise
 		# the created commit object is invalid
diff --git a/objects/util.py b/objects/util.py
index dfaaf7c4..4c9323b8 100644
--- a/objects/util.py
+++ b/objects/util.py
@@ -4,19 +4,21 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 """Module for general utility functions"""
-from git.util import IterableList
+from git.util import (
+						IterableList, 
+						Actor
+					)
 
 import re
 from collections import deque as Deque
-import platform
 
 from string import digits
 import time
 import os
 
-__all__ = ('get_object_type_by_name', 'get_user_id', 'parse_date', 'parse_actor_and_date', 
+__all__ = ('get_object_type_by_name', 'parse_date', 'parse_actor_and_date', 
 			'ProcessStreamAdapter', 'Traversable', 'altz_to_utctz_str', 'utctz_to_altz', 
-			'verify_utctz')
+			'verify_utctz', 'Actor')
 
 #{ Functions
 
@@ -57,18 +59,6 @@ def get_object_type_by_name(object_type_name):
 	else:
 		raise ValueError("Cannot handle unknown object type: %s" % object_type_name)
 		
-
-def get_user_id():
-	""":return: string identifying the currently active system user as name@node
-	:note: user can be set with the 'USER' environment variable, usually set on windows"""
-	ukn = 'UNKNOWN'
-	username = os.environ.get('USER', os.environ.get('USERNAME', ukn))
-	if username == ukn and hasattr(os, 'getlogin'):
-		username = os.getlogin()
-	# END get username from login
-	return "%s@%s" % (username, platform.node())
-		
-
 def utctz_to_altz(utctz):
 	"""we convert utctz to the timezone in seconds, it is the format time.altzone
 	returns. Git stores it as UTC timezone which has the opposite sign as well, 
@@ -193,58 +183,6 @@ def parse_actor_and_date(line):
 
 
 #{ Classes 
-
-class Actor(object):
-	"""Actors hold information about a person acting on the repository. They 
-	can be committers and authors or anything with a name and an email as 
-	mentioned in the git log entries."""
-	# precompiled regex
-	name_only_regex = re.compile( r'<(.+)>' )
-	name_email_regex = re.compile( r'(.*) <(.+?)>' )
-	
-	__slots__ = ('name', 'email')
-	
-	def __init__(self, name, email):
-		self.name = name
-		self.email = email
-
-	def __eq__(self, other):
-		return self.name == other.name and self.email == other.email
-		
-	def __ne__(self, other):
-		return not (self == other)
-		
-	def __hash__(self):
-		return hash((self.name, self.email))
-
-	def __str__(self):
-		return self.name
-
-	def __repr__(self):
-		return '<git.Actor "%s <%s>">' % (self.name, self.email)
-
-	@classmethod
-	def _from_string(cls, string):
-		"""Create an Actor from a string.
-		:param string: is the string, which is expected to be in regular git format
-
-				John Doe <jdoe@example.com>
-				
-		:return: Actor """
-		m = cls.name_email_regex.search(string)
-		if m:
-			name, email = m.groups()
-			return Actor(name, email)
-		else:
-			m = cls.name_only_regex.search(string)
-			if m:
-				return Actor(m.group(1), None)
-			else:
-				# assume best and use the whole string as name
-				return Actor(string, None)
-			# END special case name
-		# END handle name/email matching
-	
 	
 class ProcessStreamAdapter(object):
 	"""Class wireing all calls to the contained Process instance.
diff --git a/refs/log.py b/refs/log.py
index 1d07ef9a..8cb0a5ab 100644
--- a/refs/log.py
+++ b/refs/log.py
@@ -1,17 +1,22 @@
-from git.util import join_path
+from git.util import (
+						join_path,
+						Actor,
+					)
+
 from gitdb.util import (
+						bin_to_hex,
 						join,
 						file_contents_ro_filepath
 					)
 
 from git.objects.util import (
-								Actor, 
 								parse_date,
 								Serializable, 
 								utctz_to_altz,
 								altz_to_utctz_str,
 							)
 
+import time
 import os
 import re
 
@@ -104,7 +109,28 @@ class RefLog(list, Serializable):
 	Reflog entries are orded, the first added entry is first in the list, the last
 	entry, i.e. the last change of the head or reference, is last in the list."""
 	
-	__slots__ = tuple()
+	__slots__ = ('_path', )
+	
+	def __new__(cls, filepath=None):
+		inst = super(RefLog, cls).__new__(cls)
+		return inst
+		
+	def __init__(self, filepath=None):
+		"""Initialize this instance with an optional filepath, from which we will
+		initialize our data. The path is also used to write changes back using 
+		the write() method"""
+		self._path = filepath
+		if filepath is not None:
+			self._read_from_file()
+		# END handle filepath
+	
+	def _read_from_file(self):
+		fmap = file_contents_ro_filepath(self._path, stream=False, allow_mmap=True)
+		try:
+			self._deserialize(fmap)
+		finally:
+			fmap.close()
+		#END handle closing of handle
 	
 	#{ Interface
 	
@@ -115,14 +141,7 @@ class RefLog(list, Serializable):
 			at the given filepath
 		:param filepath: path to reflog 
 		:raise ValueError: If the file could not be read or was corrupted in some way"""
-		inst = cls()
-		fmap = file_contents_ro_filepath(filepath, stream=False, allow_mmap=True)
-		try:
-			inst._deserialize(fmap)
-		finally:
-			fmap.close()
-		#END handle closing of handle 
-		return inst
+		return cls(filepath)
 	
 	@classmethod
 	def path(cls, ref):
@@ -154,12 +173,35 @@ class RefLog(list, Serializable):
 	def to_file(self, filepath):
 		"""Write the contents of the reflog instance to a file at the given filepath.
 		:param filepath: path to file, parent directories are assumed to exist"""
+		# TODO: Use locked fd
 		fp = open(filepath, 'wb')
 		try:
 			self._serialize(fp)
 		finally:
 			fp.close()
 		#END handle file streams
+		
+	def append_entry(self, oldbinsha, newbinsha, message, write=True):
+		"""Append a new log entry to the revlog, changing it in place.
+		:param oldbinsha: binary sha of the previous commit
+		:param newbinsha: binary sha of the current commit
+		:param message: message describing the change to the reference
+		:param write: If True, the changes will be written right away. Otherwise
+			the change will not be written
+		:return: RefLogEntry objects which was appended to the log"""
+		if len(oldbinsha) != 20 or len(newbinsha) != 20:
+			raise ValueError("Shas need to be given in binary format")
+		#END handle sha type
+		entry = RefLogEntry((bin_to_hex(oldbinsha), bin_to_hex(newbinsha), Actor.committer(), (int(time.time()), time.altzone), message)) 
+		self.append(entry)
+		if write:
+			self.write()
+		#END handle auto-write
+		return entry
+		
+	def write(self):
+		"""Write this instance's data to the file we are originating from"""
+		return self.to_file(self._path)
 	
 	#} END interface
 	
diff --git a/refs/symbolic.py b/refs/symbolic.py
index 9dd7629c..b978e484 100644
--- a/refs/symbolic.py
+++ b/refs/symbolic.py
@@ -236,6 +236,7 @@ class SymbolicReference(object):
 		if not isdir(directory):
 			os.makedirs(directory)
 		
+		# TODO: Write using LockedFD
 		fp = open(path, "wb")
 		try:
 			fp.write(write_value)
diff --git a/repo/base.py b/repo/base.py
index 6f401628..c8613878 100644
--- a/repo/base.py
+++ b/repo/base.py
@@ -6,6 +6,7 @@
 
 from git.exc import InvalidGitRepositoryError, NoSuchPathError
 from git.cmd import Git
+from git.util import Actor
 from git.refs import *
 from git.index import IndexFile
 from git.objects import *
diff --git a/test/test_reflog.py b/test/test_reflog.py
index a017106e..67b1a9da 100644
--- a/test/test_reflog.py
+++ b/test/test_reflog.py
@@ -1,6 +1,7 @@
 from git.test.lib import *
-from git.objects import IndexObject, Actor
+from git.objects import IndexObject
 from git.refs import *
+from git.util import Actor
 
 import tempfile
 import shutil
@@ -40,6 +41,7 @@ class TestRefLog(TestBase):
 		
 		# simple read
 		reflog = RefLog.from_file(rlp_master_ro)
+		assert reflog._path is not None
 		assert isinstance(reflog, RefLog)
 		assert len(reflog)
 		
@@ -56,6 +58,8 @@ class TestRefLog(TestBase):
 		
 	
 		# test serialize and deserialize - results must match exactly
+		binsha = chr(255)*20
+		msg = "my reflog message"
 		for rlp in (rlp_head, rlp_master):
 			reflog = RefLog.from_file(rlp)
 			tfile = os.path.join(tdir, os.path.basename(rlp))
@@ -67,6 +71,18 @@ class TestRefLog(TestBase):
 			
 			# ... as well as each bytes of the written stream
 			assert open(tfile).read() == open(rlp).read()
+			
+			# append an entry - it gets written automatically
+			entry = treflog.append_entry(IndexObject.NULL_BIN_SHA, binsha, msg)
+			assert entry.oldhexsha == IndexObject.NULL_HEX_SHA
+			assert entry.newhexsha == 'f'*40
+			assert entry.message == msg
+			assert treflog == RefLog.from_file(tfile)
+			
+			# but not this time
+			treflog.append_entry(binsha, binsha, msg, write=False)
+			assert treflog != RefLog.from_file(tfile)
+			
 		# END for each reflog 
 		
 		
diff --git a/test/test_refs.py b/test/test_refs.py
index 1f3dfb9f..c7764d92 100644
--- a/test/test_refs.py
+++ b/test/test_refs.py
@@ -8,6 +8,7 @@ from mock import *
 from git.test.lib import *
 from git import *
 import git.refs as refs
+from git.util import Actor
 from git.objects.tag import TagObject
 from itertools import chain
 import os
diff --git a/test/test_util.py b/test/test_util.py
index 7a6eb27d..e55a6d15 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -16,7 +16,7 @@ from git.cmd import dashify
 import time
 
 
-class TestUtils(TestCase):
+class TestUtils(TestBase):
 	def setup(self):
 		self.testdict = {
 			"string":	"42",
@@ -102,4 +102,8 @@ class TestUtils(TestCase):
 		self.failUnlessRaises(ValueError, parse_date, '123456789 -02000')
 		self.failUnlessRaises(ValueError, parse_date, ' 123456789 -0200')
 		
-		
+	def test_actor(self):
+		for cr in (None, self.rorepo.config_reader()):
+			assert isinstance(Actor.committer(cr), Actor)
+			assert isinstance(Actor.author(cr), Actor)
+		#END assure config reader is handled
diff --git a/util.py b/util.py
index c945e6a3..3ce05888 100644
--- a/util.py
+++ b/util.py
@@ -5,8 +5,10 @@
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
 import os
+import re
 import sys
 import time
+import platform
 import tempfile
 
 from gitdb.util import (
@@ -20,7 +22,9 @@ from gitdb.util import (
 
 __all__ = ( "stream_copy", "join_path", "to_native_path_windows", "to_native_path_linux", 
 			"join_path_native", "Stats", "IndexFileSHA1Writer", "Iterable", "IterableList", 
-			"BlockingLockFile", "LockFile" )
+			"BlockingLockFile", "LockFile", 'Actor', 'get_user_id' )
+
+#{ Utility Methods
 
 def stream_copy(source, destination, chunk_size=512*1024):
 	"""Copy all data from the source stream into the destination stream in chunks
@@ -70,6 +74,119 @@ def join_path_native(a, *p):
 	use '\'"""
 	return to_native_path(join_path(a, *p))
 
+def get_user_id():
+	""":return: string identifying the currently active system user as name@node
+	:note: user can be set with the 'USER' environment variable, usually set on windows"""
+	ukn = 'UNKNOWN'
+	username = os.environ.get('USER', os.environ.get('USERNAME', ukn))
+	if username == ukn and hasattr(os, 'getlogin'):
+		username = os.getlogin()
+	# END get username from login
+	return "%s@%s" % (username, platform.node())
+
+#} END utilities
+
+#{ Classes
+
+class Actor(object):
+	"""Actors hold information about a person acting on the repository. They 
+	can be committers and authors or anything with a name and an email as 
+	mentioned in the git log entries."""
+	# PRECOMPILED REGEX
+	name_only_regex = re.compile( r'<(.+)>' )
+	name_email_regex = re.compile( r'(.*) <(.+?)>' )
+	
+	# ENVIRONMENT VARIABLES
+	# read when creating new commits
+	env_author_name = "GIT_AUTHOR_NAME"
+	env_author_email = "GIT_AUTHOR_EMAIL"
+	env_committer_name = "GIT_COMMITTER_NAME"
+	env_committer_email = "GIT_COMMITTER_EMAIL"
+	
+	# CONFIGURATION KEYS
+	conf_name = 'name'
+	conf_email = 'email'
+	
+	__slots__ = ('name', 'email')
+	
+	def __init__(self, name, email):
+		self.name = name
+		self.email = email
+
+	def __eq__(self, other):
+		return self.name == other.name and self.email == other.email
+		
+	def __ne__(self, other):
+		return not (self == other)
+		
+	def __hash__(self):
+		return hash((self.name, self.email))
+
+	def __str__(self):
+		return self.name
+
+	def __repr__(self):
+		return '<git.Actor "%s <%s>">' % (self.name, self.email)
+
+	@classmethod
+	def _from_string(cls, string):
+		"""Create an Actor from a string.
+		:param string: is the string, which is expected to be in regular git format
+
+				John Doe <jdoe@example.com>
+				
+		:return: Actor """
+		m = cls.name_email_regex.search(string)
+		if m:
+			name, email = m.groups()
+			return Actor(name, email)
+		else:
+			m = cls.name_only_regex.search(string)
+			if m:
+				return Actor(m.group(1), None)
+			else:
+				# assume best and use the whole string as name
+				return Actor(string, None)
+			# END special case name
+		# END handle name/email matching
+		
+	@classmethod
+	def _main_actor(cls, env_name, env_email, config_reader=None):
+		actor = Actor('', '')
+		default_email = get_user_id()
+		default_name = default_email.split('@')[0]
+		
+		for attr, evar, cvar, default in (('name', env_name, cls.conf_name, default_name), 
+										('email', env_email, cls.conf_email, default_email)):
+			try:
+				setattr(actor, attr, os.environ[evar])
+			except KeyError:
+				if config_reader is not None:
+					setattr(actor, attr, config_reader.get_value('user', cvar, default))
+				#END config-reader handling
+				if not getattr(actor, attr):
+					setattr(actor, attr, default)
+			#END handle name
+		#END for each item to retrieve
+		return actor
+		
+		
+	@classmethod
+	def committer(cls, config_reader=None):
+		""":return: Actor instance corresponding to the configured committer. It behaves
+		similar to the git implementation, such that the environment will override 
+		configuration values of config_reader. If no value is set at all, it will be
+		generated
+		:param config_reader: ConfigReader to use to retrieve the values from in case
+			they are not set in the environment"""
+		return cls._main_actor(cls.env_committer_name, cls.env_committer_email, config_reader)
+		
+	@classmethod
+	def author(cls, config_reader=None):
+		"""Same as committer(), but defines the main author. It may be specified in the environment, 
+		but defaults to the committer"""
+		return cls._main_actor(cls.env_author_name, cls.env_author_email, config_reader)
+		
 
 class Stats(object):
 	"""
@@ -345,4 +462,4 @@ class Iterable(object):
 		:return:  iterator yielding Items"""
 		raise NotImplementedError("To be implemented by Subclass")
 		
-		
+#} END classes