From f201d613a80f7ad6f54d90eb7a7a0d8b8c72676b Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 13 Apr 2012 10:33:14 -0700 Subject: [PATCH] Add git_reference_lookup_oid and lookup_resolved Adds a new public reference function `git_reference_lookup_oid` that directly resolved a reference name to an OID without returning the intermediate `git_reference` object (hence, no free needed). Internally, this adds a `git_reference_lookup_resolved` function that combines looking up and resolving a reference. This allows us to be more efficient with memory reallocation. The existing `git_reference_lookup` and `git_reference_resolve` are reimplmented on top of the new utility and a few places in the code are changed to use one of the two new functions. --- include/git2/refs.h | 19 ++++++ src/refs.c | 141 ++++++++++++++++++++++++--------------- src/refs.h | 23 +++++++ src/repository.c | 19 +----- src/revwalk.c | 15 +---- src/status.c | 29 +++----- src/transports/local.c | 27 +++----- tests-clar/refs/lookup.c | 42 ++++++++++++ 8 files changed, 193 insertions(+), 122 deletions(-) create mode 100644 tests-clar/refs/lookup.c diff --git a/include/git2/refs.h b/include/git2/refs.h index 5395ded4b..6f2ac3ce9 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -32,6 +32,16 @@ GIT_BEGIN_DECL */ GIT_EXTERN(int) git_reference_lookup(git_reference **reference_out, git_repository *repo, const char *name); +/** + * Lookup a reference by name and resolve immediately to OID. + * + * @param oid Pointer to oid to be filled in + * @param repo The repository in which to look up the reference + * @param name The long name for the reference + * @return 0 on success, -1 if name could not be resolved + */ +GIT_EXTERN(int) git_reference_lookup_oid(git_oid *out, git_repository *repo, const char *name); + /** * Create a new symbolic reference. * @@ -304,6 +314,15 @@ GIT_EXTERN(int) git_reference_reload(git_reference *ref); */ GIT_EXTERN(void) git_reference_free(git_reference *ref); +/** + * Compare two references. + * + * @param ref1 The first git_reference + * @param ref2 The second git_reference + * @return GIT_SUCCESS if the same, else a stable but meaningless ordering. + */ +GIT_EXTERN(int) git_reference_cmp(git_reference *ref1, git_reference *ref2); + /** @} */ GIT_END_DECL #endif diff --git a/src/refs.c b/src/refs.c index fb23a0ef8..90cb920ee 100644 --- a/src/refs.c +++ b/src/refs.c @@ -15,7 +15,8 @@ #include #include -#define MAX_NESTING_LEVEL 5 +#define DEFAULT_NESTING_LEVEL 5 +#define MAX_NESTING_LEVEL 10 enum { GIT_PACKREF_HAS_PEEL = 1, @@ -1057,24 +1058,80 @@ int git_reference_delete(git_reference *ref) int git_reference_lookup(git_reference **ref_out, git_repository *repo, const char *name) { - char normalized_name[GIT_REFNAME_MAX]; - git_reference *ref = NULL; - int result; + return git_reference_lookup_resolved(ref_out, repo, name, 0); +} + +int git_reference_lookup_oid( + git_oid *out, git_repository *repo, const char *name) +{ + int error; + git_reference *ref; + + if ((error = git_reference_lookup_resolved(&ref, repo, name, -1)) < 0) + return error; + + git_oid_cpy(out, git_reference_oid(ref)); + git_reference_free(ref); + return 0; +} + +int git_reference_lookup_resolved( + git_reference **ref_out, + git_repository *repo, + const char *name, + int max_nesting) +{ + git_reference *scan; + int result, nesting; assert(ref_out && repo && name); + *ref_out = NULL; - if (normalize_name(normalized_name, sizeof(normalized_name), name, 0) < 0) + if (max_nesting > MAX_NESTING_LEVEL) + max_nesting = MAX_NESTING_LEVEL; + else if (max_nesting < 0) + max_nesting = DEFAULT_NESTING_LEVEL; + + scan = git__calloc(1, sizeof(git_reference)); + GITERR_CHECK_ALLOC(scan); + + scan->name = git__calloc(GIT_REFNAME_MAX + 1, sizeof(char)); + GITERR_CHECK_ALLOC(scan->name); + + if ((result = normalize_name(scan->name, GIT_REFNAME_MAX, name, 0)) < 0) { + git_reference_free(scan); + return result; + } + + scan->target.symbolic = git__strdup(scan->name); + GITERR_CHECK_ALLOC(scan->target.symbolic); + + scan->owner = repo; + scan->flags = GIT_REF_SYMBOLIC; + + for (nesting = max_nesting; + nesting >= 0 && (scan->flags & GIT_REF_SYMBOLIC) != 0; + nesting--) + { + if (nesting != max_nesting) + strncpy(scan->name, scan->target.symbolic, GIT_REFNAME_MAX); + + scan->mtime = 0; + + if ((result = reference_lookup(scan)) < 0) + return result; /* lookup git_reference_free on scan already */ + } + + if ((scan->flags & GIT_REF_OID) == 0 && max_nesting != 0) { + giterr_set(GITERR_REFERENCE, + "Cannot resolve reference (>%u levels deep)", max_nesting); + git_reference_free(scan); return -1; + } - if (reference_alloc(&ref, repo, normalized_name) < 0) - return -1; - - result = reference_lookup(ref); - if (result == 0) - *ref_out = ref; - - return result; + *ref_out = scan; + return 0; } /** @@ -1381,47 +1438,10 @@ rollback: int git_reference_resolve(git_reference **ref_out, git_reference *ref) { - int result, i = 0; - git_repository *repo; - - assert(ref); - - *ref_out = NULL; - repo = ref->owner; - - /* If the reference is already resolved, we need to return a - * copy. Instead of duplicating `ref`, we look it up again to - * ensure the copy is out to date */ if (ref->flags & GIT_REF_OID) return git_reference_lookup(ref_out, ref->owner, ref->name); - - /* Otherwise, keep iterating until the reference is resolved */ - for (i = 0; i < MAX_NESTING_LEVEL; ++i) { - git_reference *new_ref; - - result = git_reference_lookup(&new_ref, repo, ref->target.symbolic); - if (result < 0) - return result; - - /* Free intermediate references, except for the original one - * we've received */ - if (i > 0) - git_reference_free(ref); - - ref = new_ref; - - /* When the reference we've just looked up is an OID, we've - * successfully resolved the symbolic ref */ - if (ref->flags & GIT_REF_OID) { - *ref_out = ref; - return 0; - } - } - - giterr_set(GITERR_REFERENCE, - "Symbolic reference too nested (%d levels deep)", MAX_NESTING_LEVEL); - - return -1; + else + return git_reference_lookup_resolved(ref_out, ref->owner, ref->target.symbolic, -1); } int git_reference_packall(git_repository *repo) @@ -1649,3 +1669,20 @@ int git_reference__normalize_name_oid( { return normalize_name(buffer_out, out_size, name, 1); } + +#define GIT_REF_TYPEMASK (GIT_REF_OID | GIT_REF_SYMBOLIC) + +int git_reference_cmp(git_reference *ref1, git_reference *ref2) +{ + assert(ref1 && ref2); + + /* let's put symbolic refs before OIDs */ + if ((ref1->flags & GIT_REF_TYPEMASK) != (ref2->flags & GIT_REF_TYPEMASK)) + return (ref1->flags & GIT_REF_SYMBOLIC) ? -1 : 1; + + if (ref1->flags & GIT_REF_SYMBOLIC) + return strcmp(ref1->target.symbolic, ref2->target.symbolic); + + return git_oid_cmp(&ref1->target.oid, &ref2->target.oid); +} + diff --git a/src/refs.h b/src/refs.h index 13b9abf15..e4a225ca3 100644 --- a/src/refs.h +++ b/src/refs.h @@ -55,4 +55,27 @@ void git_repository__refcache_free(git_refcache *refs); int git_reference__normalize_name(char *buffer_out, size_t out_size, const char *name); int git_reference__normalize_name_oid(char *buffer_out, size_t out_size, const char *name); +/** + * Lookup a reference by name and try to resolve to an OID. + * + * You can control how many dereferences this will attempt to resolve the + * reference with the `max_deref` parameter, or pass -1 to use a sane + * default. If you pass 0 for `max_deref`, this will not attempt to resolve + * the reference. For any value of `max_deref` other than 0, not + * successfully resolving the reference will be reported as an error. + + * The generated reference must be freed by the user. + * + * @param reference_out Pointer to the looked-up reference + * @param repo The repository to look up the reference + * @param name The long name for the reference (e.g. HEAD, ref/heads/master, refs/tags/v0.1.0, ...) + * @param max_deref Maximum number of dereferences to make of symbolic refs, 0 means simple lookup, < 0 means use default reasonable value + * @return 0 on success or < 0 on error; not being able to resolve the reference is an error unless 0 was passed for max_deref + */ +int git_reference_lookup_resolved( + git_reference **reference_out, + git_repository *repo, + const char *name, + int max_deref); + #endif diff --git a/src/repository.c b/src/repository.c index 18881ecce..572b51622 100644 --- a/src/repository.c +++ b/src/repository.c @@ -772,24 +772,7 @@ int git_repository_head_detached(git_repository *repo) int git_repository_head(git_reference **head_out, git_repository *repo) { - git_reference *ref, *resolved_ref; - int error; - - *head_out = NULL; - - error = git_reference_lookup(&ref, repo, GIT_HEAD_FILE); - if (error < 0) - return error; - - error = git_reference_resolve(&resolved_ref, ref); - if (error < 0) { - git_reference_free(ref); - return error; - } - - git_reference_free(ref); - *head_out = resolved_ref; - return 0; + return git_reference_lookup_resolved(head_out, repo, GIT_HEAD_FILE, -1); } int git_repository_head_orphan(git_repository *repo) diff --git a/src/revwalk.c b/src/revwalk.c index c2c098cf8..2d815b96a 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -492,21 +492,12 @@ int git_revwalk_hide(git_revwalk *walk, const git_oid *oid) static int push_ref(git_revwalk *walk, const char *refname, int hide) { - git_reference *ref, *resolved; - int error; + git_oid oid; - if (git_reference_lookup(&ref, walk->repo, refname) < 0) + if (git_reference_lookup_oid(&oid, walk->repo, refname) < 0) return -1; - error = git_reference_resolve(&resolved, ref); - git_reference_free(ref); - if (error < 0) - return -1; - - error = push_commit(walk, git_reference_oid(resolved), hide); - git_reference_free(resolved); - - return error; + return push_commit(walk, &oid, hide); } struct push_cb_data { diff --git a/src/status.c b/src/status.c index 8d2c7de14..d4f59e355 100644 --- a/src/status.c +++ b/src/status.c @@ -20,31 +20,19 @@ static int resolve_head_to_tree(git_tree **tree, git_repository *repo) { - git_reference *head = NULL; + git_oid head_oid; git_object *obj = NULL; - if (git_reference_lookup(&head, repo, GIT_HEAD_FILE) < 0) - return -1; - - if (git_reference_oid(head) == NULL) { - git_reference *resolved; - - if (git_reference_resolve(&resolved, head) < 0) { - /* cannot resolve HEAD - probably brand new repo */ - giterr_clear(); - git_reference_free(head); - return GIT_ENOTFOUND; - } - - git_reference_free(head); - head = resolved; + if (git_reference_lookup_oid(&head_oid, repo, GIT_HEAD_FILE) < 0) { + /* cannot resolve HEAD - probably brand new repo */ + giterr_clear(); + *tree = NULL; + return 0; } - if (git_object_lookup(&obj, repo, git_reference_oid(head), GIT_OBJ_ANY) < 0) + if (git_object_lookup(&obj, repo, &head_oid, GIT_OBJ_ANY) < 0) goto fail; - git_reference_free(head); - switch (git_object_type(obj)) { case GIT_OBJ_TREE: *tree = (git_tree *)obj; @@ -62,7 +50,6 @@ static int resolve_head_to_tree(git_tree **tree, git_repository *repo) fail: git_object_free(obj); - git_reference_free(head); return -1; } @@ -152,7 +139,7 @@ int git_status_foreach_ext( diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_UNTRACKED_DIRS; /* TODO: support EXCLUDE_SUBMODULES flag */ - if (show != GIT_STATUS_SHOW_WORKDIR_ONLY && + if (show != GIT_STATUS_SHOW_WORKDIR_ONLY && head != NULL && (err = git_diff_index_to_tree(repo, &diffopt, head, &idx2head)) < 0) goto cleanup; diff --git a/src/transports/local.c b/src/transports/local.c index 01c72cb41..ba1cee4f1 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -26,31 +26,22 @@ static int add_ref(transport_local *t, const char *name) { const char peeled[] = "^{}"; git_remote_head *head; - git_reference *ref = NULL, *resolved_ref = NULL; git_object *obj = NULL, *target = NULL; git_buf buf = GIT_BUF_INIT; - if (git_reference_lookup(&ref, t->repo, name) < 0) - return -1; - - if (git_reference_resolve(&resolved_ref, ref) < 0) { - git_reference_free(ref); - return -1; - } - head = git__malloc(sizeof(git_remote_head)); GITERR_CHECK_ALLOC(head); head->name = git__strdup(name); GITERR_CHECK_ALLOC(head->name); - git_oid_cpy(&head->oid, git_reference_oid(resolved_ref)); - - if (git_vector_insert(&t->refs, head) < 0) + if (git_reference_lookup_oid(&head->oid, t->repo, name) < 0 || + git_vector_insert(&t->refs, head) < 0) + { + git__free(head->name); + git__free(head); return -1; - - git_reference_free(ref); - git_reference_free(resolved_ref); + } /* If it's not a tag, we don't need to try to peel it */ if (git__prefixcmp(name, GIT_REFS_TAGS_DIR)) @@ -100,10 +91,8 @@ static int store_refs(transport_local *t) assert(t); - if (git_vector_init(&t->refs, ref_names.count, NULL) < 0) - return -1; - - if (git_reference_listall(&ref_names, t->repo, GIT_REF_LISTALL) < 0) + if (git_reference_listall(&ref_names, t->repo, GIT_REF_LISTALL) < 0 || + git_vector_init(&t->refs, (unsigned int)ref_names.count, NULL) < 0) goto on_error; /* Sort the references first */ diff --git a/tests-clar/refs/lookup.c b/tests-clar/refs/lookup.c new file mode 100644 index 000000000..d9b6c260f --- /dev/null +++ b/tests-clar/refs/lookup.c @@ -0,0 +1,42 @@ +#include "clar_libgit2.h" +#include "refs.h" + +static git_repository *g_repo; + +void test_refs_lookup__initialize(void) +{ + g_repo = cl_git_sandbox_init("testrepo.git"); +} + +void test_refs_lookup__cleanup(void) +{ + cl_git_sandbox_cleanup(); +} + +void test_refs_lookup__with_resolve(void) +{ + git_reference *a, *b, *temp; + + cl_git_pass(git_reference_lookup(&temp, g_repo, "HEAD")); + cl_git_pass(git_reference_resolve(&a, temp)); + git_reference_free(temp); + + cl_git_pass(git_reference_lookup_resolved(&b, g_repo, "HEAD", 5)); + cl_assert(git_reference_cmp(a, b) == 0); + git_reference_free(b); + + cl_git_pass(git_reference_lookup_resolved(&b, g_repo, "head-tracker", 5)); + cl_assert(git_reference_cmp(a, b) == 0); + git_reference_free(b); + + git_reference_free(a); +} + +void test_refs_lookup__oid(void) +{ + git_oid tag, expected; + + cl_git_pass(git_reference_lookup_oid(&tag, g_repo, "refs/tags/point_to_blob")); + cl_git_pass(git_oid_fromstr(&expected, "1385f264afb75a56a5bec74243be9b367ba4ca08")); + cl_assert(git_oid_cmp(&tag, &expected) == 0); +}