Merge pull request #1343 from nulltoken/topic/remote_orphaned_branch

Teach git_branch_remote_name() to work with orphaned heads

6 files changed, 55 insertions, 75 deletions

diff --git a/include/git2/branch.h b/include/git2/branch.h
index 3c7fb443c..d372c2c92 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -221,7 +221,7 @@ GIT_EXTERN(int) git_branch_is_head(
  *
  * @param repo The repository where the branch lives.
  *
- * @param branch The reference to the remote tracking branch.
+ * @param canonical_branch_name name of the remote tracking branch.
  *
  * @return Number of characters in the reference name
  *     including the trailing NUL byte; GIT_ENOTFOUND
@@ -233,7 +233,7 @@ GIT_EXTERN(int) git_branch_remote_name(
 	char *remote_name_out,
 	size_t buffer_size,
 	git_repository *repo,
-	git_reference *branch);
+	const char *canonical_branch_name);
 
 /** @} */
 GIT_END_DECL
diff --git a/src/branch.c b/src/branch.c
index 11ecbe9a1..a50387541 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -323,7 +323,7 @@ int git_branch_remote_name(
 	char *remote_name_out,
 	size_t buffer_size,
 	git_repository *repo,
-	git_reference *branch)
+	const char *canonical_branch_name)
 {
 	git_strarray remote_list = {0};
 	size_t i, remote_name_size;
@@ -332,15 +332,15 @@ int git_branch_remote_name(
 	int error = 0;
 	char *remote_name = NULL;
 
-	assert(branch);
+	assert(repo && canonical_branch_name);
 
 	if (remote_name_out && buffer_size)
 		*remote_name_out = '\0';
 
 	/* Verify that this is a remote branch */
-	if (!git_reference_is_remote(branch)) {
-		giterr_set(GITERR_INVALID,
-				   "Reference '%s' is not a remote branch.", branch->name);
+	if (!git_reference__is_remote(canonical_branch_name)) {
+		giterr_set(GITERR_INVALID, "Reference '%s' is not a remote branch.",
+			canonical_branch_name);
 		error = GIT_ERROR;
 		goto cleanup;
 	}
@@ -358,7 +358,7 @@ int git_branch_remote_name(
 
 		/* Defensivly check that we have a fetchspec */
 		if (fetchspec &&
-			git_refspec_dst_matches(fetchspec, branch->name)) {
+			git_refspec_dst_matches(fetchspec, canonical_branch_name)) {
 			/* If we have not already set out yet, then set
 			 * it to the matching remote name. Otherwise
 			 * multiple remotes match this reference, and it
diff --git a/src/refs.c b/src/refs.c
index 866c230e6..cca3f3ec8 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1926,10 +1926,15 @@ int git_reference_is_branch(git_reference *ref)
 	return git_reference__is_branch(ref->name);
 }
 
+int git_reference__is_remote(const char *ref_name)
+{
+	return git__prefixcmp(ref_name, GIT_REFS_REMOTES_DIR) == 0;
+}
+
 int git_reference_is_remote(git_reference *ref)
 {
 	assert(ref);
-	return git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR) == 0;
+	return git_reference__is_remote(ref->name);
 }
 
 static int peel_error(int error, git_reference *ref, const char* msg)
diff --git a/src/refs.h b/src/refs.h
index 1228cea87..7bd1ae68a 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -70,6 +70,7 @@ int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int f
 int git_reference__is_valid_name(const char *refname, unsigned int flags);
 int git_reference__update(git_repository *repo, const git_oid *oid, const char *ref_name);
 int git_reference__is_branch(const char *ref_name);
+int git_reference__is_remote(const char *ref_name);
 
 /**
  * Lookup a reference by name and try to resolve to an OID.
diff --git a/tests-clar/clone/empty.c b/tests-clar/clone/empty.c
index e611bc24e..0f867257a 100644
--- a/tests-clar/clone/empty.c
+++ b/tests-clar/clone/empty.c
@@ -34,7 +34,9 @@ static void cleanup_repository(void *path)
 void test_clone_empty__can_clone_an_empty_local_repo_barely(void)
 {
 	char *local_name = "refs/heads/master";
-	char tracking_name[1024];
+	const char *expected_tracked_branch_name = "refs/remotes/origin/master";
+	const char *expected_remote_name = "origin";
+	char buffer[1024];
 	git_reference *ref;
 
 	cl_set_cleanup(&cleanup_repository, "./empty");
@@ -46,8 +48,20 @@ void test_clone_empty__can_clone_an_empty_local_repo_barely(void)
 	cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&ref, g_repo_cloned, local_name));
 
 	/* ...one can still retrieve the name of the remote tracking reference */
-	cl_assert_equal_i((int)strlen("refs/remotes/origin/master") + 1U, 
-		git_branch_tracking_name(tracking_name, 1024, g_repo_cloned, local_name));
+	cl_assert_equal_i((int)strlen(expected_tracked_branch_name) + 1, 
+		git_branch_tracking_name(buffer, 1024, g_repo_cloned, local_name));
+
+	cl_assert_equal_s(expected_tracked_branch_name, buffer);
+
+	/* ...and the name of the remote... */
+	cl_assert_equal_i((int)strlen(expected_remote_name) + 1, 
+		git_branch_remote_name(buffer, 1024, g_repo_cloned, expected_tracked_branch_name));
+
+	cl_assert_equal_s(expected_remote_name, buffer);
+
+	/* ...even when the remote HEAD is orphaned as well */
+	cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&ref, g_repo_cloned,
+		expected_tracked_branch_name));
 }
 
 void test_clone_empty__can_clone_an_empty_local_repo(void)
diff --git a/tests-clar/refs/branches/remote.c b/tests-clar/refs/branches/remote.c
index be355af46..145c3182f 100644
--- a/tests-clar/refs/branches/remote.c
+++ b/tests-clar/refs/branches/remote.c
@@ -3,18 +3,15 @@
 #include "remote.h"
 
 static git_repository *g_repo;
-
-static const char *current_master_tip = "099fabac3a9ea935598528c27f866e34089c2eff";
+static const char *remote_tracking_branch_name = "refs/remotes/test/master";
+static const char *expected_remote_name = "test";
+static int expected_remote_name_length;
 
 void test_refs_branches_remote__initialize(void)
 {
-	git_oid id;
-
 	g_repo = cl_git_sandbox_init("testrepo");
-	git_oid_fromstr(&id, current_master_tip);
 
-	/* Create test/master */
-	git_reference_create(NULL, g_repo, "refs/remotes/test/master", &id, 1);
+	expected_remote_name_length = strlen(expected_remote_name) + 1;
 }
 
 void test_refs_branches_remote__cleanup(void)
@@ -24,81 +21,48 @@ void test_refs_branches_remote__cleanup(void)
 
 void test_refs_branches_remote__can_get_remote_for_branch(void)
 {
-	git_reference *ref;
-	const char *name;
-	char *expectedRemoteName = "test";
-	int expectedRemoteNameLength = strlen(expectedRemoteName) + 1;
 	char remotename[1024] = {0};
 
-	cl_git_pass(git_branch_lookup(&ref, g_repo, "test/master", GIT_BRANCH_REMOTE));
-	cl_git_pass(git_branch_name(&name, ref));
-	cl_assert_equal_s("test/master", name);
+	cl_assert_equal_i(expected_remote_name_length,
+		git_branch_remote_name(NULL, 0, g_repo, remote_tracking_branch_name));
 
-	cl_assert_equal_i(expectedRemoteNameLength,
-		git_branch_remote_name(NULL, 0, g_repo, ref));
-	cl_assert_equal_i(expectedRemoteNameLength,
-		git_branch_remote_name(remotename, expectedRemoteNameLength, g_repo, ref));
-	cl_assert_equal_s("test", remotename);
+	cl_assert_equal_i(expected_remote_name_length,
+		git_branch_remote_name(remotename, expected_remote_name_length, g_repo,
+			remote_tracking_branch_name));
 
-	git_reference_free(ref);
+	cl_assert_equal_s("test", remotename);
 }
 
 void test_refs_branches_remote__insufficient_buffer_returns_error(void)
 {
-	git_reference *ref;
-	const char *name;
-	char *expectedRemoteName = "test";
-	int expectedRemoteNameLength = strlen(expectedRemoteName) + 1;
 	char remotename[1024] = {0};
 
-	cl_git_pass(git_branch_lookup(&ref, g_repo, "test/master", GIT_BRANCH_REMOTE));
-	cl_git_pass(git_branch_name(&name, ref));
-	cl_assert_equal_s("test/master", name);
-
-	cl_assert_equal_i(expectedRemoteNameLength,
-		git_branch_remote_name(NULL, 0, g_repo, ref));
-	cl_git_fail_with(GIT_ERROR,
-		git_branch_remote_name(remotename, expectedRemoteNameLength - 1, g_repo, ref));
+	cl_assert_equal_i(expected_remote_name_length,
+		git_branch_remote_name(NULL, 0, g_repo, remote_tracking_branch_name));
 
-	git_reference_free(ref);
+	cl_git_fail_with(git_branch_remote_name(remotename,
+		expected_remote_name_length - 1, g_repo, remote_tracking_branch_name),
+			GIT_ERROR);
 }
 
 void test_refs_branches_remote__no_matching_remote_returns_error(void)
 {
-	git_reference *ref;
-	const char *name;
-	git_oid id;
+	const char *unknown = "refs/remotes/nonexistent/master";
 
-	git_oid_fromstr(&id, current_master_tip);
-
-	/* Create nonexistent/master */
-	git_reference_create(NULL, g_repo, "refs/remotes/nonexistent/master", &id, 1);
-
-	cl_git_pass(git_branch_lookup(&ref, g_repo,"nonexistent/master", GIT_BRANCH_REMOTE));
-	cl_git_pass(git_branch_name(&name, ref));
-	cl_assert_equal_s("nonexistent/master", name);
-
-	cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo, ref), GIT_ENOTFOUND);
-	git_reference_free(ref);
+	cl_git_fail_with(git_branch_remote_name(
+		NULL, 0, g_repo, unknown), GIT_ENOTFOUND);
 }
 
 void test_refs_branches_remote__local_remote_returns_error(void)
 {
-	git_reference *ref;
-	const char *name;
+	const char *local = "refs/heads/master";
 
-	cl_git_pass(git_branch_lookup(&ref,g_repo, "master", GIT_BRANCH_LOCAL));
-	cl_git_pass(git_branch_name(&name, ref));
-	cl_assert_equal_s("master",name);
-
-	cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo, ref), GIT_ERROR);
-	git_reference_free(ref);
+	cl_git_fail_with(git_branch_remote_name(
+		NULL, 0, g_repo, local), GIT_ERROR);
 }
 
 void test_refs_branches_remote__ambiguous_remote_returns_error(void)
 {
-	git_reference *ref;
-	const char *name;
 	git_remote *remote;
 
 	/* Create the remote */
@@ -110,10 +74,6 @@ void test_refs_branches_remote__ambiguous_remote_returns_error(void)
 
 	git_remote_free(remote);
 
-	cl_git_pass(git_branch_lookup(&ref,g_repo, "test/master", GIT_BRANCH_REMOTE));
-	cl_git_pass(git_branch_name(&name, ref));
-	cl_assert_equal_s("test/master", name);
-
-	cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo, ref), GIT_EAMBIGUOUS);
-	git_reference_free(ref);
+	cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo,
+		remote_tracking_branch_name), GIT_EAMBIGUOUS);
 }