Ensure renaming a reference updates the reflog

7 files changed, 64 insertions, 41 deletions

diff --git a/include/git2/refs.h b/include/git2/refs.h
index b203f242b..976b7496b 100644
--- a/include/git2/refs.h
+++ b/include/git2/refs.h
@@ -297,6 +297,8 @@ GIT_EXTERN(int) git_reference_set_target(
  * @param ref The reference to rename
  * @param new_name The new name for the reference
  * @param force Overwrite an existing reference
+ * @param signature The identity that will used to populate the reflog entry
+ * @param log_message The one line long message to be appended to the reflog
  * @return 0 on success, GIT_EINVALIDSPEC, GIT_EEXISTS or an error code
  *
  */
@@ -304,7 +306,9 @@ GIT_EXTERN(int) git_reference_rename(
 	git_reference **new_ref,
 	git_reference *ref,
 	const char *new_name,
-	int force);
+	int force,
+	const git_signature *signature,
+	const char *log_message);
 
 /**
  * Delete an existing reference.
diff --git a/src/branch.c b/src/branch.c
index 4658d3bdd..c7651c6c3 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -211,7 +211,8 @@ int git_branch_move(
 	/* first update ref then config so failure won't trash config */
 
 	error = git_reference_rename(
-		out, branch, git_buf_cstr(&new_reference_name), force);
+		out, branch, git_buf_cstr(&new_reference_name), force,
+		signature, log_message);
 	if (error < 0)
 		goto done;
 
diff --git a/src/refs.c b/src/refs.c
index 65e7e6439..adbc1666a 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -558,35 +558,27 @@ int git_reference_rename(
 	git_reference **out,
 	git_reference *ref,
 	const char *new_name,
-	int force)
+	int force,
+	const git_signature *signature,
+	const char *log_message)
 {
-	git_signature *who;
+	git_signature *who = (git_signature*)signature;
 	int error;
 
 	/* Should we return an error if there is no default? */
-	if (((error = git_signature_default(&who, ref->db->repo)) < 0) &&
+	if (!who &&
+	    ((error = git_signature_default(&who, ref->db->repo)) < 0) &&
 	    ((error = git_signature_now(&who, "unknown", "unknown")) < 0)) {
 		return error;
 	}
 
-	error = reference__rename(out, ref, new_name, force, who, NULL);
+	error = reference__rename(out, ref, new_name, force, who, log_message);
 
 	git_signature_free(who);
 
 	return error;
 }
 
-int git_reference_rename_with_log(
-	git_reference **out,
-	git_reference *ref,
-	const char *new_name,
-	int force,
-	const git_signature *who,
-	const char * message)
-{
-	return reference__rename(out, ref, new_name, force, who, message);
-}
-
 int git_reference_resolve(git_reference **ref_out, const git_reference *ref)
 {
 	switch (git_reference_type(ref)) {
diff --git a/src/remote.c b/src/remote.c
index 5d35affd1..f33f5ef3c 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1332,20 +1332,28 @@ static int rename_one_remote_reference(
 {
 	int error;
 	git_buf new_name = GIT_BUF_INIT;
+	git_buf log_message = GIT_BUF_INIT;
 
-	error = git_buf_printf(
-		&new_name,
-		GIT_REFS_REMOTES_DIR "%s%s",
-		new_remote_name,
-		reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name));
+	if ((error = git_buf_printf(
+					&new_name,
+					GIT_REFS_REMOTES_DIR "%s%s",
+					new_remote_name,
+					reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name))) < 0)
+		goto cleanup;
 
-	if (!error) {
-		error = git_reference_rename(
-			NULL, reference, git_buf_cstr(&new_name), 0);
-		git_reference_free(reference);
-	}
+	if ((error = git_buf_printf(&log_message,
+					"renamed remote %s to %s",
+					old_remote_name, new_remote_name)) < 0)
+		goto cleanup;
+
+	error = git_reference_rename(
+		NULL, reference, git_buf_cstr(&new_name), 0,
+		NULL, git_buf_cstr(&log_message));
+	git_reference_free(reference);
 
+cleanup:
 	git_buf_free(&new_name);
+	git_buf_free(&log_message);
 	return error;
 }
 
diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c
index 82b28de15..3f7d7d777 100644
--- a/tests/refs/reflog/reflog.c
+++ b/tests/refs/reflog/reflog.c
@@ -114,7 +114,7 @@ void test_refs_reflog_reflog__renaming_the_reference_moves_the_reflog(void)
 	cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&moved_log_path)));
 
 	cl_git_pass(git_reference_lookup(&master, g_repo, "refs/heads/master"));
