clone: handle overly restrictive refspecscmn/remote-default-restrict

When the fetch refspec does not include the remote's default branch, it
indicates an error in user expectations or programmer error. Error out
in that case.

This lets us get rid of the dummy refspec which can never work as its
zeroed out. In the cases where we did not find a default branch, we set
HEAD detached immediately, which lets us refactor the "normal" path,
removing `found_branch`.

2 files changed, 23 insertions, 35 deletions

diff --git a/src/clone.c b/src/clone.c
index 4472651da..43b839003 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -144,9 +144,9 @@ static int update_head_to_remote(
 		const git_signature *signature,
 		const char *reflog_message)
 {
-	int error = 0, found_branch = 0;
+	int error = 0;
 	size_t refs_len;
-	git_refspec dummy_spec, *refspec;
+	git_refspec *refspec;
 	const git_remote_head *remote_head, **refs;
 	const git_oid *remote_head_id;
 	git_buf remote_master_name = GIT_BUF_INIT;
@@ -160,23 +160,25 @@ static int update_head_to_remote(
 		return setup_tracking_config(
 			repo, "master", GIT_REMOTE_ORIGIN, GIT_REFS_HEADS_MASTER_FILE);
 
-	error = git_remote_default_branch(&branch, remote);
-	if (error == GIT_ENOTFOUND) {
-		git_buf_puts(&branch, GIT_REFS_HEADS_MASTER_FILE);
-	} else {
-		found_branch = 1;
-	}
-
-	/* Get the remote's HEAD. This is always the first ref in the list if it exists */
+	/* We know we have HEAD, let's see where it points */
 	remote_head = refs[0];
 	assert(remote_head);
 
 	remote_head_id = &remote_head->oid;
+
+	error = git_remote_default_branch(&branch, remote);
+	if (error == GIT_ENOTFOUND) {
+		error = git_repository_set_head_detached(
+			repo, remote_head_id, signature, reflog_message);
+		goto cleanup;
+	}
+
 	refspec = git_remote__matching_refspec(remote, git_buf_cstr(&branch));
 
 	if (refspec == NULL) {
-		memset(&dummy_spec, 0, sizeof(git_refspec));
-		refspec = &dummy_spec;
+		giterr_set(GITERR_NET, "the remote's default branch does not fit the refspec configuration");
+		error = GIT_EINVALIDSPEC;
+		goto cleanup;
 	}
 
 	/* Determine the remote tracking reference name from the local master */
@@ -184,21 +186,18 @@ static int update_head_to_remote(
 		&remote_master_name,
 		refspec,
 		git_buf_cstr(&branch))) < 0)
-		return error;
+		goto cleanup;
 
-	if (found_branch) {
-		error = update_head_to_new_branch(
-			repo,
-			remote_head_id,
-			git_buf_cstr(&branch),
-			signature, reflog_message);
-	} else {
-		error = git_repository_set_head_detached(
-			repo, remote_head_id, signature, reflog_message);
-	}
+	error = update_head_to_new_branch(
+		repo,
+		remote_head_id,
+		git_buf_cstr(&branch),
+		signature, reflog_message);
 
+cleanup:
 	git_buf_free(&remote_master_name);
 	git_buf_free(&branch);
+
 	return error;
 }
 
diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c
index 4e274d889..5fe5143f5 100644
--- a/tests/network/remote/remotes.c
+++ b/tests/network/remote/remotes.c
@@ -556,19 +556,8 @@ void test_network_remote_remotes__restricted_refspecs(void)
 {
 	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
 	git_repository *repo;
-	git_strarray refs;
-	size_t i, count = 0;
 
 	opts.remote_cb = remote_single_branch;
 
-	cl_git_pass(git_clone(&repo, "git://github.com/libgit2/TestGitRepository", "./restrict-refspec", &opts));
-	cl_git_pass(git_reference_list(&refs, repo));
-
-	for (i = 0; i < refs.count; i++) {
-		if (!git__prefixcmp(refs.strings[i], "refs/heads/"))
-			count++;
-	}
-	cl_assert_equal_i(1, count);
-
-	git_repository_free(repo);
+	cl_git_fail_with(GIT_EINVALIDSPEC, git_clone(&repo, "git://github.com/libgit2/TestGitRepository", "./restrict-refspec", &opts));
 }