Implemented revlog.append_entry as classmethod, to assure we will always actually write_append the new entry, instead of rewriting the whole file. Added file-locking and directory handling, so the implementation should be similar (enough) to the git reference implementation.

Next up is to implement a way to update the reflog when changing references, which is going to be a little more complicated

4 files changed, 57 insertions, 20 deletions

diff --git a/refs/log.py b/refs/log.py
index c2799f79..9728911a 100644
--- a/refs/log.py
+++ b/refs/log.py
@@ -2,12 +2,15 @@ from git.util import (
 						join_path,
 						Actor,
 						LockedFD,
+						LockFile,
+						assure_directory_exists,
+						to_native_path,
 					)
 
 from gitdb.util import (
 						bin_to_hex,
 						join,
-						file_contents_ro_filepath
+						file_contents_ro_filepath,
 					)
 
 from git.objects.util import (
@@ -151,7 +154,7 @@ class RefLog(list, Serializable):
 			instance would be found. The path is not guaranteed to point to a valid 
 			file though.
 		:param ref: SymbolicReference instance"""
-		return join(ref.repo.git_dir, "logs", ref.path)
+		return join(ref.repo.git_dir, "logs", to_native_path(ref.path))
 		
 	@classmethod
 	def iter_entries(cls, stream):
@@ -175,6 +178,8 @@ class RefLog(list, Serializable):
 		"""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"""
 		lfd = LockedFD(filepath)
+		assure_directory_exists(filepath, is_file=True)
+		
 		fp = lfd.open(write=True, stream=True)
 		try:
 			self._serialize(fp)
@@ -185,22 +190,34 @@ class RefLog(list, Serializable):
 			raise
 		#END handle change
 		
-	def append_entry(self, oldbinsha, newbinsha, message, write=True):
-		"""Append a new log entry to the revlog, changing it in place.
+	@classmethod
+	def append_entry(cls, filepath, oldbinsha, newbinsha, message):
+		"""Append a new log entry to the revlog at filepath. 
 		: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"""
+		:return: RefLogEntry objects which was appended to the log
+		:note: As we are append-only, concurrent access is not a problem as we 
+			do not interfere with readers."""
 		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
+		assure_directory_exists(filepath, is_file=True)
+		entry = RefLogEntry((bin_to_hex(oldbinsha), bin_to_hex(newbinsha), Actor.committer(), (int(time.time()), time.altzone), message))
+		
+		lf = LockFile(filepath)
+		lf._obtain_lock_or_raise()
+		
+		fd = open(filepath, 'a')
+		try:
+			fd.write(repr(entry))
+		finally:
+			fd.close()
+			lf._release_lock()
+		#END handle write operation
+		
 		return entry
 		
 	def write(self):
diff --git a/refs/symbolic.py b/refs/symbolic.py
index 94e8d726..0d8fdfd1 100644
--- a/refs/symbolic.py
+++ b/refs/symbolic.py
@@ -266,8 +266,19 @@ class SymbolicReference(object):
 			applied to this reference
 			
 		.. note:: As the log is parsed every time, its recommended to cache it for use
-			instead of calling this method repeatedly"""
+			instead of calling this method repeatedly. It should be considered read-only."""
 		return RefLog.from_file(RefLog.path(self))
+		
+	def log_append(self, oldbinsha, message, newbinsha=None):
+		"""Append a logentry to the logfile of this ref
+		:param oldbinsha: binary sha this ref used to point to
+		:param message: A message describing the change
+		:param newbinsha: The sha the ref points to now. If None, our current commit sha
+			will be used
+		:return: added RefLogEntry instance"""
+		return RefLog.append_entry(RefLog.path(self), oldbinsha, 
+									(newbinsha is None and self.commit.binsha) or newbinsha, 
+									message) 
 
 	@classmethod
 	def to_full_path(cls, path):
diff --git a/test/test_reflog.py b/test/test_reflog.py
index 67b1a9da..e99e5ba5 100644
--- a/test/test_reflog.py
+++ b/test/test_reflog.py
@@ -72,17 +72,12 @@ 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)
+			# append an entry
+			entry = RefLog.append_entry(tfile, 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)
-			
+			assert RefLog.from_file(tfile)[-1] == entry
 		# END for each reflog 
 		
 		
diff --git a/util.py b/util.py
index 3ce05888..c93ae715 100644
--- a/util.py
+++ b/util.py
@@ -22,7 +22,7 @@ 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", 'Actor', 'get_user_id' )
+			"BlockingLockFile", "LockFile", 'Actor', 'get_user_id', 'assure_directory_exists')
 
 #{ Utility Methods
 
@@ -73,6 +73,20 @@ def join_path_native(a, *p):
 	needed to play it safe on my dear windows and to assure nice paths that only 
 	use '\'"""
 	return to_native_path(join_path(a, *p))
+	
+def assure_directory_exists(path, is_file=False):
+	"""Assure that the directory pointed to by path exists.
+	:param is_file: If True, path is assumed to be a file and handled correctly.
+		Otherwise it must be a directory
+	:return: True if the directory was created, False if it already existed"""
+	if is_file:
+		path = os.path.dirname(path)
+	#END handle file 
+	if not os.path.isdir(path):
+		os.makedirs(path)
+		return True
+	return False
+	
 
 def get_user_id():
 	""":return: string identifying the currently active system user as name@node