-	cl_git_pass(git_reference_rename(&new_master, master, "refs/moved", 0));
+	cl_git_pass(git_reference_rename(&new_master, master, "refs/moved", 0, NULL, NULL));
 	git_reference_free(master);
 
 	cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&master_log_path)));
@@ -165,7 +165,7 @@ void test_refs_reflog_reflog__cannot_write_a_moved_reflog(void)
 
 	cl_git_pass(git_reflog_write(reflog));
 
-	cl_git_pass(git_reference_rename(&new_master, master, "refs/moved", 0));
+	cl_git_pass(git_reference_rename(&new_master, master, "refs/moved", 0, NULL, NULL));
 	git_reference_free(master);
 
 	cl_git_fail(git_reflog_write(reflog));
diff --git a/tests/refs/rename.c b/tests/refs/rename.c
index 120967892..63f202cbf 100644
--- a/tests/refs/rename.c
+++ b/tests/refs/rename.c
@@ -49,7 +49,7 @@ void test_refs_rename__loose(void)
 	cl_assert(reference_is_packed(looked_up_ref) == 0);
 
 	/* Now that the reference is renamed... */
-	cl_git_pass(git_reference_rename(&new_ref, looked_up_ref, new_name, 0));
+	cl_git_pass(git_reference_rename(&new_ref, looked_up_ref, new_name, 0, NULL, NULL));
 	cl_assert_equal_s(new_ref->name, new_name);
 	git_reference_free(looked_up_ref);
 
@@ -91,7 +91,7 @@ void test_refs_rename__packed(void)
 	cl_assert(reference_is_packed(looked_up_ref) != 0);
 
 	/* Now that the reference is renamed... */
-	cl_git_pass(git_reference_rename(&new_ref, looked_up_ref, brand_new_name, 0));
+	cl_git_pass(git_reference_rename(&new_ref, looked_up_ref, brand_new_name, 0, NULL, NULL));
 	cl_assert_equal_s(new_ref->name, brand_new_name);
 	git_reference_free(looked_up_ref);
 
@@ -140,7 +140,7 @@ void test_refs_rename__packed_doesnt_pack_others(void)
 	cl_assert(reference_is_packed(looked_up_ref) != 0);
 
 	/* Now that the reference is renamed... */
-	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, brand_new_name, 0));
+	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, brand_new_name, 0, NULL, NULL));
 	git_reference_free(looked_up_ref);
 
 	/* Lookup the other reference */
@@ -166,7 +166,7 @@ void test_refs_rename__name_collision(void)
 	cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, packed_head_name));
 
 	/* Can not be renamed to the name of another existing reference. */
-	cl_git_fail(git_reference_rename(&renamed_ref, looked_up_ref, packed_test_head_name, 0));
+	cl_git_fail(git_reference_rename(&renamed_ref, looked_up_ref, packed_test_head_name, 0, NULL, NULL));
 	git_reference_free(looked_up_ref);
 
 	/* Failure to rename it hasn't corrupted its state */
@@ -187,12 +187,12 @@ void test_refs_rename__invalid_name(void)
 	/* Can not be renamed with an invalid name. */
 	cl_assert_equal_i(
 		GIT_EINVALIDSPEC,
-		git_reference_rename(&renamed_ref, looked_up_ref, "Hello! I'm a very invalid name.", 0));
+		git_reference_rename(&renamed_ref, looked_up_ref, "Hello! I'm a very invalid name.", 0, NULL, NULL));
 
 	/* Can not be renamed outside of the refs hierarchy
 	 * unless it's ALL_CAPS_AND_UNDERSCORES.
 	 */
