From f61272e04727716ad09bb57e4ffa9ce4be09dc55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Feb 2014 12:51:36 +0100 Subject: [PATCH 1/6] revwalk: accept committish objects Let the user push committish objects and peel them to figure out which commit to push to our queue. This is for convenience and for allowing uses of git_revwalk_push_glob(w, "tags") with annotated tags. --- include/git2/revwalk.h | 8 ++++---- src/revwalk.c | 21 +++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/include/git2/revwalk.h b/include/git2/revwalk.h index c59b79938..4eac8d43c 100644 --- a/include/git2/revwalk.h +++ b/include/git2/revwalk.h @@ -87,7 +87,7 @@ GIT_EXTERN(void) git_revwalk_reset(git_revwalk *walker); /** * Mark a commit to start traversal from. * - * The given OID must belong to a commit on the walked + * The given OID must belong to a committish on the walked * repository. * * The given commit will be used as one of the roots @@ -127,7 +127,7 @@ GIT_EXTERN(int) git_revwalk_push_head(git_revwalk *walk); /** * Mark a commit (and its ancestors) uninteresting for the output. * - * The given OID must belong to a commit on the walked + * The given OID must belong to a committish on the walked * repository. * * The resolved commit and all its parents will be hidden from the @@ -166,7 +166,7 @@ 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. + * The reference must point to a committish. * * @param walk the walker being used for the traversal * @param refname the reference to push @@ -177,7 +177,7 @@ 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. + * The reference must point to a committish. * * @param walk the walker being used for the traversal * @param refname the reference to hide diff --git a/src/revwalk.c b/src/revwalk.c index c0a053211..3cc3140b8 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -112,23 +112,28 @@ static int process_commit_parents(git_revwalk *walk, git_commit_list_node *commi static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) { + git_oid commit_id; int error; - git_object *obj; - git_otype type; + git_object *obj, *oobj; git_commit_list_node *commit; - if ((error = git_object_lookup(&obj, walk->repo, oid, GIT_OBJ_ANY)) < 0) + if ((error = git_object_lookup(&oobj, walk->repo, oid, GIT_OBJ_ANY)) < 0) return error; - type = git_object_type(obj); - git_object_free(obj); + error = git_object_peel(&obj, oobj, GIT_OBJ_COMMIT); + git_object_free(oobj); - if (type != GIT_OBJ_COMMIT) { - giterr_set(GITERR_INVALID, "Object is no commit object"); + if (error == GIT_ENOTFOUND) { + giterr_set(GITERR_INVALID, "Object is not a committish"); return -1; } + if (error < 0) + return error; - commit = git_revwalk__commit_lookup(walk, oid); + git_oid_cpy(&commit_id, git_object_id(obj)); + git_object_free(obj); + + commit = git_revwalk__commit_lookup(walk, &commit_id); if (commit == NULL) return -1; /* error already reported by failed lookup */ From b4ef67d5ebc55943073d7baacd91f46c20d9ccf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Feb 2014 15:11:00 +0100 Subject: [PATCH 2/6] revwalk: add a failing test for pushing "tags" This shows that pusing a whole namespace can be problematic. --- tests/revwalk/basic.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index 6d55aed54..4621ab8ff 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -252,3 +252,13 @@ void test_revwalk_basic__push_range(void) cl_git_pass(git_revwalk_push_range(_walk, "9fd738e~2..9fd738e")); cl_git_pass(test_walk_only(_walk, commit_sorting_segment, 1)); } + +void test_revwalk_basic__push_mixed(void) +{ + revwalk_basic_setup_walk(NULL); + + git_revwalk_reset(_walk); + git_revwalk_sorting(_walk, 0); + cl_git_pass(git_revwalk_push_glob(_walk, "tags")); + cl_git_pass(test_walk_only(_walk, commit_sorting_segment, 1)); +} From d465e4e980333b3e7437801fc375b595fb3adf1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Feb 2014 15:19:13 +0100 Subject: [PATCH 3/6] revwalk: ignore wrong object type in glob pushes Pushing a whole namespace can cause us to attempt to push non-committish objects. Catch this situation and special-case it for ignoring this. --- include/git2/revwalk.h | 6 ++++++ src/revwalk.c | 28 ++++++++++++++++------------ tests/revwalk/basic.c | 11 ++++++++++- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/include/git2/revwalk.h b/include/git2/revwalk.h index 4eac8d43c..12829b14c 100644 --- a/include/git2/revwalk.h +++ b/include/git2/revwalk.h @@ -110,6 +110,9 @@ GIT_EXTERN(int) git_revwalk_push(git_revwalk *walk, const git_oid *id); * A leading 'refs/' is implied if not present as well as a trailing * '/ *' if the glob lacks '?', '*' or '['. * + * Any references matching this glob which do not point to a + * committish will be ignored. + * * @param walk the walker being used for the traversal * @param glob the glob pattern references should match * @return 0 or an error code @@ -149,6 +152,9 @@ GIT_EXTERN(int) git_revwalk_hide(git_revwalk *walk, const git_oid *commit_id); * A leading 'refs/' is implied if not present as well as a trailing * '/ *' if the glob lacks '?', '*' or '['. * + * Any references matching this glob which do not point to a + * committish will be ignored. + * * @param walk the walker being used for the traversal * @param glob the glob pattern references should match * @return 0 or an error code diff --git a/src/revwalk.c b/src/revwalk.c index 3cc3140b8..63d78fe64 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -110,7 +110,7 @@ static int process_commit_parents(git_revwalk *walk, git_commit_list_node *commi return error; } -static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) +static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting, int from_glob) { git_oid commit_id; int error; @@ -124,6 +124,10 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) git_object_free(oobj); if (error == GIT_ENOTFOUND) { + /* If this comes from e.g. push_glob("tags"), ignore this */ + if (from_glob) + return 0; + giterr_set(GITERR_INVALID, "Object is not a committish"); return -1; } @@ -151,24 +155,24 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) int git_revwalk_push(git_revwalk *walk, const git_oid *oid) { assert(walk && oid); - return push_commit(walk, oid, 0); + return push_commit(walk, oid, 0, false); } int git_revwalk_hide(git_revwalk *walk, const git_oid *oid) { assert(walk && oid); - return push_commit(walk, oid, 1); + return push_commit(walk, oid, 1, false); } -static int push_ref(git_revwalk *walk, const char *refname, int hide) +static int push_ref(git_revwalk *walk, const char *refname, int hide, int from_glob) { git_oid oid; if (git_reference_name_to_id(&oid, walk->repo, refname) < 0) return -1; - return push_commit(walk, &oid, hide); + return push_commit(walk, &oid, hide, from_glob); } struct push_cb_data { @@ -179,7 +183,7 @@ struct push_cb_data { static int push_glob_cb(const char *refname, void *data_) { struct push_cb_data *data = (struct push_cb_data *)data_; - return push_ref(data->walk, refname, data->hide); + return push_ref(data->walk, refname, data->hide, true); } static int push_glob(git_revwalk *walk, const char *glob, int hide) @@ -229,19 +233,19 @@ int git_revwalk_hide_glob(git_revwalk *walk, const char *glob) int git_revwalk_push_head(git_revwalk *walk) { assert(walk); - return push_ref(walk, GIT_HEAD_FILE, 0); + return push_ref(walk, GIT_HEAD_FILE, 0, false); } int git_revwalk_hide_head(git_revwalk *walk) { assert(walk); - return push_ref(walk, GIT_HEAD_FILE, 1); + return push_ref(walk, GIT_HEAD_FILE, 1, false); } int git_revwalk_push_ref(git_revwalk *walk, const char *refname) { assert(walk && refname); - return push_ref(walk, refname, 0); + return push_ref(walk, refname, 0, false); } int git_revwalk_push_range(git_revwalk *walk, const char *range) @@ -258,10 +262,10 @@ int git_revwalk_push_range(git_revwalk *walk, const char *range) return GIT_EINVALIDSPEC; } - if ((error = push_commit(walk, git_object_id(revspec.from), 1))) + if ((error = push_commit(walk, git_object_id(revspec.from), 1, false))) goto out; - error = push_commit(walk, git_object_id(revspec.to), 0); + error = push_commit(walk, git_object_id(revspec.to), 0, false); out: git_object_free(revspec.from); @@ -272,7 +276,7 @@ out: int git_revwalk_hide_ref(git_revwalk *walk, const char *refname) { assert(walk && refname); - return push_ref(walk, refname, 1); + return push_ref(walk, refname, 1, false); } static int revwalk_enqueue_timesort(git_revwalk *walk, git_commit_list_node *commit) diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index 4621ab8ff..4d6522c1c 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -255,10 +255,19 @@ void test_revwalk_basic__push_range(void) void test_revwalk_basic__push_mixed(void) { + git_oid oid; + int i = 0; + revwalk_basic_setup_walk(NULL); git_revwalk_reset(_walk); git_revwalk_sorting(_walk, 0); cl_git_pass(git_revwalk_push_glob(_walk, "tags")); - cl_git_pass(test_walk_only(_walk, commit_sorting_segment, 1)); + + while (git_revwalk_next(&oid, _walk) == 0) { + i++; + } + + /* git rev-list --count --glob=tags #=> 9 */ + cl_assert_equal_i(9, i); } From af81720236f05f72cfe763208143b5dacb2f99a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Feb 2014 15:29:16 +0100 Subject: [PATCH 4/6] revwalk: remove usage of foreach --- src/revwalk.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 63d78fe64..628057fcd 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -180,17 +180,12 @@ struct push_cb_data { int hide; }; -static int push_glob_cb(const char *refname, void *data_) -{ - struct push_cb_data *data = (struct push_cb_data *)data_; - return push_ref(data->walk, refname, data->hide, true); -} - static int push_glob(git_revwalk *walk, const char *glob, int hide) { int error = 0; git_buf buf = GIT_BUF_INIT; - struct push_cb_data data; + git_reference *ref; + git_reference_iterator *iter; size_t wildcard; assert(walk && glob); @@ -208,12 +203,20 @@ static int push_glob(git_revwalk *walk, const char *glob, int hide) if (!glob[wildcard]) git_buf_put(&buf, "/*", 2); - data.walk = walk; - data.hide = hide; + if ((error = git_reference_iterator_glob_new(&iter, walk->repo, buf.ptr)) < 0) + goto out; - error = git_reference_foreach_glob( - walk->repo, git_buf_cstr(&buf), push_glob_cb, &data); + while ((error = git_reference_next(&ref, iter)) == 0) { + error = push_ref(walk, git_reference_name(ref), hide, true); + git_reference_free(ref); + if (error < 0) + break; + } + git_reference_iterator_free(iter); + if (error == GIT_ITEROVER) + error = 0; +out: git_buf_free(&buf); return error; } From d18209eef9a7fbbccb495b6e5450d14a76623847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Feb 2014 16:27:42 +0100 Subject: [PATCH 5/6] revwalk: add a test for pushing all references This used to be broken, let's make sure we don't break this use-case. --- tests/revwalk/basic.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index 4d6522c1c..9fe8a350b 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -271,3 +271,22 @@ void test_revwalk_basic__push_mixed(void) /* git rev-list --count --glob=tags #=> 9 */ cl_assert_equal_i(9, i); } + +void test_revwalk_basic__push_all(void) +{ + git_oid oid; + int i = 0; + + revwalk_basic_setup_walk(NULL); + + git_revwalk_reset(_walk); + git_revwalk_sorting(_walk, 0); + cl_git_pass(git_revwalk_push_glob(_walk, "*")); + + while (git_revwalk_next(&oid, _walk) == 0) { + i++; + } + + /* git rev-list --count --all #=> 15 */ + cl_assert_equal_i(15, i); +} From c74077d13c582ff46668f3ace4c54f20287cfed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 2 Feb 2014 12:08:18 +0100 Subject: [PATCH 6/6] revparse: do look at all refs when matching text Now that we no longer fail to push non-commits on a glob, let's search on all refs when we rev-parse syntax asks us to match text. --- src/revparse.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/revparse.c b/src/revparse.c index 5cce3be63..60872e187 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -490,8 +490,7 @@ static int handle_grep_syntax(git_object **out, git_repository *repo, const git_ git_revwalk_sorting(walk, GIT_SORT_TIME); if (spec_oid == NULL) { - // TODO: @carlosmn: The glob should be refs/* but this makes git_revwalk_next() fails - if ((error = git_revwalk_push_glob(walk, GIT_REFS_HEADS_DIR "*")) < 0) + if ((error = git_revwalk_push_glob(walk, "refs/*")) < 0) goto cleanup; } else if ((error = git_revwalk_push(walk, spec_oid)) < 0) goto cleanup;