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: 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 | 40 +++++---- src/branch.c | 82 ++++++++++------- tests-clar/online/push.c | 37 ++++---- tests-clar/refs/branches/foreach.c | 173 ------------------------------------ tests-clar/refs/branches/iterator.c | 151 +++++++++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 243 deletions(-) delete mode 100644 tests-clar/refs/branches/foreach.c create mode 100644 tests-clar/refs/branches/iterator.c 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. + * @return 0 on success or an error code + */ +GIT_EXTERN(int) git_branch_iterator_new( + git_branch_iterator **out, + git_repository *repo, + unsigned int list_flags); + +/** + * Retrieve the next branch from the iterator * - * @param payload Extra parameter to callback function. + * @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 * - * @return 0 on success, GIT_EUSER on non-zero callback, or error code + * @param iter the iterator to free */ -GIT_EXTERN(int) git_branch_foreach( - git_repository *repo, - unsigned int list_flags, - git_branch_foreach_cb branch_cb, - void *payload); +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,48 +124,66 @@ 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; - git_reference *ref; - int error = 0; + unsigned int flags; +} branch_iter; - if (git_reference_iterator_new(&iter, repo) < 0) - return -1; +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; - 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; - } + 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; + + 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; - } - } + 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); - git_reference_free(ref); + iter->flags = list_flags; - /* check if the callback has cancelled iteration */ - if (error == GIT_EUSER) - break; + if (git_reference_iterator_new(&iter->iter, repo) < 0) { + git__free(iter); + return -1; } - if (error == GIT_ITEROVER) - error = 0; + *out = (git_branch_iterator *) iter; - git_reference_iterator_free(iter); - return error; + 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( 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/foreach.c deleted file mode 100644 index 433812cb4..000000000 --- a/tests-clar/refs/branches/foreach.c +++ /dev/null @@ -1,173 +0,0 @@ -#include "clar_libgit2.h" -#include "refs.h" - -static git_repository *repo; -static git_reference *fake_remote; - -void test_refs_branches_foreach__initialize(void) -{ - git_oid id; - - cl_fixture_sandbox("testrepo.git"); - cl_git_pass(git_repository_open(&repo, "testrepo.git")); - - cl_git_pass(git_oid_fromstr(&id, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644")); - cl_git_pass(git_reference_create(&fake_remote, repo, "refs/remotes/nulltoken/master", &id, 0)); -} - -void test_refs_branches_foreach__cleanup(void) -{ - git_reference_free(fake_remote); - fake_remote = NULL; - - git_repository_free(repo); - repo = NULL; - - cl_fixture_cleanup("testrepo.git"); - - 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; - - cl_git_pass(git_branch_foreach(repo, flags, count_branch_list_cb, &count)); - - cl_assert_equal_i(expected_count, count); -} - -void test_refs_branches_foreach__retrieve_all_branches(void) -{ - assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 14); -} - -void test_refs_branches_foreach__retrieve_remote_branches(void) -{ - assert_retrieval(GIT_BRANCH_REMOTE, 2); -} - -void test_refs_branches_foreach__retrieve_local_branches(void) -{ - assert_retrieval(GIT_BRANCH_LOCAL, 12); -} - -struct expectations { - const char *branch_name; - int encounters; -}; - -static void assert_branch_has_been_found(struct expectations *findings, const char* expected_branch_name) -{ - int pos = 0; - - for (pos = 0; findings[pos].branch_name; ++pos) { - if (strcmp(expected_branch_name, findings[pos].branch_name) == 0) { - cl_assert_equal_i(1, findings[pos].encounters); - return; - } - } - - 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) -{ - int pos = 0; - struct expectations *exp; - - GIT_UNUSED(branch_type); - - exp = (struct expectations *)payload; - - for (pos = 0; exp[pos].branch_name; ++pos) { - if (strcmp(branch_name, exp[pos].branch_name) == 0) - exp[pos].encounters++; - } - - return 0; -} - -/* - * $ git branch -r - * nulltoken/HEAD -> nulltoken/master - * nulltoken/master - */ -void test_refs_branches_foreach__retrieve_remote_symbolic_HEAD_when_present(void) -{ - struct expectations exp[] = { - { "nulltoken/HEAD", 0 }, - { "nulltoken/master", 0 }, - { NULL, 0 } - }; - - git_reference_free(fake_remote); - cl_git_pass(git_reference_symbolic_create(&fake_remote, repo, "refs/remotes/nulltoken/HEAD", "refs/remotes/nulltoken/master", 0)); - - assert_retrieval(GIT_BRANCH_REMOTE, 3); - - cl_git_pass(git_branch_foreach(repo, GIT_BRANCH_REMOTE, contains_branch_list_cb, &exp)); - - 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) -{ - struct expectations exp[] = { - { "master", 0 }, - { "origin/HEAD", 0 }, - { "origin/master", 0 }, - { "origin/packed", 0 }, - { NULL, 0 } - }; - git_repository *r2; - - r2 = cl_git_sandbox_init("testrepo2"); - - cl_git_pass(git_branch_foreach(r2, GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, - contains_branch_list_cb, &exp)); - - assert_branch_has_been_found(exp, "master"); - assert_branch_has_been_found(exp, "origin/HEAD"); - assert_branch_has_been_found(exp, "origin/master"); - assert_branch_has_been_found(exp, "origin/packed"); -} diff --git a/tests-clar/refs/branches/iterator.c b/tests-clar/refs/branches/iterator.c new file mode 100644 index 000000000..fb2c1a19d --- /dev/null +++ b/tests-clar/refs/branches/iterator.c @@ -0,0 +1,151 @@ +#include "clar_libgit2.h" +#include "refs.h" + +static git_repository *repo; +static git_reference *fake_remote; + +void test_refs_branches_iterator__initialize(void) +{ + git_oid id; + + cl_fixture_sandbox("testrepo.git"); + cl_git_pass(git_repository_open(&repo, "testrepo.git")); + + cl_git_pass(git_oid_fromstr(&id, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644")); + cl_git_pass(git_reference_create(&fake_remote, repo, "refs/remotes/nulltoken/master", &id, 0)); +} + +void test_refs_branches_iterator__cleanup(void) +{ + git_reference_free(fake_remote); + fake_remote = NULL; + + git_repository_free(repo); + repo = NULL; + + cl_fixture_cleanup("testrepo.git"); + + cl_git_sandbox_cleanup(); +} + +static void assert_retrieval(unsigned int flags, unsigned int expected_count) +{ + git_branch_iterator *iter; + git_reference *ref; + int count = 0, error; + unsigned int type; + + 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_iterator__retrieve_all_branches(void) +{ + assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 14); +} + +void test_refs_branches_iterator__retrieve_remote_branches(void) +{ + assert_retrieval(GIT_BRANCH_REMOTE, 2); +} + +void test_refs_branches_iterator__retrieve_local_branches(void) +{ + assert_retrieval(GIT_BRANCH_LOCAL, 12); +} + +struct expectations { + const char *branch_name; + int encounters; +}; + +static void assert_branch_has_been_found(struct expectations *findings, const char* expected_branch_name) +{ + int pos = 0; + + for (pos = 0; findings[pos].branch_name; ++pos) { + if (strcmp(expected_branch_name, findings[pos].branch_name) == 0) { + cl_assert_equal_i(1, findings[pos].encounters); + return; + } + } + + cl_fail("expected branch not found in list."); +} + +static void contains_branches(struct expectations exp[], git_branch_iterator *iter) +{ + git_reference *ref; + unsigned int type; + int error, pos = 0; + + 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++; + } + + git_reference_free(ref); + } + + cl_assert_equal_i(error, GIT_ITEROVER); +} + +/* + * $ git branch -r + * nulltoken/HEAD -> nulltoken/master + * nulltoken/master + */ +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 }, + { NULL, 0 } + }; + + git_reference_free(fake_remote); + cl_git_pass(git_reference_symbolic_create(&fake_remote, repo, "refs/remotes/nulltoken/HEAD", "refs/remotes/nulltoken/master", 0)); + + assert_retrieval(GIT_BRANCH_REMOTE, 3); + + 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"); +} + +void test_refs_branches_iterator__mix_of_packed_and_loose(void) +{ + git_branch_iterator *iter; + struct expectations exp[] = { + { "master", 0 }, + { "origin/HEAD", 0 }, + { "origin/master", 0 }, + { "origin/packed", 0 }, + { NULL, 0 } + }; + git_repository *r2; + + r2 = cl_git_sandbox_init("testrepo2"); + + 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"); + assert_branch_has_been_found(exp, "origin/master"); + assert_branch_has_been_found(exp, "origin/packed"); +} -- cgit v1.2.1