From 8ec889a45fded32bf8508f99d77ea666d0aacdd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 2 Nov 2013 14:07:02 +0100 Subject: [PATCH] branch: move from foreach to an iterator Create a git_branch_iterator type which is equivalent to the foreach but lets us write loops instead of callbacks. Since the introduction of git_reference_shorthand(), the added value of passing the name is reduced. --- include/git2/branch.h | 42 ++++---- src/branch.c | 86 +++++++++------- tests-clar/online/push.c | 37 +++---- .../refs/branches/{foreach.c => iterator.c} | 98 +++++++------------ 4 files changed, 130 insertions(+), 133 deletions(-) rename tests-clar/refs/branches/{foreach.c => iterator.c} (55%) diff --git a/include/git2/branch.h b/include/git2/branch.h index de414e9b0..b5e7d60ea 100644 --- a/include/git2/branch.h +++ b/include/git2/branch.h @@ -66,33 +66,41 @@ GIT_EXTERN(int) git_branch_create( */ GIT_EXTERN(int) git_branch_delete(git_reference *branch); -typedef int (*git_branch_foreach_cb)( - const char *branch_name, - git_branch_t branch_type, - void *payload); +/** Iterator type for branches */ +typedef struct git_branch_iterator git_branch_iterator; /** - * Loop over all the branches and issue a callback for each one. - * - * If the callback returns a non-zero value, this will stop looping. + * Create an iterator which loops over the requested branches. * + * @param out the iterator * @param repo Repository where to find the branches. - * * @param list_flags Filtering flags for the branch * listing. Valid values are GIT_BRANCH_LOCAL, GIT_BRANCH_REMOTE * or a combination of the two. * - * @param branch_cb Callback to invoke per found branch. - * - * @param payload Extra parameter to callback function. - * - * @return 0 on success, GIT_EUSER on non-zero callback, or error code + * @return 0 on success or an error code */ -GIT_EXTERN(int) git_branch_foreach( +GIT_EXTERN(int) git_branch_iterator_new( + git_branch_iterator **out, git_repository *repo, - unsigned int list_flags, - git_branch_foreach_cb branch_cb, - void *payload); + unsigned int list_flags); + +/** + * Retrieve the next branch from the iterator + * + * @param out the reference + * @param out_type the type of branch (local or remote-tracking) + * @param iter the branch iterator + * @return 0 on success, GIT_ITEROVER if there are no more branches or an error code. + */ +GIT_EXTERN(int) git_branch_next(git_reference **out, unsigned int *out_type, git_branch_iterator *iter); + +/** + * Free a branch iterator + * + * @param iter the iterator to free + */ +GIT_EXTERN(void) git_branch_iterator_free(git_branch_iterator *iter); /** * Move/rename an existing local branch reference. diff --git a/src/branch.c b/src/branch.c index 3a745c127..4660f00b7 100644 --- a/src/branch.c +++ b/src/branch.c @@ -124,50 +124,68 @@ on_error: return error; } -int git_branch_foreach( - git_repository *repo, - unsigned int list_flags, - git_branch_foreach_cb callback, - void *payload) -{ +typedef struct { git_reference_iterator *iter; + unsigned int flags; +} branch_iter; + +int git_branch_next(git_reference **out, unsigned int *out_type, git_branch_iterator *_iter) +{ + branch_iter *iter = (branch_iter *) _iter; git_reference *ref; - int error = 0; + int error; - if (git_reference_iterator_new(&iter, repo) < 0) - return -1; + while ((error = git_reference_next(&ref, iter->iter)) == 0) { + if ((iter->flags & GIT_BRANCH_LOCAL) && + !git__prefixcmp(ref->name, GIT_REFS_HEADS_DIR)) { + *out = ref; + *out_type = GIT_BRANCH_LOCAL; - while ((error = git_reference_next(&ref, iter)) == 0) { - if (list_flags & GIT_BRANCH_LOCAL && - git__prefixcmp(ref->name, GIT_REFS_HEADS_DIR) == 0) { - if (callback(ref->name + strlen(GIT_REFS_HEADS_DIR), - GIT_BRANCH_LOCAL, payload)) { - error = GIT_EUSER; - } + return 0; + } else if ((iter->flags & GIT_BRANCH_REMOTE) && + !git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR)) { + *out = ref; + *out_type = GIT_BRANCH_REMOTE; + + return 0; + } else { + git_reference_free(ref); } - - if (list_flags & GIT_BRANCH_REMOTE && - git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR) == 0) { - if (callback(ref->name + strlen(GIT_REFS_REMOTES_DIR), - GIT_BRANCH_REMOTE, payload)) { - error = GIT_EUSER; - } - } - - git_reference_free(ref); - - /* check if the callback has cancelled iteration */ - if (error == GIT_EUSER) - break; } - if (error == GIT_ITEROVER) - error = 0; - - git_reference_iterator_free(iter); return error; } +int git_branch_iterator_new( + git_branch_iterator **out, + git_repository *repo, + unsigned int list_flags) +{ + branch_iter *iter; + + iter = git__calloc(1, sizeof(branch_iter)); + GITERR_CHECK_ALLOC(iter); + + iter->flags = list_flags; + + if (git_reference_iterator_new(&iter->iter, repo) < 0) { + git__free(iter); + return -1; + } + + *out = (git_branch_iterator *) iter; + + return 0; +} + +void git_branch_iterator_free(git_branch_iterator *_iter) +{ + branch_iter *iter = (branch_iter *) _iter; + + git_reference_iterator_free(iter->iter); + git__free(iter); +} + int git_branch_move( git_reference **out, git_reference *branch, diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c index 5a747bba7..320ecb71e 100644 --- a/tests-clar/online/push.c +++ b/tests-clar/online/push.c @@ -163,18 +163,6 @@ static void verify_refs(git_remote *remote, expected_ref expected_refs[], size_t git_vector_free(&actual_refs); } -static int tracking_branch_list_cb(const char *branch_name, git_branch_t branch_type, void *payload) -{ - git_vector *tracking = (git_vector *)payload; - - if (branch_type == GIT_BRANCH_REMOTE) - git_vector_insert(tracking, git__strdup(branch_name)); - else - GIT_UNUSED(branch_name); - - return 0; -} - /** * Verifies that after git_push_update_tips(), remote tracking branches have the expected * names and oids. @@ -189,14 +177,24 @@ static void verify_tracking_branches(git_remote *remote, expected_ref expected_r size_t i, j; git_buf msg = GIT_BUF_INIT; git_buf ref_name = GIT_BUF_INIT; - git_buf canonical_ref_name = GIT_BUF_INIT; git_vector actual_refs = GIT_VECTOR_INIT; + git_branch_iterator *iter; char *actual_ref; git_oid oid; - int failed = 0; + int failed = 0, error; + unsigned int branch_type; + git_reference *ref; /* Get current remote branches */ - cl_git_pass(git_branch_foreach(remote->repo, GIT_BRANCH_REMOTE, tracking_branch_list_cb, &actual_refs)); + cl_git_pass(git_branch_iterator_new(&iter, remote->repo, GIT_BRANCH_REMOTE)); + + while ((error = git_branch_next(&ref, &branch_type, iter)) == 0) { + cl_assert_equal_i(branch_type, GIT_BRANCH_REMOTE); + + cl_git_pass(git_vector_insert(&actual_refs, git__strdup(git_reference_name(ref)))); + } + + cl_assert_equal_i(error, GIT_ITEROVER); /* Loop through expected refs, make sure they exist */ for (i = 0; i < expected_refs_len; i++) { @@ -212,11 +210,7 @@ static void verify_tracking_branches(git_remote *remote, expected_ref expected_r /* Find matching remote branch */ git_vector_foreach(&actual_refs, j, actual_ref) { - - /* Construct canonical ref name from the actual_ref name */ - git_buf_clear(&canonical_ref_name); - cl_git_pass(git_buf_printf(&canonical_ref_name, "refs/remotes/%s", actual_ref)); - if (!strcmp(git_buf_cstr(&ref_name), git_buf_cstr(&canonical_ref_name))) + if (!strcmp(git_buf_cstr(&ref_name), actual_ref)) break; } @@ -227,7 +221,7 @@ static void verify_tracking_branches(git_remote *remote, expected_ref expected_r } /* Make sure tracking branch is at expected commit ID */ - cl_git_pass(git_reference_name_to_id(&oid, remote->repo, git_buf_cstr(&canonical_ref_name))); + cl_git_pass(git_reference_name_to_id(&oid, remote->repo, actual_ref)); if (git_oid_cmp(expected_refs[i].oid, &oid) != 0) { git_buf_puts(&msg, "Tracking branch commit does not match expected ID."); @@ -256,7 +250,6 @@ failed: git_vector_free(&actual_refs); git_buf_free(&msg); - git_buf_free(&canonical_ref_name); git_buf_free(&ref_name); return; } diff --git a/tests-clar/refs/branches/foreach.c b/tests-clar/refs/branches/iterator.c similarity index 55% rename from tests-clar/refs/branches/foreach.c rename to tests-clar/refs/branches/iterator.c index 433812cb4..fb2c1a19d 100644 --- a/tests-clar/refs/branches/foreach.c +++ b/tests-clar/refs/branches/iterator.c @@ -4,7 +4,7 @@ static git_repository *repo; static git_reference *fake_remote; -void test_refs_branches_foreach__initialize(void) +void test_refs_branches_iterator__initialize(void) { git_oid id; @@ -15,7 +15,7 @@ void test_refs_branches_foreach__initialize(void) cl_git_pass(git_reference_create(&fake_remote, repo, "refs/remotes/nulltoken/master", &id, 0)); } -void test_refs_branches_foreach__cleanup(void) +void test_refs_branches_iterator__cleanup(void) { git_reference_free(fake_remote); fake_remote = NULL; @@ -28,39 +28,35 @@ void test_refs_branches_foreach__cleanup(void) cl_git_sandbox_cleanup(); } -static int count_branch_list_cb(const char *branch_name, git_branch_t branch_type, void *payload) -{ - int *count; - - GIT_UNUSED(branch_type); - GIT_UNUSED(branch_name); - - count = (int *)payload; - (*count)++; - - return 0; -} - static void assert_retrieval(unsigned int flags, unsigned int expected_count) { - int count = 0; + git_branch_iterator *iter; + git_reference *ref; + int count = 0, error; + unsigned int type; - cl_git_pass(git_branch_foreach(repo, flags, count_branch_list_cb, &count)); + cl_git_pass(git_branch_iterator_new(&iter, repo, flags)); + while ((error = git_branch_next(&ref, &type, iter)) == 0) { + count++; + git_reference_free(ref); + } + git_branch_iterator_free(iter); + cl_assert_equal_i(error, GIT_ITEROVER); cl_assert_equal_i(expected_count, count); } -void test_refs_branches_foreach__retrieve_all_branches(void) +void test_refs_branches_iterator__retrieve_all_branches(void) { assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 14); } -void test_refs_branches_foreach__retrieve_remote_branches(void) +void test_refs_branches_iterator__retrieve_remote_branches(void) { assert_retrieval(GIT_BRANCH_REMOTE, 2); } -void test_refs_branches_foreach__retrieve_local_branches(void) +void test_refs_branches_iterator__retrieve_local_branches(void) { assert_retrieval(GIT_BRANCH_LOCAL, 12); } @@ -84,21 +80,22 @@ static void assert_branch_has_been_found(struct expectations *findings, const ch cl_fail("expected branch not found in list."); } -static int contains_branch_list_cb(const char *branch_name, git_branch_t branch_type, void *payload) +static void contains_branches(struct expectations exp[], git_branch_iterator *iter) { - int pos = 0; - struct expectations *exp; + git_reference *ref; + unsigned int type; + int error, pos = 0; - GIT_UNUSED(branch_type); + while ((error = git_branch_next(&ref, &type, iter)) == 0) { + for (pos = 0; exp[pos].branch_name; ++pos) { + if (strcmp(git_reference_shorthand(ref), exp[pos].branch_name) == 0) + exp[pos].encounters++; + } - exp = (struct expectations *)payload; - - for (pos = 0; exp[pos].branch_name; ++pos) { - if (strcmp(branch_name, exp[pos].branch_name) == 0) - exp[pos].encounters++; + git_reference_free(ref); } - return 0; + cl_assert_equal_i(error, GIT_ITEROVER); } /* @@ -106,8 +103,9 @@ static int contains_branch_list_cb(const char *branch_name, git_branch_t branch_ * nulltoken/HEAD -> nulltoken/master * nulltoken/master */ -void test_refs_branches_foreach__retrieve_remote_symbolic_HEAD_when_present(void) +void test_refs_branches_iterator__retrieve_remote_symbolic_HEAD_when_present(void) { + git_branch_iterator *iter; struct expectations exp[] = { { "nulltoken/HEAD", 0 }, { "nulltoken/master", 0 }, @@ -119,39 +117,17 @@ void test_refs_branches_foreach__retrieve_remote_symbolic_HEAD_when_present(void assert_retrieval(GIT_BRANCH_REMOTE, 3); - cl_git_pass(git_branch_foreach(repo, GIT_BRANCH_REMOTE, contains_branch_list_cb, &exp)); + cl_git_pass(git_branch_iterator_new(&iter, repo, GIT_BRANCH_REMOTE)); + contains_branches(exp, iter); + git_branch_iterator_free(iter); assert_branch_has_been_found(exp, "nulltoken/HEAD"); assert_branch_has_been_found(exp, "nulltoken/master"); } -static int branch_list_interrupt_cb( - const char *branch_name, git_branch_t branch_type, void *payload) -{ - int *count; - - GIT_UNUSED(branch_type); - GIT_UNUSED(branch_name); - - count = (int *)payload; - (*count)++; - - return (*count == 5); -} - -void test_refs_branches_foreach__can_cancel(void) -{ - int count = 0; - - cl_assert_equal_i(GIT_EUSER, - git_branch_foreach(repo, GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, - branch_list_interrupt_cb, &count)); - - cl_assert_equal_i(5, count); -} - -void test_refs_branches_foreach__mix_of_packed_and_loose(void) +void test_refs_branches_iterator__mix_of_packed_and_loose(void) { + git_branch_iterator *iter; struct expectations exp[] = { { "master", 0 }, { "origin/HEAD", 0 }, @@ -163,8 +139,10 @@ void test_refs_branches_foreach__mix_of_packed_and_loose(void) r2 = cl_git_sandbox_init("testrepo2"); - cl_git_pass(git_branch_foreach(r2, GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, - contains_branch_list_cb, &exp)); + cl_git_pass(git_branch_iterator_new(&iter, r2, GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE)); + contains_branches(exp, iter); + + git_branch_iterator_free(iter); assert_branch_has_been_found(exp, "master"); assert_branch_has_been_found(exp, "origin/HEAD");