reflog: introduce git_reflog_write()

include/git2/reflog.h

@@ -33,6 +33,15 @@ GIT_BEGIN_DECL
 GIT_EXTERN(int) git_reflog_read(git_reflog **reflog, git_reference *ref);
 
 /**
+ * Write an existing in-memory reflog object back to disk
+ * using an atomic file lock.
+ *
+ * @param reflog an existing reflog object
+ * @return 0 or an error code
+ */
+GIT_EXTERN(int) git_reflog_write(git_reflog *reflog);
+
+/**
  * Add a new entry to the reflog for the given reference
  *
  * If there is no reflog file for the given

src/reflog.c

@@ -28,65 +28,83 @@ static int reflog_init(git_reflog **reflog, git_reference *ref)
 		return -1;
 	}
 
+	log->owner = git_reference_owner(ref);
 	*reflog = log;
 
 	return 0;
 }
 
-static int reflog_write(const char *log_path, const char *oid_old,
-			const char *oid_new, const git_signature *committer,
-			const char *msg)
+static int serialize_reflog_entry(
+	git_buf *buf,
+	const git_oid *oid_old,
+	const git_oid *oid_new,
+	const git_signature *committer,
+	const char *msg)
 {
-	int error;
-	git_buf log = GIT_BUF_INIT;
-	git_filebuf fbuf = GIT_FILEBUF_INIT;
-	bool trailing_newline = false;
+	char raw_old[GIT_OID_HEXSZ+1];
+	char raw_new[GIT_OID_HEXSZ+1];
 
-	assert(log_path && oid_old && oid_new && committer);
+	git_oid_tostr(raw_old, GIT_OID_HEXSZ+1, oid_old);
+	git_oid_tostr(raw_new, GIT_OID_HEXSZ+1, oid_new);
+
+	git_buf_clear(buf);
+
+	git_buf_puts(buf, raw_old);
+	git_buf_putc(buf, ' ');
+	git_buf_puts(buf, raw_new);
+
+	git_signature__writebuf(buf, " ", committer);
+
+	/* drop trailing LF */
+	git_buf_rtrim(buf);
 
 	if (msg) {
 		const char *newline = strchr(msg, '\n');
-		if (newline) {
-			if (*(newline + 1) == '\0')
-				trailing_newline = true;
-			else {
-				giterr_set(GITERR_INVALID, "Reflog message cannot contain newline");
-				return -1;
-			}
+
+		if (newline && newline[1] != '\0') {
+			giterr_set(GITERR_INVALID, "Reflog message cannot contain newline");
+			return -1;
 		}
+
+		git_buf_putc(buf, '\t');
+		git_buf_puts(buf, msg);
+
+		/* drop potential trailing LF */
+		git_buf_rtrim(buf);
 	}
 
-	git_buf_puts(&log, oid_old);
-	git_buf_putc(&log, ' ');
+	git_buf_putc(buf, '\n');
+
+	return git_buf_oom(buf);
+}
+
+static int reflog_write(const char *log_path, const git_oid *oid_old,
+			const git_oid *oid_new, const git_signature *committer,
+			const char *msg)
+{
+	int error = -1;
+	git_buf log = GIT_BUF_INIT;
+	git_filebuf fbuf = GIT_FILEBUF_INIT;
 
-	git_buf_puts(&log, oid_new);
+	assert(log_path && oid_old && oid_new && committer);
 
-	git_signature__writebuf(&log, " ", committer);
-	git_buf_truncate(&log, log.size - 1); /* drop LF */
+	if (serialize_reflog_entry(&log, oid_old, oid_new, committer, msg) < 0)
+		goto cleanup;
 
-	if (msg) {
-		git_buf_putc(&log, '\t');
-		git_buf_puts(&log, msg);
-	}
+	if ((error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND)) < 0)
+		goto cleanup;
 
-	if (!trailing_newline)
-		git_buf_putc(&log, '\n');
+	if ((error = git_filebuf_write(&fbuf, log.ptr, log.size)) < 0)
+		goto cleanup;
 
