From 081d229106faae3c37c08940fd510a3511bfbd59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 3 Mar 2012 04:46:21 +0100 Subject: [PATCH 1/8] revwalk: don't assume malloc succeeds --- src/revwalk.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 997771f08..ffa0be6f8 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -58,6 +58,9 @@ struct git_revwalk { static commit_list *commit_list_insert(commit_object *item, commit_list **list_p) { commit_list *new_list = git__malloc(sizeof(commit_list)); + if (new_list == NULL) + return NULL; + new_list->item = item; new_list->next = *list_p; *list_p = new_list; @@ -490,7 +493,8 @@ static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) if (--parent->in_degree == 0 && parent->topo_delay) { parent->topo_delay = 0; - commit_list_insert(parent, &walk->iterator_topo); + if (commit_list_insert(parent, &walk->iterator_topo) == NULL) + return GIT_ENOMEM; } } @@ -520,7 +524,8 @@ static int prepare_walk(git_revwalk *walk) parent->in_degree++; } - commit_list_insert(next, &walk->iterator_topo); + if (commit_list_insert(next, &walk->iterator_topo) == NULL) + return GIT_ENOMEM; } if (error != GIT_EREVWALKOVER) @@ -532,7 +537,8 @@ static int prepare_walk(git_revwalk *walk) if (walk->sorting & GIT_SORT_REVERSE) { while ((error = walk->get_next(&next, walk)) == GIT_SUCCESS) - commit_list_insert(next, &walk->iterator_reverse); + if (commit_list_insert(next, &walk->iterator_reverse) == NULL) + return GIT_ENOMEM; if (error != GIT_EREVWALKOVER) return git__rethrow(error, "Failed to prepare revision walk"); From 06b9d915901b3dd9dc85016bb000e631eb1da1d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 28 Feb 2012 02:19:57 +0100 Subject: [PATCH 2/8] revwalk: allow pushing/hiding a reference by name The code was already there, so factor it out and let users push an OID by giving it a reference name. Only refs to commits are supported. Annotated tags will throw an error. --- include/git2/revwalk.h | 22 ++++++++++++ src/revwalk.c | 71 ++++++++++++++++++-------------------- tests-clar/revwalk/basic.c | 5 ++- 3 files changed, 57 insertions(+), 41 deletions(-) diff --git a/include/git2/revwalk.h b/include/git2/revwalk.h index e7ec2abf3..632c67588 100644 --- a/include/git2/revwalk.h +++ b/include/git2/revwalk.h @@ -163,6 +163,28 @@ GIT_EXTERN(int) git_revwalk_hide_glob(git_revwalk *walk, const char *glob); */ GIT_EXTERN(int) git_revwalk_hide_head(git_revwalk *walk); +/** + * Push the OID pointed to by a reference + * + * The reference must point to a commit. + * + * @param walk the walker being used for the traversal + * @param refname the referece to push + * @return GIT_SUCCESS or an error code + */ +GIT_EXTERN(int) git_revwalk_push_ref(git_revwalk *walk, const char *refname); + +/** + * Hide the OID pointed to by a reference + * + * The reference must point to a commit. + * + * @param walk the walker being used for the traversal + * @param refname the referece to hide + * @return GIT_SUCCESS or an error code + */ +GIT_EXTERN(int) git_revwalk_hide_ref(git_revwalk *walk, const char *refname); + /** * Get the next commit from the revision walk. * diff --git a/src/revwalk.c b/src/revwalk.c index ffa0be6f8..1a398ce2d 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -310,6 +310,23 @@ int git_revwalk_hide(git_revwalk *walk, const git_oid *oid) return push_commit(walk, oid, 1); } +static int push_ref(git_revwalk *walk, const char *refname, int hide) +{ + git_reference *ref, *resolved; + int error; + + error = git_reference_lookup(&ref, walk->repo, refname); + if (error < GIT_SUCCESS) + return error; + error = git_reference_resolve(&resolved, ref); + git_reference_free(ref); + if (error < GIT_SUCCESS) + return error; + error = push_commit(walk, git_reference_oid(resolved), hide); + git_reference_free(resolved); + return error; +} + struct push_cb_data { git_revwalk *walk; const char *glob; @@ -320,21 +337,8 @@ static int push_glob_cb(const char *refname, void *data_) { struct push_cb_data *data = (struct push_cb_data *)data_; - if (!git__fnmatch(data->glob, refname, 0)) { - git_reference *ref, *resolved; - int error; - - error = git_reference_lookup(&ref, data->walk->repo, refname); - if (error < GIT_SUCCESS) - return error; - error = git_reference_resolve(&resolved, ref); - git_reference_free(ref); - if (error < GIT_SUCCESS) - return error; - error = push_commit(data->walk, git_reference_oid(resolved), data->hide); - git_reference_free(resolved); - return error; - } + if (!git__fnmatch(data->glob, refname, 0)) + return push_ref(data->walk, refname, data->hide); return GIT_SUCCESS; } @@ -394,37 +398,28 @@ int git_revwalk_hide_glob(git_revwalk *walk, const char *glob) return push_glob(walk, glob, 1); } -static int push_head(git_revwalk *walk, int hide) -{ - git_reference *ref, *resolved; - int error; - - error = git_reference_lookup(&ref, walk->repo, "HEAD"); - if (error < GIT_SUCCESS) { - return error; - } - error = git_reference_resolve(&resolved, ref); - if (error < GIT_SUCCESS) { - return error; - } - git_reference_free(ref); - - error = push_commit(walk, git_reference_oid(resolved), hide); - - git_reference_free(resolved); - return error; -} - int git_revwalk_push_head(git_revwalk *walk) { assert(walk); - return push_head(walk, 0); + return push_ref(walk, GIT_HEAD_FILE, 0); } int git_revwalk_hide_head(git_revwalk *walk) { assert(walk); - return push_head(walk, 1); + return push_ref(walk, GIT_HEAD_FILE, 1); +} + +int git_revwalk_push_ref(git_revwalk *walk, const char *refname) +{ + assert(walk && refname); + return push_ref(walk, refname, 0); +} + +int git_revwalk_hide_ref(git_revwalk *walk, const char *refname) +{ + assert(walk && refname); + return push_ref(walk, refname, 1); } static int revwalk_enqueue_timesort(git_revwalk *walk, commit_object *commit) diff --git a/tests-clar/revwalk/basic.c b/tests-clar/revwalk/basic.c index cc88ec65b..a364d82b0 100644 --- a/tests-clar/revwalk/basic.c +++ b/tests-clar/revwalk/basic.c @@ -148,14 +148,13 @@ void test_revwalk_basic__push_head(void) cl_assert(i == 7); } -void test_revwalk_basic__push_head_hide_glob(void) +void test_revwalk_basic__push_head_hide_ref(void) { int i = 0; git_oid oid; cl_git_pass(git_revwalk_push_head(_walk)); - /* This is a hack, as we know this will only match the packed-test branch */ - cl_git_pass(git_revwalk_hide_glob(_walk, "heads/packed-test*")); + cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/packed-test")); while (git_revwalk_next(&oid, _walk) == GIT_SUCCESS) { i++; From de7ab85dc614ba702a89f3f0c761c68ee1e00fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 3 Mar 2012 03:31:51 +0100 Subject: [PATCH 3/8] Implement git_merge_base() It's implemented in revwalk.c so it has access to the revision walker's commit cache and related functions. The algorithm is the one used by git, modified so it fits better with the library's functions. --- include/git2/revwalk.h | 3 + src/revwalk.c | 176 +++++++++++++++++++++++++++++++-- tests-clar/revwalk/mergebase.c | 37 +++++++ 3 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 tests-clar/revwalk/mergebase.c diff --git a/include/git2/revwalk.h b/include/git2/revwalk.h index 632c67588..db27c62b1 100644 --- a/include/git2/revwalk.h +++ b/include/git2/revwalk.h @@ -232,6 +232,9 @@ GIT_EXTERN(void) git_revwalk_free(git_revwalk *walk); */ GIT_EXTERN(git_repository *) git_revwalk_repository(git_revwalk *walk); +GIT_EXTERN(int) git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *two); + + /** @} */ GIT_END_DECL #endif diff --git a/src/revwalk.c b/src/revwalk.c index 1a398ce2d..92885d688 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -15,13 +15,19 @@ #include +#define PARENT1 (1 << 0) +#define PARENT2 (1 << 1) +#define RESULT (1 << 2) +#define STALE (1 << 3) + typedef struct commit_object { git_oid oid; uint32_t time; unsigned int seen:1, uninteresting:1, topo_delay:1, - parsed:1; + parsed:1, + flags : 4; unsigned short in_degree; unsigned short out_degree; @@ -55,6 +61,14 @@ struct git_revwalk { unsigned int sorting; }; +static int commit_time_cmp(void *a, void *b) +{ + commit_object *commit_a = (commit_object *)a; + commit_object *commit_b = (commit_object *)b; + + return (commit_a->time < commit_b->time); +} + static commit_list *commit_list_insert(commit_object *item, commit_list **list_p) { commit_list *new_list = git__malloc(sizeof(commit_list)); @@ -67,6 +81,20 @@ static commit_list *commit_list_insert(commit_object *item, commit_list **list_p return new_list; } +static commit_list *commit_list_insert_by_date(commit_object *item, commit_list **list_p) +{ + commit_list **pp = list_p; + commit_list *p; + + while ((p = *pp) != NULL) { + if (commit_time_cmp(p->item, item) < 0) + break; + + pp = &p->next; + } + + return commit_list_insert(item, pp); +} static void commit_list_free(commit_list **list_p) { commit_list *list = *list_p; @@ -92,14 +120,6 @@ static commit_object *commit_list_pop(commit_list **stack) return item; } -static int commit_time_cmp(void *a, void *b) -{ - commit_object *commit_a = (commit_object *)a; - commit_object *commit_b = (commit_object *)b; - - return (commit_a->time < commit_b->time); -} - static uint32_t object_table_hash(const void *key, int hash_id) { uint32_t r; @@ -244,6 +264,144 @@ static int commit_parse(git_revwalk *walk, commit_object *commit) return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse commit"); } +static commit_object *interesting(commit_list *list) +{ + while (list) { + commit_object *commit = list->item; + list = list->next; + if (commit->flags & STALE) + continue; + + return commit; + } + + return NULL; +} + +static int merge_bases_many(commit_list **out, git_revwalk *walk, commit_object *one, git_vector *twos) +{ + int error; + unsigned int i; + commit_object *two; + commit_list *list = NULL, *result = NULL; + + /* if the commit is repeated, we have a our merge base already */ + git_vector_foreach(twos, i, two) { + if (one == two) + return commit_list_insert(one, out) ? GIT_SUCCESS : GIT_ENOMEM; + } + + if ((error = commit_parse(walk, one)) < GIT_SUCCESS) + return error; + + one->flags |= PARENT1; + if (commit_list_insert(one, &list) == NULL) + return GIT_ENOMEM; + + git_vector_foreach(twos, i, two) { + commit_parse(walk, two); + two->flags |= PARENT2; + if (commit_list_insert_by_date(two, &list) == NULL) + return GIT_ENOMEM; + } + + /* as long as there are non-STALE commits */ + while (interesting(list)) { + commit_object *commit = list->item; + commit_list *next; + int flags; + + next = list->next; + git__free(list); + list = next; + + flags = commit->flags & (PARENT1 | PARENT2 | STALE); + if (flags == (PARENT1 | PARENT2)) { + if (!(commit->flags & RESULT)) { + commit->flags |= RESULT; + if (commit_list_insert_by_date(commit, &result) == NULL) + return GIT_ENOMEM; + } + /* we mark the parents of a merge stale */ + flags |= STALE; + } + + for (i = 0; i < commit->out_degree; i++) { + commit_object *p = commit->parents[i]; + if ((p->flags & flags) == flags) + continue; + + if ((error = commit_parse(walk, p)) < GIT_SUCCESS) + return error; + + p->flags |= flags; + if (commit_list_insert_by_date(p, &list) == NULL) + return GIT_ENOMEM; + } + } + + commit_list_free(&list); + /* filter out any stale commits in the results */ + list = result; + result = NULL; + + while (list) { + struct commit_list *next = list->next; + if (!(list->item->flags & STALE)) + if (commit_list_insert_by_date(list->item, &result) == NULL) + return GIT_ENOMEM; + + free(list); + list = next; + } + + *out = result; + return GIT_SUCCESS; +} + +int git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *two) +{ + git_revwalk *walk; + git_vector list; + commit_list *result; + commit_object *commit; + void *contents[1]; + int error; + + error = git_revwalk_new(&walk, repo); + if (error < GIT_SUCCESS) + return error; + + commit = commit_lookup(walk, two); + if (commit == NULL) + goto cleanup; + + /* This is just one value, so we can do it on the stack */ + memset(&list, 0x0, sizeof(git_vector)); + contents[0] = commit; + list.length = 1; + list.contents = contents; + + commit = commit_lookup(walk, one); + if (commit == NULL) + goto cleanup; + + error = merge_bases_many(&result, walk, commit, &list); + if (error < GIT_SUCCESS) + return error; + + if (result == NULL) + return GIT_ENOTFOUND; + + git_oid_cpy(out, &result->item->oid); + commit_list_free(&result); + +cleanup: + git_revwalk_free(walk); + + return GIT_SUCCESS; +} + static void mark_uninteresting(commit_object *commit) { unsigned short i; diff --git a/tests-clar/revwalk/mergebase.c b/tests-clar/revwalk/mergebase.c new file mode 100644 index 000000000..91bc6ae8c --- /dev/null +++ b/tests-clar/revwalk/mergebase.c @@ -0,0 +1,37 @@ +#include "clar_libgit2.h" + +static git_repository *_repo; + +void test_revwalk_mergebase__initialize(void) +{ + cl_git_pass(git_repository_open(&_repo, cl_fixture("testrepo.git"))); +} + +void test_revwalk_mergebase__cleanup(void) +{ + git_repository_free(_repo); +} + +void test_revwalk_mergebase__single1(void) +{ + git_oid result, one, two, expected; + + git_oid_fromstr(&one, "c47800c7266a2be04c571c04d5a6614691ea99bd "); + git_oid_fromstr(&two, "9fd738e8f7967c078dceed8190330fc8648ee56a"); + git_oid_fromstr(&expected, "5b5b025afb0b4c913b4c338a42934a3863bf3644"); + + cl_git_pass(git_merge_base(&result, _repo, &one, &two)); + cl_assert(git_oid_cmp(&result, &expected) == 0); +} + +void test_revwalk_mergebase__single2(void) +{ + git_oid result, one, two, expected; + + git_oid_fromstr(&one, "763d71aadf09a7951596c9746c024e7eece7c7af"); + git_oid_fromstr(&two, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); + git_oid_fromstr(&expected, "c47800c7266a2be04c571c04d5a6614691ea99bd"); + + cl_git_pass(git_merge_base(&result, _repo, &one, &two)); + cl_assert(git_oid_cmp(&result, &expected) == 0); +} From 2c4ef1dd0da560e91b6440aa430537b98e8345dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 3 Mar 2012 16:43:24 +0100 Subject: [PATCH 4/8] revwalk: use merge bases to speed up processing There is no need walk down the parents of a merge base to mark them as uninteresting because we'll never see them. Calculate the merge bases in prepare_walk() so mark_uninteresting() can stop at a merge base instead of walking all the way to the root. --- src/revwalk.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 92885d688..727bcd7a1 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -59,6 +59,10 @@ struct git_revwalk { unsigned walking:1; unsigned int sorting; + + /* merge base calculation */ + commit_object *one; + git_vector twos; }; static int commit_time_cmp(void *a, void *b) @@ -341,6 +345,7 @@ static int merge_bases_many(commit_list **out, git_revwalk *walk, commit_object } commit_list_free(&list); + /* filter out any stale commits in the results */ list = result; result = NULL; @@ -409,6 +414,10 @@ static void mark_uninteresting(commit_object *commit) commit->uninteresting = 1; + /* This means we've reached a merge base, so there's no need to walk any more */ + if ((commit->flags & (RESULT | STALE)) == RESULT) + return; + for (i = 0; i < commit->out_degree; ++i) if (!commit->parents[i]->uninteresting) mark_uninteresting(commit->parents[i]); @@ -452,7 +461,15 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) if (commit == NULL) return git__throw(GIT_ENOTFOUND, "Failed to push commit. Object not found"); - return process_commit(walk, commit, uninteresting); + commit->uninteresting = uninteresting; + if (walk->one == NULL && !uninteresting) { + walk->one = commit; + } else { + if (git_vector_insert(&walk->twos, commit) < GIT_SUCCESS) + return GIT_ENOMEM; + } + + return GIT_SUCCESS; } int git_revwalk_push(git_revwalk *walk, const git_oid *oid) @@ -666,7 +683,25 @@ static int revwalk_next_reverse(commit_object **object_out, git_revwalk *walk) static int prepare_walk(git_revwalk *walk) { int error; - commit_object *next; + unsigned int i; + commit_object *next, *two; + commit_list *bases = NULL; + + /* first figure out what the merge bases are */ + error = merge_bases_many(&bases, walk, walk->one, &walk->twos); + if (error < GIT_SUCCESS) + return error; + + commit_list_free(&bases); + error = process_commit(walk, walk->one, walk->one->uninteresting); + if (error < GIT_SUCCESS) + return error; + + git_vector_foreach(&walk->twos, i, two) { + error = process_commit(walk, two, two->uninteresting); + if (error < GIT_SUCCESS) + return error; + } if (walk->sorting & GIT_SORT_TOPOLOGICAL) { unsigned short i; @@ -729,6 +764,7 @@ int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) git_pqueue_init(&walk->iterator_time, 8, commit_time_cmp); git_vector_init(&walk->memory_alloc, 8, NULL); + git_vector_init(&walk->twos, 4, NULL); alloc_chunk(walk); walk->get_next = &revwalk_next_unsorted; @@ -772,6 +808,7 @@ void git_revwalk_free(git_revwalk *walk) git__free(git_vector_get(&walk->memory_alloc, i)); git_vector_free(&walk->memory_alloc); + git_vector_free(&walk->twos); git__free(walk); } From 5cf7bccd2b72582186963f8758e883e0a978041a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 3 Mar 2012 17:25:05 +0100 Subject: [PATCH 5/8] revwalk: add test hiding a commit without a merge base Nothing should be hidden and this shouldn't bother the merge base calculation. --- tests-clar/revwalk/basic.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests-clar/revwalk/basic.c b/tests-clar/revwalk/basic.c index a364d82b0..7d54ce990 100644 --- a/tests-clar/revwalk/basic.c +++ b/tests-clar/revwalk/basic.c @@ -163,3 +163,19 @@ void test_revwalk_basic__push_head_hide_ref(void) /* git log HEAD --oneline --not refs/heads/packed-test | wc -l => 4 */ cl_assert(i == 4); } + +void test_revwalk_basic__push_head_hide_ref_nobase(void) +{ + int i = 0; + git_oid oid; + + cl_git_pass(git_revwalk_push_head(_walk)); + cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/packed")); + + while (git_revwalk_next(&oid, _walk) == GIT_SUCCESS) { + i++; + } + + /* git log HEAD --oneline --not refs/heads/packed | wc -l => 7 */ + cl_assert(i == 7); +} From f9e4bfa39ba4042c107ee93a22fb0e50ef982d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 4 Mar 2012 03:00:35 +0100 Subject: [PATCH 6/8] revwalk: use a priority queue for calculating merge bases As parents are older than their children, we're appending to the commit list most of the time, which makes an ordered linked list quite inefficient. While we're there, don't sort the results list in the main loop, as we're sorting them afterwards and it creates extra work. --- src/revwalk.c | 73 +++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 727bcd7a1..f5fdc4932 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -268,18 +268,16 @@ static int commit_parse(git_revwalk *walk, commit_object *commit) return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse commit"); } -static commit_object *interesting(commit_list *list) +static int interesting(git_pqueue *list) { - while (list) { - commit_object *commit = list->item; - list = list->next; - if (commit->flags & STALE) - continue; - - return commit; + unsigned int i; + for (i = 1; i < git_pqueue_size(list); i++) { + commit_object *commit = list->d[i]; + if ((commit->flags & STALE) == 0) + return 1; } - return NULL; + return 0; } static int merge_bases_many(commit_list **out, git_revwalk *walk, commit_object *one, git_vector *twos) @@ -287,44 +285,45 @@ static int merge_bases_many(commit_list **out, git_revwalk *walk, commit_object int error; unsigned int i; commit_object *two; - commit_list *list = NULL, *result = NULL; + commit_list *result = NULL, *tmp = NULL; + git_pqueue list; /* if the commit is repeated, we have a our merge base already */ git_vector_foreach(twos, i, two) { if (one == two) - return commit_list_insert(one, out) ? GIT_SUCCESS : GIT_ENOMEM; + return commit_list_insert(one, out) ? 0 : -1; } - if ((error = commit_parse(walk, one)) < GIT_SUCCESS) - return error; + if (git_pqueue_init(&list, twos->length * 2, commit_time_cmp) < 0) + return -1; + + if (commit_parse(walk, one) < 0) + return -1; one->flags |= PARENT1; - if (commit_list_insert(one, &list) == NULL) - return GIT_ENOMEM; + if (git_pqueue_insert(&list, one) < 0) + return -1; git_vector_foreach(twos, i, two) { commit_parse(walk, two); two->flags |= PARENT2; - if (commit_list_insert_by_date(two, &list) == NULL) - return GIT_ENOMEM; + if (git_pqueue_insert(&list, two) < 0) + return -1; } /* as long as there are non-STALE commits */ - while (interesting(list)) { - commit_object *commit = list->item; - commit_list *next; + while (interesting(&list)) { + commit_object *commit; int flags; - next = list->next; - git__free(list); - list = next; + commit = git_pqueue_pop(&list); flags = commit->flags & (PARENT1 | PARENT2 | STALE); if (flags == (PARENT1 | PARENT2)) { if (!(commit->flags & RESULT)) { commit->flags |= RESULT; - if (commit_list_insert_by_date(commit, &result) == NULL) - return GIT_ENOMEM; + if (commit_list_insert(commit, &result) == NULL) + return -1; } /* we mark the parents of a merge stale */ flags |= STALE; @@ -339,29 +338,29 @@ static int merge_bases_many(commit_list **out, git_revwalk *walk, commit_object return error; p->flags |= flags; - if (commit_list_insert_by_date(p, &list) == NULL) - return GIT_ENOMEM; + if (git_pqueue_insert(&list, p) < 0) + return -1; } } - commit_list_free(&list); + git_pqueue_free(&list); /* filter out any stale commits in the results */ - list = result; + tmp = result; result = NULL; - while (list) { - struct commit_list *next = list->next; - if (!(list->item->flags & STALE)) - if (commit_list_insert_by_date(list->item, &result) == NULL) - return GIT_ENOMEM; + while (tmp) { + struct commit_list *next = tmp->next; + if (!(tmp->item->flags & STALE)) + if (commit_list_insert_by_date(tmp->item, &result) == NULL) + return -1; - free(list); - list = next; + git__free(tmp); + tmp = next; } *out = result; - return GIT_SUCCESS; + return 0; } int git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *two) From bf787bd87c04e5213fc277c583970c52dea6781f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 8 Apr 2012 18:56:50 +0200 Subject: [PATCH 7/8] Move git_merge_base() to is own header and document it --- include/git2.h | 1 + include/git2/merge.h | 35 +++++++++++++++++++++++++++++++++++ include/git2/revwalk.h | 3 --- src/revwalk.c | 1 + 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 include/git2/merge.h diff --git a/include/git2.h b/include/git2.h index 7d053c4af..d75387318 100644 --- a/include/git2.h +++ b/include/git2.h @@ -22,6 +22,7 @@ #include "git2/repository.h" #include "git2/revwalk.h" +#include "git2/merge.h" #include "git2/refs.h" #include "git2/reflog.h" diff --git a/include/git2/merge.h b/include/git2/merge.h new file mode 100644 index 000000000..5a0b2e7f2 --- /dev/null +++ b/include/git2/merge.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2009-2012 the libgit2 contributors + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_git_merge_h__ +#define INCLUDE_git_merge_h__ + +#include "common.h" +#include "types.h" +#include "oid.h" + +/** + * @file git2/merge.h + * @brief Git merge-base routines + * @defgroup git_revwalk Git merge-base routines + * @ingroup Git + * @{ + */ +GIT_BEGIN_DECL + +/** + * Find a merge base between two commits + * + * @param out the OID of a merge base between 'one' and 'two' + * @param repo the repository where the commits exist + * @param one one of the commits + * @param two the other commit + */ +GIT_EXTERN(int) git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *two); + +/** @} */ +GIT_END_DECL +#endif diff --git a/include/git2/revwalk.h b/include/git2/revwalk.h index db27c62b1..632c67588 100644 --- a/include/git2/revwalk.h +++ b/include/git2/revwalk.h @@ -232,9 +232,6 @@ GIT_EXTERN(void) git_revwalk_free(git_revwalk *walk); */ GIT_EXTERN(git_repository *) git_revwalk_repository(git_revwalk *walk); -GIT_EXTERN(int) git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *two); - - /** @} */ GIT_END_DECL #endif diff --git a/src/revwalk.c b/src/revwalk.c index f5fdc4932..ac0414b1e 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -12,6 +12,7 @@ #include "pqueue.h" #include "git2/revwalk.h" +#include "git2/merge.h" #include From eb8117b841f22063ba8fe27653cac79957c548ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 12 Apr 2012 20:25:07 +0200 Subject: [PATCH 8/8] error-handling: revwalk --- src/revwalk.c | 204 +++++++++++++++++++++++++------------------------- 1 file changed, 104 insertions(+), 100 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index ac0414b1e..c2c098cf8 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -214,38 +214,43 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo } commit->parents = alloc_parents(commit, parents); - if (commit->parents == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(commit->parents); buffer = parents_start; for (i = 0; i < parents; ++i) { git_oid oid; - if (git_oid_fromstr(&oid, (char *)buffer + strlen("parent ")) < GIT_SUCCESS) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Parent object is corrupted"); + if (git_oid_fromstr(&oid, (char *)buffer + strlen("parent ")) < 0) + return -1; commit->parents[i] = commit_lookup(walk, &oid); if (commit->parents[i] == NULL) - return GIT_ENOMEM; + return -1; buffer += parent_len; } commit->out_degree = (unsigned short)parents; - if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Object is corrupted"); + if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) { + giterr_set(GITERR_ODB, "Failed to parse commit. Object is corrupted"); + return -1; + } buffer = memchr(buffer, '>', buffer_end - buffer); - if (buffer == NULL) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Can't find author"); + if (buffer == NULL) { + giterr_set(GITERR_ODB, "Failed to parse commit. Can't find author"); + return -1; + } - if (git__strtol32(&commit_time, (char *)buffer + 2, NULL, 10) < GIT_SUCCESS) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Can't parse commit time"); + if (git__strtol32(&commit_time, (char *)buffer + 2, NULL, 10) < 0) { + giterr_set(GITERR_ODB, "Failed to parse commit. Can't parse commit time"); + return -1; + } commit->time = (time_t)commit_time; commit->parsed = 1; - return GIT_SUCCESS; + return 0; } static int commit_parse(git_revwalk *walk, commit_object *commit) @@ -254,19 +259,20 @@ static int commit_parse(git_revwalk *walk, commit_object *commit) int error; if (commit->parsed) - return GIT_SUCCESS; + return 0; - if ((error = git_odb_read(&obj, walk->odb, &commit->oid)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to parse commit. Can't read object"); + if ((error = git_odb_read(&obj, walk->odb, &commit->oid)) < 0) + return -1; if (obj->raw.type != GIT_OBJ_COMMIT) { git_odb_object_free(obj); - return git__throw(GIT_EOBJTYPE, "Failed to parse commit. Object is no commit object"); + giterr_set(GITERR_INVALID, "Failed to parse commit. Object is no commit object"); + return -1; } error = commit_quick_parse(walk, commit, &obj->raw); git_odb_object_free(obj); - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse commit"); + return error; } static int interesting(git_pqueue *list) @@ -368,18 +374,16 @@ int git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *tw { git_revwalk *walk; git_vector list; - commit_list *result; + commit_list *result = NULL; commit_object *commit; void *contents[1]; - int error; - error = git_revwalk_new(&walk, repo); - if (error < GIT_SUCCESS) - return error; + if (git_revwalk_new(&walk, repo) < 0) + return -1; commit = commit_lookup(walk, two); if (commit == NULL) - goto cleanup; + goto on_error; /* This is just one value, so we can do it on the stack */ memset(&list, 0x0, sizeof(git_vector)); @@ -389,22 +393,25 @@ int git_merge_base(git_oid *out, git_repository *repo, git_oid *one, git_oid *tw commit = commit_lookup(walk, one); if (commit == NULL) - goto cleanup; + goto on_error; - error = merge_bases_many(&result, walk, commit, &list); - if (error < GIT_SUCCESS) - return error; + if (merge_bases_many(&result, walk, commit, &list) < 0) + goto on_error; - if (result == NULL) + if (!result) { + git_revwalk_free(walk); return GIT_ENOTFOUND; + } git_oid_cpy(out, &result->item->oid); commit_list_free(&result); - -cleanup: git_revwalk_free(walk); - return GIT_SUCCESS; + return 0; + +on_error: + git_revwalk_free(walk); + return -1; } static void mark_uninteresting(commit_object *commit) @@ -425,18 +432,16 @@ static void mark_uninteresting(commit_object *commit) static int process_commit(git_revwalk *walk, commit_object *commit, int hide) { - int error; - if (hide) mark_uninteresting(commit); if (commit->seen) - return GIT_SUCCESS; + return 0; commit->seen = 1; - if ((error = commit_parse(walk, commit)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to process commit"); + if (commit_parse(walk, commit) < 0) + return -1; return walk->enqueue(walk, commit); } @@ -444,13 +449,13 @@ static int process_commit(git_revwalk *walk, commit_object *commit, int hide) static int process_commit_parents(git_revwalk *walk, commit_object *commit) { unsigned short i; - int error = GIT_SUCCESS; - for (i = 0; i < commit->out_degree && error == GIT_SUCCESS; ++i) { - error = process_commit(walk, commit->parents[i], commit->uninteresting); + for (i = 0; i < commit->out_degree; ++i) { + if (process_commit(walk, commit->parents[i], commit->uninteresting) < 0) + return -1; } - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to process commit parents"); + return 0; } static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) @@ -459,17 +464,17 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) commit = commit_lookup(walk, oid); if (commit == NULL) - return git__throw(GIT_ENOTFOUND, "Failed to push commit. Object not found"); + return -1; commit->uninteresting = uninteresting; if (walk->one == NULL && !uninteresting) { walk->one = commit; } else { - if (git_vector_insert(&walk->twos, commit) < GIT_SUCCESS) - return GIT_ENOMEM; + if (git_vector_insert(&walk->twos, commit) < 0) + return -1; } - return GIT_SUCCESS; + return 0; } int git_revwalk_push(git_revwalk *walk, const git_oid *oid) @@ -490,15 +495,17 @@ static int push_ref(git_revwalk *walk, const char *refname, int hide) git_reference *ref, *resolved; int error; - error = git_reference_lookup(&ref, walk->repo, refname); - if (error < GIT_SUCCESS) - return error; + if (git_reference_lookup(&ref, walk->repo, refname) < 0) + return -1; + error = git_reference_resolve(&resolved, ref); git_reference_free(ref); - if (error < GIT_SUCCESS) - return error; + if (error < 0) + return -1; + error = push_commit(walk, git_reference_oid(resolved), hide); git_reference_free(resolved); + return error; } @@ -515,14 +522,13 @@ static int push_glob_cb(const char *refname, void *data_) if (!git__fnmatch(data->glob, refname, 0)) return push_ref(data->walk, refname, data->hide); - return GIT_SUCCESS; + return 0; } static int push_glob(git_revwalk *walk, const char *glob, int hide) { git_buf buf = GIT_BUF_INIT; struct push_cb_data data; - int error; regex_t preg; assert(walk && glob); @@ -537,28 +543,32 @@ static int push_glob(git_revwalk *walk, const char *glob, int hide) /* If no '?', '*' or '[' exist, we append '/ *' to the glob */ memset(&preg, 0x0, sizeof(regex_t)); if (regcomp(&preg, "[?*[]", REG_EXTENDED)) { - error = git__throw(GIT_EOSERR, "Regex failed to compile"); - goto cleanup; + giterr_set(GITERR_OS, "Regex failed to compile"); + git_buf_free(&buf); + return -1; } if (regexec(&preg, glob, 0, NULL, 0)) git_buf_puts(&buf, "/*"); - if (git_buf_oom(&buf)) { - error = GIT_ENOMEM; - goto cleanup; - } + if (git_buf_oom(&buf)) + goto on_error; data.walk = walk; data.glob = git_buf_cstr(&buf); data.hide = hide; - error = git_reference_foreach(walk->repo, GIT_REF_LISTALL, push_glob_cb, &data); + if (git_reference_foreach(walk->repo, GIT_REF_LISTALL, push_glob_cb, &data) < 0) + goto on_error; -cleanup: regfree(&preg); git_buf_free(&buf); - return error; + return 0; + +on_error: + regfree(&preg); + git_buf_free(&buf); + return -1; } int git_revwalk_push_glob(git_revwalk *walk, const char *glob) @@ -613,16 +623,16 @@ static int revwalk_next_timesort(commit_object **object_out, git_revwalk *walk) commit_object *next; while ((next = git_pqueue_pop(&walk->iterator_time)) != NULL) { - if ((error = process_commit_parents(walk, next)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to load next revision"); + if ((error = process_commit_parents(walk, next)) < 0) + return -1; if (!next->uninteresting) { *object_out = next; - return GIT_SUCCESS; + return 0; } } - return git__throw(GIT_EREVWALKOVER, "Failed to load next revision"); + return GIT_EREVWALKOVER; } static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) @@ -631,16 +641,16 @@ static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) commit_object *next; while ((next = commit_list_pop(&walk->iterator_rand)) != NULL) { - if ((error = process_commit_parents(walk, next)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to load next revision"); + if ((error = process_commit_parents(walk, next)) < 0) + return -1; if (!next->uninteresting) { *object_out = next; - return GIT_SUCCESS; + return 0; } } - return git__throw(GIT_EREVWALKOVER, "Failed to load next revision"); + return GIT_EREVWALKOVER; } static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) @@ -651,7 +661,7 @@ static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) for (;;) { next = commit_list_pop(&walk->iterator_topo); if (next == NULL) - return git__throw(GIT_EREVWALKOVER, "Failed to load next revision"); + return GIT_EREVWALKOVER; if (next->in_degree > 0) { next->topo_delay = 1; @@ -664,19 +674,19 @@ static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) if (--parent->in_degree == 0 && parent->topo_delay) { parent->topo_delay = 0; if (commit_list_insert(parent, &walk->iterator_topo) == NULL) - return GIT_ENOMEM; + return -1; } } *object_out = next; - return GIT_SUCCESS; + return 0; } } static int revwalk_next_reverse(commit_object **object_out, git_revwalk *walk) { *object_out = commit_list_pop(&walk->iterator_reverse); - return *object_out ? GIT_SUCCESS : GIT_EREVWALKOVER; + return *object_out ? 0 : GIT_EREVWALKOVER; } @@ -688,36 +698,33 @@ static int prepare_walk(git_revwalk *walk) commit_list *bases = NULL; /* first figure out what the merge bases are */ - error = merge_bases_many(&bases, walk, walk->one, &walk->twos); - if (error < GIT_SUCCESS) - return error; + if (merge_bases_many(&bases, walk, walk->one, &walk->twos) < 0) + return -1; commit_list_free(&bases); - error = process_commit(walk, walk->one, walk->one->uninteresting); - if (error < GIT_SUCCESS) - return error; + if (process_commit(walk, walk->one, walk->one->uninteresting) < 0) + return -1; git_vector_foreach(&walk->twos, i, two) { - error = process_commit(walk, two, two->uninteresting); - if (error < GIT_SUCCESS) - return error; + if (process_commit(walk, two, two->uninteresting) < 0) + return -1; } if (walk->sorting & GIT_SORT_TOPOLOGICAL) { unsigned short i; - while ((error = walk->get_next(&next, walk)) == GIT_SUCCESS) { + while ((error = walk->get_next(&next, walk)) == 0) { for (i = 0; i < next->out_degree; ++i) { commit_object *parent = next->parents[i]; parent->in_degree++; } if (commit_list_insert(next, &walk->iterator_topo) == NULL) - return GIT_ENOMEM; + return -1; } if (error != GIT_EREVWALKOVER) - return git__rethrow(error, "Failed to prepare revision walk"); + return -1; walk->get_next = &revwalk_next_toposort; } @@ -726,16 +733,16 @@ static int prepare_walk(git_revwalk *walk) while ((error = walk->get_next(&next, walk)) == GIT_SUCCESS) if (commit_list_insert(next, &walk->iterator_reverse) == NULL) - return GIT_ENOMEM; + return -1; if (error != GIT_EREVWALKOVER) - return git__rethrow(error, "Failed to prepare revision walk"); + return -1; walk->get_next = &revwalk_next_reverse; } walk->walking = 1; - return GIT_SUCCESS; + return 0; } @@ -744,12 +751,10 @@ static int prepare_walk(git_revwalk *walk) int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) { - int error; git_revwalk *walk; walk = git__malloc(sizeof(git_revwalk)); - if (walk == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(walk); memset(walk, 0x0, sizeof(git_revwalk)); @@ -759,7 +764,7 @@ int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) if (walk->commits == NULL) { git__free(walk); - return GIT_ENOMEM; + return -1; } git_pqueue_init(&walk->iterator_time, 8, commit_time_cmp); @@ -772,14 +777,13 @@ int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) walk->repo = repo; - error = git_repository_odb(&walk->odb, repo); - if (error < GIT_SUCCESS) { + if (git_repository_odb(&walk->odb, repo) < 0) { git_revwalk_free(walk); - return error; + return -1; } *revwalk_out = walk; - return GIT_SUCCESS; + return 0; } void git_revwalk_free(git_revwalk *walk) @@ -844,8 +848,8 @@ int git_revwalk_next(git_oid *oid, git_revwalk *walk) assert(walk && oid); if (!walk->walking) { - if ((error = prepare_walk(walk)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to load next revision"); + if ((error = prepare_walk(walk)) < 0) + return -1; } error = walk->get_next(&next, walk); @@ -855,11 +859,11 @@ int git_revwalk_next(git_oid *oid, git_revwalk *walk) return GIT_EREVWALKOVER; } - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to load next revision"); + if (error < 0) + return -1; git_oid_cpy(oid, &next->oid); - return GIT_SUCCESS; + return 0; } void git_revwalk_reset(git_revwalk *walk)