From d8057a5b0ed644b1f72a4eb80f82da7ce8977958 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 27 Aug 2012 11:53:59 -0700 Subject: [PATCH] Make git_object_peel a bit smarter This expands the types of peeling that `git_object_peel` knows how to do to include TAG -> BLOB peeling, and makes the errors slightly more consistent depending on the situation. It also adds a new special behavior where peeling to ANY will peel until the object type changes (e.g. chases TAGs to a non-TAG). Using this expanded peeling, this replaces peeling code that was embedded in `git_tag_peel` and `git_reset`. --- include/git2/object.h | 11 +++++---- include/git2/reset.h | 2 +- src/object.c | 51 ++++++++++++++++++++-------------------- src/reset.c | 30 ++++------------------- src/tag.c | 17 +------------- tests-clar/object/peel.c | 16 ++++++++++--- 6 files changed, 53 insertions(+), 74 deletions(-) diff --git a/include/git2/object.h b/include/git2/object.h index 722434dec..fd6ae95c1 100644 --- a/include/git2/object.h +++ b/include/git2/object.h @@ -168,11 +168,14 @@ GIT_EXTERN(int) git_object_typeisloose(git_otype type); GIT_EXTERN(size_t) git_object__size(git_otype type); /** - * Recursively peel an object until an object of the specified - * type is met + * Recursively peel an object until an object of the specified type is met. * - * The retrieved `peeled` object is owned by the repository - * and should be closed with the `git_object_free` method. + * The retrieved `peeled` object is owned by the repository and should be + * closed with the `git_object_free` method. + * + * If you pass `GIT_OBJ_ANY` as the target type, then the object will be + * peeled until the type changes (e.g. a tag will be chased until the + * referenced object is no longer a tag). * * @param peeled Pointer to the peeled git_object * @param object The object to be processed diff --git a/include/git2/reset.h b/include/git2/reset.h index 125178748..cd263fa99 100644 --- a/include/git2/reset.h +++ b/include/git2/reset.h @@ -37,7 +37,7 @@ GIT_BEGIN_DECL * * @return GIT_SUCCESS or an error code */ -GIT_EXTERN(int) git_reset(git_repository *repo, const git_object *target, git_reset_type reset_type); +GIT_EXTERN(int) git_reset(git_repository *repo, git_object *target, git_reset_type reset_type); /** @} */ GIT_END_DECL diff --git a/src/object.c b/src/object.c index 227774047..5130d97ac 100644 --- a/src/object.c +++ b/src/object.c @@ -334,6 +334,12 @@ int git_object__resolve_to_type(git_object **obj, git_otype type) return error; } +static int peel_error(int error, const char* msg) +{ + giterr_set(GITERR_INVALID, "The given object cannot be peeled - %s", msg); + return error; +} + static int dereference_object(git_object **dereferenced, git_object *obj) { git_otype type = git_object_type(obj); @@ -341,48 +347,36 @@ static int dereference_object(git_object **dereferenced, git_object *obj) switch (type) { case GIT_OBJ_COMMIT: return git_commit_tree((git_tree **)dereferenced, (git_commit*)obj); - break; case GIT_OBJ_TAG: return git_tag_target(dereferenced, (git_tag*)obj); - break; + + case GIT_OBJ_BLOB: + return peel_error(GIT_ERROR, "cannot dereference blob"); + + case GIT_OBJ_TREE: + return peel_error(GIT_ERROR, "cannot dereference tree"); default: - return GIT_ENOTFOUND; - break; + return peel_error(GIT_ENOTFOUND, "unexpected object type encountered"); } } -static int peel_error(int error, const char* msg) -{ - giterr_set(GITERR_INVALID, "The given object cannot be peeled - %s", msg); - return error; -} - int git_object_peel( - git_object **peeled, - git_object *object, - git_otype target_type) + git_object **peeled, + git_object *object, + git_otype target_type) { git_object *source, *deref = NULL; - assert(object); + assert(object && peeled); if (git_object_type(object) == target_type) return git_object__dup(peeled, object); - if (target_type == GIT_OBJ_BLOB - || target_type == GIT_OBJ_ANY) - return peel_error(GIT_EAMBIGUOUS, "Ambiguous target type"); - - if (git_object_type(object) == GIT_OBJ_BLOB) - return peel_error(GIT_ERROR, "A blob cannot be dereferenced"); - source = object; - while (true) { - if (dereference_object(&deref, source) < 0) - goto cleanup; + while (!dereference_object(&deref, source)) { if (source != object) git_object_free(source); @@ -392,13 +386,20 @@ int git_object_peel( return 0; } + if (target_type == GIT_OBJ_ANY && + git_object_type(deref) != git_object_type(object)) + { + *peeled = deref; + return 0; + } + source = deref; deref = NULL; } -cleanup: if (source != object) git_object_free(source); + git_object_free(deref); return -1; } diff --git a/src/reset.c b/src/reset.c index 1379f6442..f9e16f7c6 100644 --- a/src/reset.c +++ b/src/reset.c @@ -20,10 +20,9 @@ static int reset_error_invalid(const char *msg) int git_reset( git_repository *repo, - const git_object *target, + git_object *target, git_reset_type reset_type) { - git_otype target_type = GIT_OBJ_BAD; git_object *commit = NULL; git_index *index = NULL; git_tree *tree = NULL; @@ -38,26 +37,9 @@ int git_reset( if (reset_type == GIT_RESET_MIXED && git_repository_is_bare(repo)) return reset_error_invalid("Mixed reset is not allowed in a bare repository."); - target_type = git_object_type(target); - - switch (target_type) - { - case GIT_OBJ_TAG: - if (git_tag_peel(&commit, (git_tag *)target) < 0) - goto cleanup; - - if (git_object_type(commit) != GIT_OBJ_COMMIT) { - reset_error_invalid("The given target does not resolve to a commit."); - goto cleanup; - } - break; - - case GIT_OBJ_COMMIT: - commit = (git_object *)target; - break; - - default: - return reset_error_invalid("Only git_tag and git_commit objects are valid targets."); + if (git_object_peel(&commit, target, GIT_OBJ_COMMIT) < 0) { + reset_error_invalid("The given target does not resolve to a commit"); + goto cleanup; } //TODO: Check for unmerged entries @@ -93,9 +75,7 @@ int git_reset( error = 0; cleanup: - if (target_type == GIT_OBJ_TAG) - git_object_free(commit); - + git_object_free(commit); git_index_free(index); git_tree_free(tree); diff --git a/src/tag.c b/src/tag.c index 463619f63..6495d470f 100644 --- a/src/tag.c +++ b/src/tag.c @@ -445,20 +445,5 @@ int git_tag_list(git_strarray *tag_names, git_repository *repo) int git_tag_peel(git_object **tag_target, git_tag *tag) { - int error; - git_object *target; - - assert(tag_target && tag); - - if (git_tag_target(&target, tag) < 0) - return -1; - - if (git_object_type(target) == GIT_OBJ_TAG) { - error = git_tag_peel(tag_target, (git_tag *)target); - git_object_free(target); - return error; - } - - *tag_target = target; - return 0; + return git_object_peel(tag_target, (git_object *)tag, GIT_OBJ_ANY); } diff --git a/tests-clar/object/peel.c b/tests-clar/object/peel.c index f6d2a776f..f4ea1eb0f 100644 --- a/tests-clar/object/peel.c +++ b/tests-clar/object/peel.c @@ -65,7 +65,7 @@ void test_object_peel__can_peel_a_commit(void) void test_object_peel__cannot_peel_a_tree(void) { - assert_peel_error(GIT_EAMBIGUOUS, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB); + assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB); } void test_object_peel__cannot_peel_a_blob(void) @@ -73,7 +73,17 @@ void test_object_peel__cannot_peel_a_blob(void) assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_COMMIT); } -void test_object_peel__cannot_target_any_object(void) +void test_object_peel__target_any_object_for_type_change(void) { - assert_peel_error(GIT_EAMBIGUOUS, "e90810b8df3e80c413d903f631643c716887138d", GIT_OBJ_ANY); + /* tag to commit */ + assert_peel("e90810b8df3e80c413d903f631643c716887138d", "7b4384978d2493e851f9cca7858815fac9b10980", GIT_OBJ_ANY); + + /* commit to tree */ + assert_peel("53fc32d17276939fc79ed05badaef2db09990016", "e90810b8df3e80c413d903f631643c716887138d", GIT_OBJ_ANY); + + /* fail to peel tree */ + assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_ANY); + + /* fail to peel blob */ + assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY); }