smart: use push_glob instead of manual filtering

The code worked under the assumption that anything under `refs/tags` are
tag objects, and all the rest would be peelable to a commit. As it is
completely valid to have tags to blobs under a non `refs/tags` ref, this
would cause failures when trying to peel a tag to a commit.

Fix the broken filtering by switching to `git_revwalk_push_glob`, which
already handles this case.

Author: tiennou

src/transports/smart_protocol.c

@@ -270,50 +270,6 @@ static int store_common(transport_smart *t)
 	return 0;
 }
 
-static int fetch_setup_walk(git_revwalk **out, git_repository *repo)
-{
-	git_revwalk *walk = NULL;
-	git_strarray refs;
-	unsigned int i;
-	git_reference *ref = NULL;
-	int error;
-
-	if ((error = git_reference_list(&refs, repo)) < 0)
-		return error;
-
-	if ((error = git_revwalk_new(&walk, repo)) < 0)
-		return error;
-
-	git_revwalk_sorting(walk, GIT_SORT_TIME);
-
-	for (i = 0; i < refs.count; ++i) {
-		git_reference_free(ref);
-		ref = NULL;
-
-		/* No tags */
-		if (!git__prefixcmp(refs.strings[i], GIT_REFS_TAGS_DIR))
-			continue;
-
-		if ((error = git_reference_lookup(&ref, repo, refs.strings[i])) < 0)
-			goto on_error;
-
-		if (git_reference_type(ref) == GIT_REFERENCE_SYMBOLIC)
-			continue;
-
-		if ((error = git_revwalk_push(walk, git_reference_target(ref))) < 0)
-			goto on_error;
-	}
-
-	*out = walk;
-
-on_error:
-	if (error)
-		git_revwalk_free(walk);
-	git_reference_free(ref);
-	git_strarray_free(&refs);
-	return error;
-}
-
 static int wait_while_ack(gitno_buffer *buf)
 {
 	int error;
@@ -358,7 +314,10 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 	if ((error = git_pkt_buffer_wants(wants, count, &t->caps, &data)) < 0)
 		return error;
 
-	if ((error = fetch_setup_walk(&walk, repo)) < 0)
+	if ((error = git_revwalk_new(&walk, repo)) < 0)
+		goto on_error;
+
+	if ((error = git_revwalk_push_glob(walk, "refs/*")) < 0)
 		goto on_error;
 
 	/*

tests/network/fetchlocal.c

@@ -419,15 +419,15 @@ void test_network_fetchlocal__multi_remotes(void)
 	cl_git_pass(git_remote_fetch(test, NULL, &options, NULL));
 
 	cl_git_pass(git_reference_list(&refnames, repo));
-	cl_assert_equal_i(32, (int)refnames.count);
+	cl_assert_equal_i(33, (int)refnames.count);
 	git_strarray_free(&refnames);
 
 	cl_git_pass(git_remote_set_url(repo, "test_with_pushurl", cl_git_fixture_url("testrepo.git")));
 	cl_git_pass(git_remote_lookup(&test2, repo, "test_with_pushurl"));
 	cl_git_pass(git_remote_fetch(test2, NULL, &options, NULL));
 
 	cl_git_pass(git_reference_list(&refnames, repo));
-	cl_assert_equal_i(44, (int)refnames.count);
+	cl_assert_equal_i(45, (int)refnames.count);
 
 	git_strarray_free(&refnames);
 	git_remote_free(test);

tests/network/remote/local.c

@@ -62,7 +62,7 @@ void test_network_remote_local__retrieve_advertised_references(void)
 
 	cl_git_pass(git_remote_ls(&refs, &refs_len, remote));
 
-	cl_assert_equal_i(refs_len, 28);
+	cl_assert_equal_i(refs_len, 29);
 }
 
 void test_network_remote_local__retrieve_advertised_before_connect(void)
@@ -86,7 +86,7 @@ void test_network_remote_local__retrieve_advertised_references_after_disconnect(
 
 	cl_git_pass(git_remote_ls(&refs, &refs_len, remote));
 
-	cl_assert_equal_i(refs_len, 28);
+	cl_assert_equal_i(refs_len, 29);
 }
 
 void test_network_remote_local__retrieve_advertised_references_from_spaced_repository(void)
@@ -101,7 +101,7 @@ void test_network_remote_local__retrieve_advertised_references_from_spaced_repos
 
 	cl_git_pass(git_remote_ls(&refs, &refs_len, remote));
 
-	cl_assert_equal_i(refs_len, 28);
+	cl_assert_equal_i(refs_len, 29);
 
 	git_remote_free(remote);	/* Disconnect from the "spaced repo" before the cleanup */
 	remote = NULL;

tests/refs/foreachglob.c

@@ -48,8 +48,8 @@ static void assert_retrieval(const char *glob, int expected_count)
 
 void test_refs_foreachglob__retrieve_all_refs(void)
 {
-	/* 12 heads (including one packed head) + 1 note + 2 remotes + 7 tags */
-	assert_retrieval("*", 22);
+	/* 12 heads (including one packed head) + 1 note + 2 remotes + 7 tags + 1 blob */
+	assert_retrieval("*", 23);
 }
 
 void test_refs_foreachglob__retrieve_remote_branches(void)

tests/refs/iterator.c

@@ -15,6 +15,7 @@ void test_refs_iterator__cleanup(void)
 }
 
 static const char *refnames[] = {
+	"refs/blobs/annotated_tag_to_blob",
 	"refs/heads/br2",
 	"refs/heads/cannot-fetch",
 	"refs/heads/chomped",
@@ -40,6 +41,7 @@ static const char *refnames[] = {
 };
 
 static const char *refnames_with_symlink[] = {
+	"refs/blobs/annotated_tag_to_blob",
 	"refs/heads/br2",
 	"refs/heads/cannot-fetch",
 	"refs/heads/chomped",
@@ -99,7 +101,7 @@ void test_refs_iterator__list(void)
 	git_vector output;
 	git_reference *ref;
 
-	cl_git_pass(git_vector_init(&output, 32, &refcmp_cb));
+	cl_git_pass(git_vector_init(&output, 33, &refcmp_cb));
 	cl_git_pass(git_reference_iterator_new(&iter, repo));
 
 	while (1) {
@@ -143,7 +145,7 @@ static int refs_foreach_cb(git_reference *reference, void *payload)
 void test_refs_iterator__foreach(void)
 {
 	git_vector output;
-	cl_git_pass(git_vector_init(&output, 32, &refcmp_cb));
+	cl_git_pass(git_vector_init(&output, 33, &refcmp_cb));
 	cl_git_pass(git_reference_foreach(repo, refs_foreach_cb, &output));
 	assert_all_refnames_match(refnames, &output);
 }

tests/resources/testrepo.git/refs/blobs/annotated_tag_to_blob

@@ -0,0 +1 @@
+521d87c1ec3aef9824daf6d96cc0ae3710766d91