Commit bf9e8cc86b9c32946a395fd12a9b1a5cb71575a9

nulltoken 2012-07-20T16:34:08

branch: make git_branch_move() reference based

diff --git a/include/git2/branch.h b/include/git2/branch.h
index fcddb93..724cfba 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -96,24 +96,19 @@ GIT_EXTERN(int) git_branch_foreach(
 );
 
 /**
- * Move/rename an existing branch reference.
+ * Move/rename an existing local branch reference.
  *
- * @param repo Repository where lives the branch.
- *
- * @param old_branch_name Current name of the branch to be moved;
- * this name is validated for consistency.
+ * @param branch Current underlying reference of the branch.
  *
  * @param new_branch_name Target name of the branch once the move
  * is performed; this name is validated for consistency.
  *
  * @param force Overwrite existing branch.
  *
- * @return 0 on success, GIT_ENOTFOUND if the branch
- * doesn't exist or an error code.
+ * @return 0 on success, or an error code.
  */
 GIT_EXTERN(int) git_branch_move(
-		git_repository *repo,
-		const char *old_branch_name,
+		git_reference *branch,
 		const char *new_branch_name,
 		int force);
 
diff --git a/src/branch.c b/src/branch.c
index f0945b6..4a56fd1 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -180,27 +180,31 @@ int git_branch_foreach(
 	return git_reference_foreach(repo, GIT_REF_LISTALL, &branch_foreach_cb, (void *)&filter);
 }
 
-int git_branch_move(git_repository *repo, const char *old_branch_name, const char *new_branch_name, int force)
+static int not_a_local_branch(git_reference *ref)
 {
-	git_reference *reference = NULL;
-	git_buf old_reference_name = GIT_BUF_INIT, new_reference_name = GIT_BUF_INIT;
-	int error = 0;
+	giterr_set(GITERR_INVALID, "Reference '%s' is not a local branch.", git_reference_name(ref));
+	return -1;
+}
 
-	if ((error = git_buf_joinpath(&old_reference_name, GIT_REFS_HEADS_DIR, old_branch_name)) < 0)
-		goto cleanup;
+int git_branch_move(
+	git_reference *branch,
+	const char *new_branch_name,
+	int force)
+{
+	git_buf new_reference_name = GIT_BUF_INIT;
+	int error;
 
-	/* We need to be able to return GIT_ENOTFOUND */
-	if ((error = git_reference_lookup(&reference, repo, git_buf_cstr(&old_reference_name))) < 0)
-		goto cleanup;
+	assert(branch && new_branch_name);
+
+	if (!git_reference_is_branch(branch))
+		return not_a_local_branch(branch);
 
 	if ((error = git_buf_joinpath(&new_reference_name, GIT_REFS_HEADS_DIR, new_branch_name)) < 0)
 		goto cleanup;
 
-	error = git_reference_rename(reference, git_buf_cstr(&new_reference_name), force);
+	error = git_reference_rename(branch, git_buf_cstr(&new_reference_name), force);
 
 cleanup:
-	git_reference_free(reference);
-	git_buf_free(&old_reference_name);
 	git_buf_free(&new_reference_name);
 
 	return error;
diff --git a/tests-clar/refs/branches/move.c b/tests-clar/refs/branches/move.c
index 258f74c..6750473 100644
--- a/tests-clar/refs/branches/move.c
+++ b/tests-clar/refs/branches/move.c
@@ -1,71 +1,64 @@
 #include "clar_libgit2.h"
+#include "refs.h"
 
 static git_repository *repo;
+static git_reference *ref;
 
 void test_refs_branches_move__initialize(void)
 {
-	cl_fixture_sandbox("testrepo.git");
-	cl_git_pass(git_repository_open(&repo, "testrepo.git"));
+	repo = cl_git_sandbox_init("testrepo.git");
+
+	cl_git_pass(git_reference_lookup(&ref, repo, "refs/heads/br2"));
 }
 
 void test_refs_branches_move__cleanup(void)
 {
-	git_repository_free(repo);
-
-	cl_fixture_cleanup("testrepo.git");
+	git_reference_free(ref);
+	cl_git_sandbox_cleanup();
 }
 
 #define NEW_BRANCH_NAME "new-branch-on-the-block"
 
 void test_refs_branches_move__can_move_a_local_branch(void)
 {
-	cl_git_pass(git_branch_move(repo, "br2", NEW_BRANCH_NAME, 0));
+	cl_git_pass(git_branch_move(ref, NEW_BRANCH_NAME, 0));
+	cl_assert_equal_s(GIT_REFS_HEADS_DIR NEW_BRANCH_NAME, git_reference_name(ref));
 }
 
 void test_refs_branches_move__can_move_a_local_branch_to_a_different_namespace(void)
 {
 	/* Downward */
-	cl_git_pass(git_branch_move(repo, "br2", "somewhere/" NEW_BRANCH_NAME, 0));
+	cl_git_pass(git_branch_move(ref, "somewhere/" NEW_BRANCH_NAME, 0));
 
 	/* Upward */
-	cl_git_pass(git_branch_move(repo, "somewhere/" NEW_BRANCH_NAME, "br2", 0));
+	cl_git_pass(git_branch_move(ref, "br2", 0));
 }
 
 void test_refs_branches_move__can_move_a_local_branch_to_a_partially_colliding_namespace(void)
 {
 	/* Downward */
-	cl_git_pass(git_branch_move(repo, "br2", "br2/" NEW_BRANCH_NAME, 0));
+	cl_git_pass(git_branch_move(ref, "br2/" NEW_BRANCH_NAME, 0));
 
 	/* Upward */
-	cl_git_pass(git_branch_move(repo, "br2/" NEW_BRANCH_NAME, "br2", 0));
+	cl_git_pass(git_branch_move(ref, "br2", 0));
 }
 
 void test_refs_branches_move__can_not_move_a_branch_if_its_destination_name_collide_with_an_existing_one(void)
 {
-	cl_git_fail(git_branch_move(repo, "br2", "master", 0));
+	cl_git_fail(git_branch_move(ref, "master", 0));
 }
 
-void test_refs_branches_move__can_not_move_a_non_existing_branch(void)
+void test_refs_branches_move__can_not_move_a_non_branch(void)
 {
-	cl_git_fail(git_branch_move(repo, "i-am-no-branch", NEW_BRANCH_NAME, 0));
-}
+	git_reference *tag;
 
-void test_refs_branches_move__can_force_move_over_an_existing_branch(void)
-{
-	cl_git_pass(git_branch_move(repo, "br2", "master", 1));
-}
+	cl_git_pass(git_reference_lookup(&tag, repo, "refs/tags/e90810b"));
+	cl_git_fail(git_branch_move(tag, NEW_BRANCH_NAME, 0));
 
-void test_refs_branches_move__can_not_move_a_branch_through_its_canonical_name(void)
-{
-	cl_git_fail(git_branch_move(repo, "refs/heads/br2", NEW_BRANCH_NAME, 1));
+	git_reference_free(tag);
 }
 
-void test_refs_branches_move__moving_a_non_exisiting_branch_returns_ENOTFOUND(void)
+void test_refs_branches_move__can_force_move_over_an_existing_branch(void)
 {
-	int error;
-
-	error = git_branch_move(repo, "where/am/I", NEW_BRANCH_NAME, 0);
-	cl_git_fail(error);
-
-	cl_assert_equal_i(GIT_ENOTFOUND, error);
+	cl_git_pass(git_branch_move(ref, "master", 1));
 }