Merge pull request libgit2#6345 from arroz/fix/branch-move-reflog

Fixes libgit2#6344: git_branch_move now renames the reflog instead of deleting.

Author: ethomson

src/libgit2/refdb_fs.c

@@ -1785,40 +1785,41 @@ static int refdb_fs_backend__rename(
 		(error = refdb_fs_backend__lookup(&old, _backend, old_name)) < 0)
 		return error;
 
-	if ((error = refdb_fs_backend__delete(_backend, old_name, NULL, NULL)) < 0) {
+	if ((error = loose_lock(&file, backend, old->name)) < 0) {
 		git_reference_free(old);
 		return error;
 	}
 
 	new = git_reference__realloc(&old, new_name);
 	if (!new) {
 		git_reference_free(old);
+		git_filebuf_cleanup(&file);
 		return -1;
 	}
 
-	if ((error = loose_lock(&file, backend, new->name)) < 0) {
+	if ((error = refdb_fs_backend__delete_tail(_backend, &file, old_name, NULL, NULL)) < 0) {
 		git_reference_free(new);
+		git_filebuf_cleanup(&file);
 		return error;
 	}
 
-	/* Try to rename the refog; it's ok if the old doesn't exist */
-	error = refdb_reflog_fs__rename(_backend, old_name, new_name);
-	if (((error == 0) || (error == GIT_ENOTFOUND)) &&
-	    ((error = reflog_append(backend, new, git_reference_target(new), NULL, who, message)) < 0)) {
+	if ((error = loose_lock(&file, backend, new_name)) < 0) {
 		git_reference_free(new);
-		git_filebuf_cleanup(&file);
 		return error;
 	}
 
-	if (error < 0) {
+	/* Try to rename the refog; it's ok if the old doesn't exist */
+	error = refdb_reflog_fs__rename(_backend, old_name, new_name);
+	if (((error == 0) || (error == GIT_ENOTFOUND)) &&
+		((error = reflog_append(backend, new, git_reference_target(new), NULL, who, message)) < 0)) {
 		git_reference_free(new);
 		git_filebuf_cleanup(&file);
 		return error;
 	}
 
-
 	if ((error = loose_commit(&file, new)) < 0 || out == NULL) {
 		git_reference_free(new);
+		git_filebuf_cleanup(&file);
 		return error;
 	}
 
@@ -2401,7 +2402,12 @@ static int refdb_reflog_fs__delete(git_refdb_backend *_backend, const char *name
 	if ((error = reflog_path(&path, backend->repo, name)) < 0)
 		goto out;
 
-	if (!git_fs_path_exists(path.ptr))
+	/*
+	 * If a reference was moved downwards, eg refs/heads/br2 -> refs/heads/br2/new-name,
+	 * refs/heads/br2 does exist but it's a directory. That's a valid situation.
+	 * Proceed only if it's a file.
+	 */
+	if (!git_fs_path_isfile(path.ptr))
 		goto out;
 
 	if ((error = p_unlink(path.ptr)) < 0)

tests/libgit2/refs/branches/move.c

@@ -210,3 +210,41 @@ void test_refs_branches_move__can_move_with_unicode(void)
 	git_reference_free(original_ref);
 	git_reference_free(new_ref);
 }
+
+void test_refs_branches_move__moves_reflog_correctly(void)
+{
+	git_reference *original_ref, *new_ref;
+    git_reflog *original_reflog, *new_reflog;
+
+	cl_git_pass(git_reference_lookup(&original_ref, repo, "refs/heads/br2"));
+
+    cl_git_pass(git_reflog_read(&original_reflog, repo, "refs/heads/br2"));
+    cl_assert_equal_i(2, git_reflog_entrycount(original_reflog));
+
+	cl_git_pass(git_branch_move(&new_ref, original_ref, NEW_BRANCH_NAME, 0));
+	cl_assert_equal_s(GIT_REFS_HEADS_DIR NEW_BRANCH_NAME, git_reference_name(new_ref));
+
+    cl_git_pass(git_reflog_read(&new_reflog, repo, GIT_REFS_HEADS_DIR NEW_BRANCH_NAME));
+    cl_assert_equal_i(3, git_reflog_entrycount(new_reflog));
+
+    git_reference_free(original_ref);
+	git_reference_free(new_ref);
+    git_reflog_free(original_reflog);
+    git_reflog_free(new_reflog);
+}
+
+void test_refs_branches_move__failed_move_restores_reflog(void)
+{
+	git_reference *original_ref, *new_ref;
+	git_reflog *recovered_reflog;
+
+	cl_git_pass(git_reference_lookup(&original_ref, repo, "refs/heads/br2"));
+
+	cl_assert_equal_i(GIT_EINVALIDSPEC, git_branch_move(&new_ref, original_ref, "Inv@{id", 0));
+
+	cl_git_pass(git_reflog_read(&recovered_reflog, repo, "refs/heads/br2"));
+	cl_assert_equal_i(2, git_reflog_entrycount(recovered_reflog));
+
+	git_reference_free(original_ref);
+	git_reflog_free(recovered_reflog);
+}