refs: fix moving of the reflog when renaming a ref

src/reflog.c

@@ -269,18 +269,50 @@ cleanup:
 
 int git_reflog_rename(git_reference *ref, const char *new_name)
 {
-	int error;
+	int error = -1, fd;
 	git_buf old_path = GIT_BUF_INIT;
 	git_buf new_path = GIT_BUF_INIT;
+	git_buf temp_path = GIT_BUF_INIT;
 
-	if (!git_buf_join_n(&old_path, '/', 3, ref->owner->path_repository,
+	assert(ref && new_name);
-			GIT_REFLOG_DIR, ref->name) &&
-		!git_buf_join_n(&new_path, '/', 3, ref->owner->path_repository,
-			GIT_REFLOG_DIR, new_name))
-		error = p_rename(git_buf_cstr(&old_path), git_buf_cstr(&new_path));
-	else
-		error = -1;
 
+	if (git_buf_joinpath(&temp_path, ref->owner->path_repository, GIT_REFLOG_DIR) < 0)
+		return -1;
+
+	if (git_buf_joinpath(&old_path, git_buf_cstr(&temp_path), ref->name) < 0)
+		goto cleanup;
+
+	if (git_buf_joinpath(&new_path, git_buf_cstr(&temp_path), new_name) < 0)
+		goto cleanup;
+
+	/*
+	 * Move the reflog to a temporary place. This two-phase renaming is required
+	 * in order to cope with funny renaming use cases when one tries to move a reference
+	 * to a partially colliding namespace:
+	 *  - a/b -> a/b/c
+	 *  - a/b/c/d -> a/b/c
+	 */
+	if (git_buf_joinpath(&temp_path, git_buf_cstr(&temp_path), "temp_reflog") < 0)
+		goto cleanup;
+
+	if ((fd = git_futils_mktmp(&temp_path, git_buf_cstr(&temp_path))) < 0)
+		goto cleanup;
+	p_close(fd);
+
+	if (p_rename(git_buf_cstr(&old_path), git_buf_cstr(&temp_path)) < 0)
+		goto cleanup;
+
+	if (git_path_isdir(git_buf_cstr(&new_path)) && 
+		(git_futils_rmdir_r(git_buf_cstr(&new_path), GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0))
+		goto cleanup;
+
+	if (git_futils_mkpath2file(git_buf_cstr(&new_path), GIT_REFLOG_DIR_MODE) < 0)
+		goto cleanup;
+
+	error = p_rename(git_buf_cstr(&temp_path), git_buf_cstr(&new_path));
+
+cleanup:
+	git_buf_free(&temp_path);
 	git_buf_free(&old_path);
 	git_buf_free(&new_path);
 

src/refs.c

@@ -1406,6 +1406,7 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force)
 	/*
 	 * Rename the reflog file.
 	 */
+	git_buf_clear(&aux_path);
 	if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0)
 		goto cleanup;
 

tests-clar/refs/reflog.c

@@ -26,7 +26,7 @@ static void assert_signature(git_signature *expected, git_signature *actual)
 // Fixture setup and teardown
 void test_refs_reflog__initialize(void)
 {
-   g_repo = cl_git_sandbox_init("testrepo");
+   g_repo = cl_git_sandbox_init("testrepo.git");
 }
 
 void test_refs_reflog__cleanup(void)
@@ -61,7 +61,7 @@ void test_refs_reflog__write_then_read(void)
 	cl_git_pass(git_reflog_write(ref, &oid, committer, commit_msg));
 
 	/* Reopen a new instance of the repository */
-	cl_git_pass(git_repository_open(&repo2, "testrepo"));
+	cl_git_pass(git_repository_open(&repo2, "testrepo.git"));
 
 	/* Lookup the preivously created branch */
 	cl_git_pass(git_reference_lookup(&lookedup_ref, repo2, new_ref));
@@ -121,3 +121,27 @@ void test_refs_reflog__dont_write_bad(void)
 
 	git_reference_free(ref);
 }
+
+void test_refs_reflog__renaming_the_reference_moves_the_reflog(void)
+{
+	git_reference *master;
+	git_buf master_log_path = GIT_BUF_INIT, moved_log_path = GIT_BUF_INIT;
+
+	git_buf_joinpath(&master_log_path, git_repository_path(g_repo), GIT_REFLOG_DIR);
+	git_buf_puts(&moved_log_path, git_buf_cstr(&master_log_path));
+	git_buf_joinpath(&master_log_path, git_buf_cstr(&master_log_path), "refs/heads/master");
+	git_buf_joinpath(&moved_log_path, git_buf_cstr(&moved_log_path), "refs/moved");
+
+	cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&master_log_path)));
+	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(master, "refs/moved", 0));
+
+	cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&master_log_path)));
+	cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&moved_log_path)));
+
+	git_reference_free(master);
+	git_buf_free(&moved_log_path);
+	git_buf_free(&master_log_path);
+}