Commit 15c30b72e16528bdf71c0343e4d238600c0df7a1

Carlos Martín Nieto 2014-09-02T13:23:54

clone: handle overly restrictive refspecs 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`.

diff --git a/src/clone.c b/src/clone.c
index 4472651..43b8390 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 4e274d8..5fe5143 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));
 }