-	if (git_buf_oom(&log)) {
-		git_buf_free(&log);
-		return -1;
-	}
+	error = git_filebuf_commit(&fbuf, GIT_REFLOG_FILE_MODE);
+	goto success;
 
-	error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND);
-	if (!error) {
-		if ((error = git_filebuf_write(&fbuf, log.ptr, log.size)) < 0)
-			git_filebuf_cleanup(&fbuf);
-		else
-			error = git_filebuf_commit(&fbuf, GIT_REFLOG_FILE_MODE);
-	}
+cleanup:
+	git_filebuf_cleanup(&fbuf);
 
+success:
 	git_buf_free(&log);
-
 	return error;
 }
 
@@ -184,6 +202,12 @@ void git_reflog_free(git_reflog *reflog)
 	git__free(reflog);
 }
 
+static int retrieve_reflog_path(git_buf *path, git_reference *ref)
+{
+	return git_buf_join_n(path, '/', 3,
+		git_reference_owner(ref)->path_repository, GIT_REFLOG_DIR, ref->name);
+}
+
 int git_reflog_read(git_reflog **reflog, git_reference *ref)
 {
 	int error;
@@ -196,8 +220,7 @@ int git_reflog_read(git_reflog **reflog, git_reference *ref)
 	if (reflog_init(&log, ref) < 0)
 		return -1;
 
-	error = git_buf_join_n(&log_path, '/', 3,
-		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
+	error = retrieve_reflog_path(&log_path, ref);
 
 	if (!error)
 		error = git_futils_readbuffer(&log_file, log_path.ptr);
@@ -216,14 +239,53 @@ int git_reflog_read(git_reflog **reflog, git_reference *ref)
 	return error;
 }
 
+int git_reflog_write(git_reflog *reflog)
+{
+	int error = -1;
+	unsigned int i;
+	git_reflog_entry *entry;
+	git_buf log_path = GIT_BUF_INIT;
+	git_buf log = GIT_BUF_INIT;
+	git_filebuf fbuf = GIT_FILEBUF_INIT;
+
+	assert(reflog);
+
+
+	if (git_buf_join_n(&log_path, '/', 3,
+		git_repository_path(reflog->owner), GIT_REFLOG_DIR, reflog->ref_name) < 0)
+		return -1;
+
+	if ((error = git_filebuf_open(&fbuf, git_buf_cstr(&log_path), 0)) < 0)
+		goto cleanup;
+
+	git_vector_foreach(&reflog->entries, i, entry) {
+		if (serialize_reflog_entry(&log, &(entry->oid_old), &(entry->oid_cur), entry->committer, entry->msg) < 0)
+			goto cleanup;
+
+		if ((error = git_filebuf_write(&fbuf, log.ptr, log.size)) < 0)
+			goto cleanup;
+	}
+	
+	error = git_filebuf_commit(&fbuf, GIT_REFLOG_FILE_MODE);
+	goto success;
+
+cleanup:
+	git_filebuf_cleanup(&fbuf);
+
+success:
+	git_buf_free(&log);
+	git_buf_free(&log_path);
+	return error;
+}
+
 int git_reflog_append(git_reference *ref, const git_oid *oid_old,
 				const git_signature *committer, const char *msg)
 {
 	int error;
-	char old[GIT_OID_HEXSZ+1];
-	char new[GIT_OID_HEXSZ+1];
+
 	git_buf log_path = GIT_BUF_INIT;
 	git_reference *r;
+	git_oid zero_oid;
 	const git_oid *oid;
 
 	if ((error = git_reference_resolve(&r, ref)) < 0)
@@ -237,12 +299,7 @@ int git_reflog_append(git_reference *ref, const git_oid *oid_old,
 		return -1;
 	}
 
-	git_oid_tostr(new, GIT_OID_HEXSZ+1, oid);
-
-	git_reference_free(r);
-
-	error = git_buf_join_n(&log_path, '/', 3,
-		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
+	error = retrieve_reflog_path(&log_path, ref);
 	if (error < 0)
 		goto cleanup;
 
@@ -260,14 +317,14 @@ int git_reflog_append(git_reference *ref, const git_oid *oid_old,
 	if (error < 0)
 		goto cleanup;
 
-	if (oid_old)
-		git_oid_tostr(old, sizeof(old), oid_old);
-	else
-		memmove(old, GIT_OID_HEX_ZERO, sizeof(old));
-
-	error = reflog_write(log_path.ptr, old, new, committer, msg);
+	if (!oid_old) {
+		git_oid_fromstr(&zero_oid, GIT_OID_HEX_ZERO);
+		error = reflog_write(log_path.ptr, &zero_oid, oid, committer, msg);
+	} else
+		error = reflog_write(log_path.ptr, oid_old, oid, committer, msg);
 
 cleanup:
+	git_reference_free(r);
 	git_buf_free(&log_path);
 	return error;
 }
@@ -281,7 +338,7 @@ int git_reflog_rename(git_reference *ref, const char *new_name)
 
 	assert(ref && new_name);
 
-	if (git_buf_joinpath(&temp_path, ref->owner->path_repository, GIT_REFLOG_DIR) < 0)
+	if (git_buf_joinpath(&temp_path, git_reference_owner(ref)->path_repository, GIT_REFLOG_DIR) < 0)
 		return -1;
 
 	if (git_buf_joinpath(&old_path, git_buf_cstr(&temp_path), ref->name) < 0)
@@ -329,8 +386,7 @@ int git_reflog_delete(git_reference *ref)
 	int error;
 	git_buf path = GIT_BUF_INIT;
 
-	error = git_buf_join_n(
-		&path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
+	error = retrieve_reflog_path(&path, ref);
 
 	if (!error && git_path_exists(path.ptr))
 		error = p_unlink(path.ptr);

src/reflog.h

@@ -30,6 +30,7 @@ struct git_reflog_entry {
 
 struct git_reflog {
 	char *ref_name;
+	git_repository *owner;
 	git_vector entries;
 };
 

tests-clar/refs/reflog/drop.c

@@ -109,3 +109,22 @@ void test_refs_reflog_drop__can_drop_all_the_entries(void)
 
 	cl_assert_equal_i(0, git_reflog_entrycount(g_reflog));
 }
+
+void test_refs_reflog_drop__can_persist_deletion_on_disk(void)
+{
+	git_reference *ref;
+
+	cl_assert(entrycount > 2);
+
+	cl_git_pass(git_reference_lookup(&ref, g_repo, g_reflog->ref_name));
+	cl_git_pass(git_reflog_entry_drop(g_reflog, entrycount - 1, 1));
+	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
+	cl_git_pass(git_reflog_write(g_reflog));
+
+	git_reflog_free(g_reflog);
+
+	git_reflog_read(&g_reflog, ref);
+	git_reference_free(ref);
+
+	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
+}

tests-clar/refs/reflog/reflog.c

@@ -7,7 +7,7 @@
 
 static const char *new_ref = "refs/heads/test-reflog";
 static const char *current_master_tip = "a65fedf39aefe402d3bb6e24df4d4f5fe4547750";
-static const char *commit_msg = "commit: bla bla";
+#define commit_msg "commit: bla bla"
 
 static git_repository *g_repo;
 
@@ -57,13 +57,13 @@ void test_refs_reflog_reflog__append_then_read(void)
 
 	cl_git_pass(git_reflog_append(ref, NULL, committer, NULL));
 	cl_git_fail(git_reflog_append(ref, NULL, committer, "no ancestor NULL for an existing reflog"));
-	cl_git_fail(git_reflog_append(ref, NULL, committer, "no\nnewline"));
-	cl_git_pass(git_reflog_append(ref, &oid, committer, commit_msg));
+	cl_git_fail(git_reflog_append(ref, NULL, committer, "no inner\nnewline"));
+	cl_git_pass(git_reflog_append(ref, &oid, committer, commit_msg "\n"));
 
 	/* Reopen a new instance of the repository */
 	cl_git_pass(git_repository_open(&repo2, "testrepo.git"));
 
-	/* Lookup the preivously created branch */
+	/* Lookup the previously created branch */
 	cl_git_pass(git_reference_lookup(&lookedup_ref, repo2, new_ref));
 
 	/* Read and parse the reflog for this branch */