-	cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_rename(&renamed_ref, looked_up_ref, "i-will-sudo-you", 0));
+	cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_rename(&renamed_ref, looked_up_ref, "i-will-sudo-you", 0, NULL, NULL));
 
 	/* Failure to rename it hasn't corrupted its state */
 	git_reference_free(looked_up_ref);
@@ -213,7 +213,7 @@ void test_refs_rename__force_loose_packed(void)
 	git_oid_cpy(&oid, git_reference_target(looked_up_ref));
 
 	/* Can be force-renamed to the name of another existing reference. */
-	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, packed_test_head_name, 1));
+	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, packed_test_head_name, 1, NULL, NULL));
 	git_reference_free(looked_up_ref);
 	git_reference_free(renamed_ref);
 
@@ -238,7 +238,7 @@ void test_refs_rename__force_loose(void)
 	git_oid_cpy(&oid, git_reference_target(looked_up_ref));
 
 	/* Can be force-renamed to the name of another existing reference. */
-	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, "refs/heads/test", 1));
+	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, "refs/heads/test", 1, NULL, NULL));
 	git_reference_free(looked_up_ref);
 	git_reference_free(renamed_ref);
 
@@ -307,7 +307,7 @@ void test_refs_rename__prefix(void)
 	cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, ref_two_name));
 
 	/* Can be rename to a new name starting with the old name. */
-	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, ref_two_name_new, 0));
+	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, ref_two_name_new, 0, NULL, NULL));
 	git_reference_free(looked_up_ref);
 	git_reference_free(renamed_ref);
 
@@ -341,7 +341,7 @@ void test_refs_rename__move_up(void)
 	cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, ref_two_name_new));
 
 	/* Can be renamed upward the reference tree. */
-	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, ref_two_name, 0));
+	cl_git_pass(git_reference_rename(&renamed_ref, looked_up_ref, ref_two_name, 0, NULL, NULL));
 	git_reference_free(looked_up_ref);
 	git_reference_free(renamed_ref);
 
@@ -361,7 +361,25 @@ void test_refs_rename__propagate_eexists(void)
 
 	cl_git_pass(git_reference_lookup(&ref, g_repo, packed_head_name));
 
-	cl_assert_equal_i(GIT_EEXISTS, git_reference_rename(&new_ref, ref, packed_test_head_name, 0));
+	cl_assert_equal_i(GIT_EEXISTS, git_reference_rename(&new_ref, ref, packed_test_head_name, 0, NULL, NULL));
 
 	git_reference_free(ref);
 }
+
+void test_refs_rename__writes_to_reflog(void)
+{
+	git_reference *ref, *new_ref;
+	git_reflog *log;
+	const git_reflog_entry *entry;
+
+	cl_git_pass(git_reference_lookup(&ref, g_repo, ref_master_name));
+	cl_git_pass(git_reference_rename(&new_ref, ref, ref_one_name_new, false,
+				NULL, "message"));
+	cl_git_pass(git_reflog_read(&log, g_repo, git_reference_name(new_ref)));
+	entry = git_reflog_entry_byindex(log, 0);
+	cl_assert_equal_s("message", git_reflog_entry_message(entry));
+
+	git_reflog_free(log);
+	git_reference_free(ref);
+	git_reference_free(new_ref);
+}
diff --git a/tests/refs/revparse.c b/tests/refs/revparse.c
index 88f270326..a540f389d 100644
--- a/tests/refs/revparse.c
+++ b/tests/refs/revparse.c
@@ -325,7 +325,7 @@ static void create_fake_stash_reference_and_reflog(git_repository *repo)
 	cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&log_path)));
 
 	cl_git_pass(git_reference_lookup(&master, repo, "refs/heads/master"));
-	cl_git_pass(git_reference_rename(&new_master, master, "refs/fakestash", 0));
+	cl_git_pass(git_reference_rename(&new_master, master, "refs/fakestash", 0, NULL, NULL));
 	git_reference_free(master);
 
 	cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&log_path)));