From 2638a03affdf57c989f573d48afca3b849cb4c1f Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Tue, 28 May 2013 17:50:47 +0200 Subject: [PATCH 1/7] This refs iterator pleases the gods. --- src/refdb_fs.c | 169 +++++++++++++++++++++++-------------------------- 1 file changed, 80 insertions(+), 89 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 7de56a1a0..97f0b07c4 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -29,7 +29,8 @@ GIT__USE_STRMAP; enum { PACKREF_HAS_PEEL = 1, PACKREF_WAS_LOOSE = 2, - PACKREF_CANNOT_PEEL = 4 + PACKREF_CANNOT_PEEL = 4, + PACKREF_SHADOWED = 8, }; enum { @@ -552,27 +553,16 @@ static int refdb_fs_backend__lookup( return result; } -struct dirent_list_data { - refdb_fs_backend *backend; - size_t repo_path_len; - unsigned int list_type:2; - - git_reference_foreach_cb callback; - void *callback_payload; - int callback_error; -}; - typedef struct { git_reference_iterator parent; - unsigned int loose; - /* packed */ - git_strmap *h; - khiter_t k; - /* loose */ - git_iterator *fsiter; - git_buf buf; + + git_vector loose; + unsigned int loose_pos; + khiter_t packed_pos; } refdb_fs_iter; +static int iter_load_loose_paths(refdb_fs_iter *iter); + static int refdb_fs_backend__iterator(git_reference_iterator **out, git_refdb_backend *_backend) { refdb_fs_iter *iter; @@ -588,103 +578,104 @@ static int refdb_fs_backend__iterator(git_reference_iterator **out, git_refdb_ba GITERR_CHECK_ALLOC(iter); iter->parent.backend = _backend; - iter->h = backend->refcache.packfile; - iter->k = kh_begin(backend->refcache.packfile); + iter_load_loose_paths(iter); *out = (git_reference_iterator *)iter; - return 0; } static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) { refdb_fs_iter *iter = (refdb_fs_iter *) _iter; + char *loose_path; + size_t i; - git_buf_free(&iter->buf); - git_iterator_free(iter->fsiter); + git_vector_foreach(&iter->loose, i, loose_path) { + free(loose_path); + } + + git_vector_free(&iter->loose); git__free(iter); } -static int iter_packed(const char **out, refdb_fs_iter *iter) -{ - /* Move forward to the next entry */ - while (!kh_exist(iter->h, iter->k)) { - iter->k++; - if (iter->k == kh_end(iter->h)) - return GIT_ITEROVER; - } - - *out = kh_key(iter->h, iter->k); - iter->k++; - - return 0; -} - -static int iter_loose(const char **out, refdb_fs_iter *iter) -{ - const git_index_entry *entry; - int retry; - git_strmap *packfile_refs; - refdb_fs_backend *backend = (refdb_fs_backend *) iter->parent.backend; - - packfile_refs = backend->refcache.packfile; - - do { - khiter_t pos; - if (git_iterator_current(&entry, iter->fsiter) < 0) - return -1; - - git_buf_clear(&iter->buf); - if (!entry) - return GIT_ITEROVER; - - if (git_buf_printf(&iter->buf, "refs/%s", entry->path) < 0) - return -1; - - git_iterator_advance(NULL, iter->fsiter); - - /* Skip this one if we already listed it in packed */ - pos = git_strmap_lookup_index(packfile_refs, git_buf_cstr(&iter->buf)); - retry = 0; - if (git_strmap_valid_index(packfile_refs, pos) || - !git_reference_is_valid_name(git_buf_cstr(&iter->buf))) - retry = 1; - - *out = git_buf_cstr(&iter->buf); - } while (retry); - - return 0; -} - -static int iter_loose_setup(refdb_fs_iter *iter) +static int iter_load_loose_paths(refdb_fs_iter *iter) { refdb_fs_backend *backend = (refdb_fs_backend *) iter->parent.backend; + git_strmap *packfile = backend->refcache.packfile; - git_buf_clear(&iter->buf); - if (git_buf_printf(&iter->buf, "%s/refs", backend->path) < 0) + git_buf path = GIT_BUF_INIT; + git_iterator *fsit; + const git_index_entry *entry = NULL; + + if (git_buf_printf(&path, "%s/refs", backend->path) < 0) return -1; - return git_iterator_for_filesystem(&iter->fsiter, git_buf_cstr(&iter->buf), 0, NULL, NULL); + if (git_iterator_for_filesystem(&fsit, git_buf_cstr(&path), 0, NULL, NULL) < 0) + return -1; + + git_vector_init(&iter->loose, 8, NULL); + git_buf_sets(&path, GIT_REFS_DIR); + + while (!git_iterator_current(&entry, fsit) && entry) { + const char *ref_name; + khiter_t pos; + + git_buf_truncate(&path, strlen(GIT_REFS_DIR)); + git_buf_puts(&path, entry->path); + ref_name = git_buf_cstr(&path); + + if (git__suffixcmp(ref_name, ".lock") == 0) { + git_iterator_advance(NULL, fsit); + continue; + } + + pos = git_strmap_lookup_index(packfile, ref_name); + if (git_strmap_valid_index(packfile, pos)) { + struct packref *ref = git_strmap_value_at(packfile, pos); + ref->flags |= PACKREF_SHADOWED; + } + + git_vector_insert(&iter->loose, git__strdup(ref_name)); + git_iterator_advance(NULL, fsit); + } + + git_iterator_free(fsit); + git_buf_free(&path); + + return 0; } static int refdb_fs_backend__next(const char **out, git_reference_iterator *_iter) { refdb_fs_iter *iter = (refdb_fs_iter *)_iter; + refdb_fs_backend *backend = (refdb_fs_backend *) iter->parent.backend; + git_strmap *packfile = backend->refcache.packfile; - if (iter->loose) - return iter_loose(out, iter); - - if (iter->k != kh_end(iter->h)) { - int error = iter_packed(out, iter); - if (error != GIT_ITEROVER) - return error; + if (iter->loose_pos < iter->loose.length) { + const char *path = git_vector_get(&iter->loose, iter->loose_pos++); + *out = path; + return 0; } - if (iter_loose_setup(iter) < 0) - return -1; - iter->loose = 1; + if (iter->packed_pos < kh_end(packfile)) { + struct packref *ref = NULL; - return iter_loose(out, iter); + do { + while (!kh_exist(packfile, iter->packed_pos)) { + iter->packed_pos++; + if (iter->packed_pos == kh_end(packfile)) + return GIT_ITEROVER; + } + + ref = kh_val(packfile, iter->packed_pos); + iter->packed_pos++; + } while (ref->flags & PACKREF_SHADOWED); + + *out = ref->name; + return 0; + } + + return GIT_ITEROVER; } static int loose_write(refdb_fs_backend *backend, const git_reference *ref) From 56960b8396d3aef0b39f32aa7a9749202f925ada Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Tue, 28 May 2013 20:47:55 +0200 Subject: [PATCH 2/7] Liike this --- include/git2/refs.h | 15 ++++++---- include/git2/sys/refdb_backend.h | 2 +- src/branch.c | 17 ++++++----- src/refdb.c | 19 +++++++++---- src/refdb.h | 2 +- src/refdb_fs.c | 18 ++++++++---- src/refs.c | 2 +- src/remote.c | 49 +++++++++----------------------- tests-clar/refdb/testdb.c | 12 ++++++-- tests-clar/refs/iterator.c | 35 ++++++++++++----------- 10 files changed, 90 insertions(+), 81 deletions(-) diff --git a/include/git2/refs.h b/include/git2/refs.h index 468d0f930..4d9ec8344 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -351,7 +351,9 @@ GIT_EXTERN(int) git_reference_cmp(git_reference *ref1, git_reference *ref2); * @param repo the repository * @return 0 or an error code */ -GIT_EXTERN(int) git_reference_iterator_new(git_reference_iterator **out, git_repository *repo); +GIT_EXTERN(int) git_reference_iterator_new( + git_reference_iterator **out, + git_repository *repo); /** * Create an iterator for the repo's references that match the @@ -362,16 +364,19 @@ GIT_EXTERN(int) git_reference_iterator_new(git_reference_iterator **out, git_rep * @param glob the glob to match against the reference names * @return 0 or an error code */ -GIT_EXTERN(int) git_reference_iterator_glob_new(git_reference_iterator **out, git_repository *repo, const char *glob); +GIT_EXTERN(int) git_reference_iterator_glob_new( + git_reference_iterator **out, + git_repository *repo, + const char *glob); /** - * Get the next reference name + * Get the next reference * - * @param out pointer in which to store the string + * @param out pointer in which to store the reference * @param iter the iterator * @param 0, GIT_ITEROVER if there are no more; or an error code */ -GIT_EXTERN(int) git_reference_next(const char **out, git_reference_iterator *iter); +GIT_EXTERN(int) git_reference_next(git_reference **out, git_reference_iterator *iter); /** * Free the iterator and its associated resources diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index 548597fbc..0820cd9c5 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -85,7 +85,7 @@ struct git_refdb_backend { * A refdb implementation must provide this function. */ int (*next)( - const char **name, + git_reference **ref, git_reference_iterator *iter); /** diff --git a/src/branch.c b/src/branch.c index dd6dc68f4..84efadae1 100644 --- a/src/branch.c +++ b/src/branch.c @@ -131,28 +131,32 @@ int git_branch_foreach( void *payload) { git_reference_iterator *iter; - const char *name; + git_reference *ref; int error; if (git_reference_iterator_new(&iter, repo) < 0) return -1; - while ((error = git_reference_next(&name, iter)) == 0) { + while ((error = git_reference_next(&ref, iter)) == 0) { if (list_flags & GIT_BRANCH_LOCAL && - git__prefixcmp(name, GIT_REFS_HEADS_DIR) == 0) { - if (callback(name + strlen(GIT_REFS_HEADS_DIR), GIT_BRANCH_LOCAL, payload)) { + 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; break; } } if (list_flags & GIT_BRANCH_REMOTE && - git__prefixcmp(name, GIT_REFS_REMOTES_DIR) == 0) { - if (callback(name + strlen(GIT_REFS_REMOTES_DIR), GIT_BRANCH_REMOTE, payload)) { + 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; break; } } + + git_reference_free(ref); } if (error == GIT_ITEROVER) @@ -160,7 +164,6 @@ int git_branch_foreach( git_reference_iterator_free(iter); return error; - } int git_branch_move( diff --git a/src/refdb.c b/src/refdb.c index 9f9037ce7..6cb879288 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -160,17 +160,26 @@ int git_refdb_iterator_glob(git_reference_iterator **out, git_refdb *db, const c return 0; } -int git_refdb_next(const char **out, git_reference_iterator *iter) +int git_refdb_next(git_reference **out, git_reference_iterator *iter) { int error; - if (!iter->glob) - return iter->backend->next(out, iter); + if (!iter->glob) { + if ((error = iter->backend->next(out, iter)) < 0) + return error; + + (*out)->db = iter->backend; + return 0; + } /* If the iterator has a glob, we need to filter */ while ((error = iter->backend->next(out, iter)) == 0) { - if (!p_fnmatch(iter->glob, *out, 0)) - break; + if (!p_fnmatch(iter->glob, (*out)->name, 0)) { + (*out)->db = iter->backend; + return 0; + } + + git_reference_free(*out); } return error; diff --git a/src/refdb.h b/src/refdb.h index 2edd05d18..82522e191 100644 --- a/src/refdb.h +++ b/src/refdb.h @@ -28,7 +28,7 @@ int git_refdb_lookup( int git_refdb_iterator(git_reference_iterator **out, git_refdb *db); int git_refdb_iterator_glob(git_reference_iterator **out, git_refdb *db, const char *glob); -int git_refdb_next(const char **out, git_reference_iterator *iter); +int git_refdb_next(git_reference **out, git_reference_iterator *iter); void git_refdb_iterator_free(git_reference_iterator *iter); int git_refdb_write(git_refdb *refdb, const git_reference *ref); diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 97f0b07c4..457964570 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -645,16 +645,19 @@ static int iter_load_loose_paths(refdb_fs_iter *iter) return 0; } -static int refdb_fs_backend__next(const char **out, git_reference_iterator *_iter) +static int refdb_fs_backend__next(git_reference **out, git_reference_iterator *_iter) { refdb_fs_iter *iter = (refdb_fs_iter *)_iter; - refdb_fs_backend *backend = (refdb_fs_backend *) iter->parent.backend; + refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.backend; git_strmap *packfile = backend->refcache.packfile; - if (iter->loose_pos < iter->loose.length) { + while (iter->loose_pos < iter->loose.length) { const char *path = git_vector_get(&iter->loose, iter->loose_pos++); - *out = path; - return 0; + + if (loose_lookup(out, backend, path) == 0) + return 0; + + giterr_clear(); } if (iter->packed_pos < kh_end(packfile)) { @@ -671,7 +674,10 @@ static int refdb_fs_backend__next(const char **out, git_reference_iterator *_ite iter->packed_pos++; } while (ref->flags & PACKREF_SHADOWED); - *out = ref->name; + *out = git_reference__alloc(ref->name, &ref->oid, &ref->peel); + if (*out == NULL) + return -1; + return 0; } diff --git a/src/refs.c b/src/refs.c index 9c6c5c623..43231b0cf 100644 --- a/src/refs.c +++ b/src/refs.c @@ -664,7 +664,7 @@ int git_reference_iterator_glob_new(git_reference_iterator **out, git_repository return git_refdb_iterator_glob(out, refdb, glob); } -int git_reference_next(const char **out, git_reference_iterator *iter) +int git_reference_next(git_reference **out, git_reference_iterator *iter) { return git_refdb_next(out, iter); } diff --git a/src/remote.c b/src/remote.c index 7a64622a5..266a3d914 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1239,30 +1239,25 @@ static int update_branch_remote_config_entry( } static int rename_one_remote_reference( - git_repository *repo, - const char *reference_name, + git_reference *reference, const char *old_remote_name, const char *new_remote_name) { int error = -1; git_buf new_name = GIT_BUF_INIT; - git_reference *reference = NULL; git_reference *newref = NULL; if (git_buf_printf( &new_name, GIT_REFS_REMOTES_DIR "%s%s", new_remote_name, - reference_name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name)) < 0) + reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name)) < 0) return -1; - if (git_reference_lookup(&reference, repo, reference_name) < 0) - goto cleanup; - + /* TODO: can we make this NULL? */ error = git_reference_rename(&newref, reference, git_buf_cstr(&new_name), 0); git_reference_free(reference); -cleanup: git_reference_free(newref); git_buf_free(&new_name); return error; @@ -1273,46 +1268,28 @@ static int rename_remote_references( const char *old_name, const char *new_name) { - git_vector refnames; int error = -1; - unsigned int i; - char *name; - const char *refname; + git_reference *ref; git_reference_iterator *iter; - if (git_vector_init(&refnames, 8, NULL) < 0) + if (git_reference_iterator_new(&iter, repo) < 0) return -1; - if (git_reference_iterator_new(&iter, repo) < 0) - goto cleanup; - - while ((error = git_reference_next(&refname, iter)) == 0) { - if (git__prefixcmp(refname, GIT_REFS_REMOTES_DIR)) + while ((error = git_reference_next(&ref, iter)) == 0) { + if (git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR)) continue; - if ((error = git_vector_insert(&refnames, git__strdup(refname))) < 0) - break; - + if ((error = rename_one_remote_reference(ref, old_name, new_name)) < 0) { + git_reference_iterator_free(iter); + return error; + } } git_reference_iterator_free(iter); + if (error == GIT_ITEROVER) - error = 0; - else - goto cleanup; + return 0; - git_vector_foreach(&refnames, i, name) { - if ((error = rename_one_remote_reference(repo, name, old_name, new_name)) < 0) - goto cleanup; - } - - error = 0; -cleanup: - git_vector_foreach(&refnames, i, name) { - git__free(name); - } - - git_vector_free(&refnames); return error; } diff --git a/tests-clar/refdb/testdb.c b/tests-clar/refdb/testdb.c index 961e18d44..c89bcce9f 100644 --- a/tests-clar/refdb/testdb.c +++ b/tests-clar/refdb/testdb.c @@ -134,7 +134,7 @@ static int refdb_test_backend__iterator(git_reference_iterator **out, git_refdb_ return 0; } -static int refdb_test_backend__next(const char **name, git_reference_iterator *_iter) +static int refdb_test_backend__next(git_reference **out, git_reference_iterator *_iter) { refdb_test_entry *entry; refdb_test_backend *backend = (refdb_test_backend *) _iter->backend; @@ -144,9 +144,15 @@ static int refdb_test_backend__next(const char **name, git_reference_iterator *_ if (!entry) return GIT_ITEROVER; - *name = entry->name; - iter->i++; + if (entry->type == GIT_REF_OID) { + *out = git_reference__alloc(entry->name, &entry->target.oid, NULL); + } else if (entry->type == GIT_REF_SYMBOLIC) { + *out = git_reference__alloc_symbolic(entry->name, entry->target.symbolic); + } else { + return -1; + } + iter->i++; return 0; } diff --git a/tests-clar/refs/iterator.c b/tests-clar/refs/iterator.c index d5555c657..7a966892b 100644 --- a/tests-clar/refs/iterator.c +++ b/tests-clar/refs/iterator.c @@ -38,40 +38,43 @@ static const char *refnames[] = { "refs/tags/wrapped_tag", }; +static int refcmp_cb(const void *a, const void *b) +{ + const git_reference *refa = (const git_reference *)a; + const git_reference *refb = (const git_reference *)b; + + return strcmp(refa->name, refb->name); +} + void test_refs_iterator__list(void) { git_reference_iterator *iter; git_vector output; - char *refname; + git_reference *ref; int error; size_t i; - cl_git_pass(git_vector_init(&output, 32, git__strcmp_cb)); + cl_git_pass(git_vector_init(&output, 32, &refcmp_cb)); cl_git_pass(git_reference_iterator_new(&iter, repo)); do { - const char *name; - error = git_reference_next(&name, iter); + error = git_reference_next(&ref, iter); cl_assert(error == 0 || error == GIT_ITEROVER); if (error != GIT_ITEROVER) { - char *dup = git__strdup(name); - cl_assert(dup != NULL); - cl_git_pass(git_vector_insert(&output, dup)); + cl_git_pass(git_vector_insert(&output, ref)); } } while (!error); + git_reference_iterator_free(iter); cl_assert_equal_i(output.length, ARRAY_SIZE(refnames)); git_vector_sort(&output); - git_vector_foreach(&output, i, refname) { - cl_assert_equal_s(refname, refnames[i]); + + git_vector_foreach(&output, i, ref) { + cl_assert_equal_s(ref->name, refnames[i]); + git_reference_free(ref); } - git_reference_iterator_free(iter); - - git_vector_foreach(&output, i, refname) { - git__free(refname); - } git_vector_free(&output); } @@ -79,14 +82,14 @@ void test_refs_iterator__empty(void) { git_reference_iterator *iter; git_odb *odb; - const char *name; + git_reference *ref; git_repository *empty; cl_git_pass(git_odb_new(&odb)); cl_git_pass(git_repository_wrap_odb(&empty, odb)); cl_git_pass(git_reference_iterator_new(&iter, empty)); - cl_assert_equal_i(GIT_ITEROVER, git_reference_next(&name, iter)); + cl_assert_equal_i(GIT_ITEROVER, git_reference_next(&ref, iter)); git_reference_iterator_free(iter); git_odb_free(odb); From ec24e542969f9d49e41e4c2cb3eac2259b1818c2 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 29 May 2013 22:47:37 +0200 Subject: [PATCH 3/7] What are the chances, really --- include/git2/refs.h | 12 +- include/git2/sys/refdb_backend.h | 49 +++---- src/clone.c | 2 +- src/refdb.c | 84 ++++-------- src/refdb.h | 7 +- src/refdb_fs.c | 132 +++++++++++++------ src/refs.c | 125 +++++++++++------- src/repository.c | 4 +- src/tag.c | 2 +- tests-clar/refdb/inmemory.c | 217 ------------------------------- tests-clar/refdb/testdb.c | 52 -------- 11 files changed, 236 insertions(+), 450 deletions(-) delete mode 100644 tests-clar/refdb/inmemory.c diff --git a/include/git2/refs.h b/include/git2/refs.h index 4d9ec8344..44b658d5b 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -308,7 +308,8 @@ GIT_EXTERN(int) git_reference_delete(git_reference *ref); */ GIT_EXTERN(int) git_reference_list(git_strarray *array, git_repository *repo); -typedef int (*git_reference_foreach_cb)(const char *refname, void *payload); +typedef int (*git_reference_foreach_cb)(git_reference *reference, void *payload); +typedef int (*git_reference_foreach_name_cb)(const char *name, void *payload); /** * Perform a callback on each reference in the repository. @@ -328,6 +329,11 @@ GIT_EXTERN(int) git_reference_foreach( git_reference_foreach_cb callback, void *payload); +GIT_EXTERN(int) git_reference_foreach_name( + git_repository *repo, + git_reference_foreach_name_cb callback, + void *payload); + /** * Free the given reference. * @@ -378,6 +384,8 @@ GIT_EXTERN(int) git_reference_iterator_glob_new( */ GIT_EXTERN(int) git_reference_next(git_reference **out, git_reference_iterator *iter); +GIT_EXTERN(int) git_reference_next_name(const char **out, git_reference_iterator *iter); + /** * Free the iterator and its associated resources * @@ -406,7 +414,7 @@ GIT_EXTERN(void) git_reference_iterator_free(git_reference_iterator *iter); GIT_EXTERN(int) git_reference_foreach_glob( git_repository *repo, const char *glob, - git_reference_foreach_cb callback, + git_reference_foreach_name_cb callback, void *payload); /** diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index 0820cd9c5..a78d22658 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -33,8 +33,27 @@ GIT_BEGIN_DECL * and assign `iter->parent.backend` to your `git_refdb_backend`. */ struct git_reference_iterator { - git_refdb_backend *backend; - char *glob; + git_refdb *db; + + /** + * Return the current reference and advance the iterator. + */ + int (*next)( + git_reference **ref, + git_reference_iterator *iter); + + /** + * Return the name of the current reference and advance the iterator + */ + int (*next_name)( + const char **ref_name, + git_reference_iterator *iter); + + /** + * Free the iterator + */ + void (*free)( + git_reference_iterator *iter); }; /** An instance for a custom backend */ @@ -65,36 +84,10 @@ struct git_refdb_backend { * A refdb implementation must provide this function. */ int (*iterator)( - git_reference_iterator **iter, - struct git_refdb_backend *backend); - - /** - * Allocate a glob-filtering iterator object for the backend. - * - * A refdb implementation may provide this function. If it's - * not available, the glob matching will be done by the frontend. - */ - int (*iterator_glob)( git_reference_iterator **iter, struct git_refdb_backend *backend, const char *glob); - /** - * Return the current value and advance the iterator. - * - * A refdb implementation must provide this function. - */ - int (*next)( - git_reference **ref, - git_reference_iterator *iter); - - /** - * Free the iterator - * - * A refdb implementation must provide this function. - */ - void (*iterator_free)( - git_reference_iterator *iter); /* * Writes the given reference to the refdb. A refdb implementation * must provide this function. diff --git a/src/clone.c b/src/clone.c index 7ebdb5765..72906c3ce 100644 --- a/src/clone.c +++ b/src/clone.c @@ -241,7 +241,7 @@ static int update_head_to_remote(git_repository *repo, git_remote *remote) } /* Not master. Check all the other refs. */ - if (git_reference_foreach( + if (git_reference_foreach_name( repo, reference_matches_remote_head, &head_info) < 0) diff --git a/src/refdb.c b/src/refdb.c index 6cb879288..e0701d347 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -114,99 +114,65 @@ int git_refdb_lookup(git_reference **out, git_refdb *db, const char *ref_name) assert(db && db->backend && out && ref_name); - if (!(error = db->backend->lookup(&ref, db->backend, ref_name))) { - ref->db = db; - *out = ref; - } else { - *out = NULL; - } + error = db->backend->lookup(&ref, db->backend, ref_name); + if (error < 0) + return error; - return error; + GIT_REFCOUNT_INC(db); + ref->db = db; + + *out = ref; + return 0; } -int git_refdb_iterator(git_reference_iterator **out, git_refdb *db) +int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob) { if (!db->backend || !db->backend->iterator) { giterr_set(GITERR_REFERENCE, "This backend doesn't support iterators"); return -1; } - if (db->backend->iterator(out, db->backend) < 0) + if (db->backend->iterator(out, db->backend, glob) < 0) return -1; + GIT_REFCOUNT_INC(db); + (*out)->db = db; + return 0; } -int git_refdb_iterator_glob(git_reference_iterator **out, git_refdb *db, const char *glob) -{ - if (!db->backend) { - giterr_set(GITERR_REFERENCE, "There are no backends loaded"); - return -1; - } - - if (db->backend->iterator_glob) - return db->backend->iterator_glob(out, db->backend, glob); - - /* If the backend doesn't support glob-filtering themselves, we have to do it */ - if (db->backend->iterator(out, db->backend) < 0) - return -1; - - (*out)->glob = git__strdup(glob); - if (!(*out)->glob) { - db->backend->iterator_free(*out); - return -1; - } - - return 0; -} - -int git_refdb_next(git_reference **out, git_reference_iterator *iter) +int git_refdb_iterator_next(git_reference **out, git_reference_iterator *iter) { int error; - if (!iter->glob) { - if ((error = iter->backend->next(out, iter)) < 0) - return error; + if ((error = iter->next(out, iter)) < 0) + return error; - (*out)->db = iter->backend; - return 0; - } + GIT_REFCOUNT_INC(iter->db); + (*out)->db = iter->db; - /* If the iterator has a glob, we need to filter */ - while ((error = iter->backend->next(out, iter)) == 0) { - if (!p_fnmatch(iter->glob, (*out)->name, 0)) { - (*out)->db = iter->backend; - return 0; - } + return 0; +} - git_reference_free(*out); - } - - return error; +int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter) +{ + return iter->next_name(out, iter); } void git_refdb_iterator_free(git_reference_iterator *iter) { - git__free(iter->glob); - iter->backend->iterator_free(iter); + GIT_REFCOUNT_DEC(iter->db, refdb_free); + iter->free(iter); } -struct glob_cb_data { - const char *glob; - git_reference_foreach_cb callback; - void *payload; -}; - int git_refdb_write(git_refdb *db, const git_reference *ref) { assert(db && db->backend); - return db->backend->write(db->backend, ref); } int git_refdb_delete(struct git_refdb *db, const git_reference *ref) { assert(db && db->backend); - return db->backend->delete(db->backend, ref); } diff --git a/src/refdb.h b/src/refdb.h index 82522e191..1dcd70da8 100644 --- a/src/refdb.h +++ b/src/refdb.h @@ -26,13 +26,12 @@ int git_refdb_lookup( git_refdb *refdb, const char *ref_name); -int git_refdb_iterator(git_reference_iterator **out, git_refdb *db); -int git_refdb_iterator_glob(git_reference_iterator **out, git_refdb *db, const char *glob); -int git_refdb_next(git_reference **out, git_reference_iterator *iter); +int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob); +int git_refdb_iterator_next(git_reference **out, git_reference_iterator *iter); +int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter); void git_refdb_iterator_free(git_reference_iterator *iter); int git_refdb_write(git_refdb *refdb, const git_reference *ref); - int git_refdb_delete(git_refdb *refdb, const git_reference *ref); #endif diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 457964570..ecd033de0 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -556,34 +556,12 @@ static int refdb_fs_backend__lookup( typedef struct { git_reference_iterator parent; + char *glob; git_vector loose; unsigned int loose_pos; khiter_t packed_pos; } refdb_fs_iter; -static int iter_load_loose_paths(refdb_fs_iter *iter); - -static int refdb_fs_backend__iterator(git_reference_iterator **out, git_refdb_backend *_backend) -{ - refdb_fs_iter *iter; - refdb_fs_backend *backend; - - assert(_backend); - backend = (refdb_fs_backend *)_backend; - - if (packed_load(backend) < 0) - return -1; - - iter = git__calloc(1, sizeof(refdb_fs_iter)); - GITERR_CHECK_ALLOC(iter); - - iter->parent.backend = _backend; - iter_load_loose_paths(iter); - - *out = (git_reference_iterator *)iter; - return 0; -} - static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) { refdb_fs_iter *iter = (refdb_fs_iter *) _iter; @@ -595,14 +573,14 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) } git_vector_free(&iter->loose); + + git__free(iter->glob); git__free(iter); } -static int iter_load_loose_paths(refdb_fs_iter *iter) +static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) { - refdb_fs_backend *backend = (refdb_fs_backend *) iter->parent.backend; git_strmap *packfile = backend->refcache.packfile; - git_buf path = GIT_BUF_INIT; git_iterator *fsit; const git_index_entry *entry = NULL; @@ -624,7 +602,8 @@ static int iter_load_loose_paths(refdb_fs_iter *iter) git_buf_puts(&path, entry->path); ref_name = git_buf_cstr(&path); - if (git__suffixcmp(ref_name, ".lock") == 0) { + if (git__suffixcmp(ref_name, ".lock") == 0 || + (iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) { git_iterator_advance(NULL, fsit); continue; } @@ -645,10 +624,11 @@ static int iter_load_loose_paths(refdb_fs_iter *iter) return 0; } -static int refdb_fs_backend__next(git_reference **out, git_reference_iterator *_iter) +static int refdb_fs_backend__iterator_next( + git_reference **out, git_reference_iterator *_iter) { refdb_fs_iter *iter = (refdb_fs_iter *)_iter; - refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.backend; + refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; git_strmap *packfile = backend->refcache.packfile; while (iter->loose_pos < iter->loose.length) { @@ -660,19 +640,23 @@ static int refdb_fs_backend__next(git_reference **out, git_reference_iterator *_ giterr_clear(); } - if (iter->packed_pos < kh_end(packfile)) { + while (iter->packed_pos < kh_end(packfile)) { struct packref *ref = NULL; - do { - while (!kh_exist(packfile, iter->packed_pos)) { - iter->packed_pos++; - if (iter->packed_pos == kh_end(packfile)) - return GIT_ITEROVER; - } - - ref = kh_val(packfile, iter->packed_pos); + while (!kh_exist(packfile, iter->packed_pos)) { iter->packed_pos++; - } while (ref->flags & PACKREF_SHADOWED); + if (iter->packed_pos == kh_end(packfile)) + return GIT_ITEROVER; + } + + ref = kh_val(packfile, iter->packed_pos); + iter->packed_pos++; + + if (ref->flags & PACKREF_SHADOWED) + continue; + + if (iter->glob && p_fnmatch(iter->glob, ref->name, 0) != 0) + continue; *out = git_reference__alloc(ref->name, &ref->oid, &ref->peel); if (*out == NULL) @@ -684,6 +668,74 @@ static int refdb_fs_backend__next(git_reference **out, git_reference_iterator *_ return GIT_ITEROVER; } +static int refdb_fs_backend__iterator_next_name( + const char **out, git_reference_iterator *_iter) +{ + refdb_fs_iter *iter = (refdb_fs_iter *)_iter; + refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; + git_strmap *packfile = backend->refcache.packfile; + + while (iter->loose_pos < iter->loose.length) { + const char *path = git_vector_get(&iter->loose, iter->loose_pos++); + + if (git_strmap_exists(packfile, path)) + continue; + + *out = path; + return 0; + } + + while (iter->packed_pos < kh_end(packfile)) { + while (!kh_exist(packfile, iter->packed_pos)) { + iter->packed_pos++; + if (iter->packed_pos == kh_end(packfile)) + return GIT_ITEROVER; + } + + *out = kh_key(packfile, iter->packed_pos); + iter->packed_pos++; + + if (iter->glob && p_fnmatch(iter->glob, *out, 0) != 0) + continue; + + return 0; + } + + return GIT_ITEROVER; +} + +static int refdb_fs_backend__iterator( + git_reference_iterator **out, git_refdb_backend *_backend, const char *glob) +{ + refdb_fs_iter *iter; + refdb_fs_backend *backend; + + assert(_backend); + backend = (refdb_fs_backend *)_backend; + + if (packed_load(backend) < 0) + return -1; + + iter = git__calloc(1, sizeof(refdb_fs_iter)); + GITERR_CHECK_ALLOC(iter); + + if (glob != NULL) + iter->glob = git__strdup(glob); + + iter->parent.next = refdb_fs_backend__iterator_next; + iter->parent.next_name = refdb_fs_backend__iterator_next_name; + iter->parent.free = refdb_fs_backend__iterator_free; + + if (iter_load_loose_paths(backend, iter) < 0) { + refdb_fs_backend__iterator_free((git_reference_iterator *)iter); + return -1; + } + + *out = (git_reference_iterator *)iter; + return 0; +} + + static int loose_write(refdb_fs_backend *backend, const git_reference *ref) { git_filebuf file = GIT_FILEBUF_INIT; @@ -1118,8 +1170,6 @@ int git_refdb_backend_fs( backend->parent.exists = &refdb_fs_backend__exists; backend->parent.lookup = &refdb_fs_backend__lookup; backend->parent.iterator = &refdb_fs_backend__iterator; - backend->parent.next = &refdb_fs_backend__next; - backend->parent.iterator_free = &refdb_fs_backend__iterator_free; backend->parent.write = &refdb_fs_backend__write; backend->parent.delete = &refdb_fs_backend__delete; backend->parent.compress = &refdb_fs_backend__compress; diff --git a/src/refs.c b/src/refs.c index 43231b0cf..a8583de19 100644 --- a/src/refs.c +++ b/src/refs.c @@ -104,20 +104,20 @@ struct reference_available_t { int available; }; -static int _reference_available_cb(const char *ref, void *data) +static int _reference_available_cb(const char *refname, void *data) { struct reference_available_t *d; - assert(ref && data); + assert(refname && data); d = (struct reference_available_t *)data; - if (!d->old_ref || strcmp(d->old_ref, ref)) { - size_t reflen = strlen(ref); + if (!d->old_ref || strcmp(d->old_ref, refname)) { + size_t reflen = strlen(refname); size_t newlen = strlen(d->new_ref); size_t cmplen = reflen < newlen ? reflen : newlen; - const char *lead = reflen < newlen ? d->new_ref : ref; + const char *lead = reflen < newlen ? d->new_ref : refname; - if (!strncmp(d->new_ref, ref, cmplen) && lead[cmplen] == '/') { + if (!strncmp(d->new_ref, refname, cmplen) && lead[cmplen] == '/') { d->available = 0; return -1; } @@ -126,6 +126,9 @@ static int _reference_available_cb(const char *ref, void *data) return 0; } +/** + * TODO: this should be part of the FS backend + */ static int reference_path_available( git_repository *repo, const char *ref, @@ -138,8 +141,7 @@ static int reference_path_available( data.old_ref = old_ref; data.available = 1; - error = git_reference_foreach( - repo, _reference_available_cb, (void *)&data); + error = git_reference_foreach_name(repo, _reference_available_cb, (void *)&data); if (error < 0) return error; @@ -430,6 +432,7 @@ static int reference__create( ref = git_reference__alloc_symbolic(name, symbolic); } + /* TODO: this needs to be written more explicitly */ GITERR_CHECK_ALLOC(ref); ref->db = refdb; @@ -558,6 +561,7 @@ int git_reference_rename( if (result == NULL) return -1; + /* TODO: this is bad */ result->db = ref->db; /* Check if we have to update HEAD. */ @@ -623,14 +627,69 @@ int git_reference_foreach( void *payload) { git_reference_iterator *iter; - const char *name; + git_reference *ref; int error; if (git_reference_iterator_new(&iter, repo) < 0) return -1; - while ((error = git_reference_next(&name, iter)) == 0) { - if (callback(name, payload)) { + while ((error = git_reference_next(&ref, iter)) == 0) { + if (callback(ref, payload)) { + error = GIT_EUSER; + goto out; + } + } + + if (error == GIT_ITEROVER) + error = 0; + +out: + git_reference_iterator_free(iter); + return error; +} + +int git_reference_foreach_name( + git_repository *repo, + git_reference_foreach_name_cb callback, + void *payload) +{ + git_reference_iterator *iter; + const char *refname; + int error; + + if (git_reference_iterator_new(&iter, repo) < 0) + return -1; + + while ((error = git_reference_next_name(&refname, iter)) == 0) { + if (callback(refname, payload)) { + error = GIT_EUSER; + goto out; + } + } + + if (error == GIT_ITEROVER) + error = 0; + +out: + git_reference_iterator_free(iter); + return error; +} + +int git_reference_foreach_glob( + git_repository *repo, + const char *glob, + git_reference_foreach_name_cb callback, + void *payload) +{ + git_reference_iterator *iter; + const char *refname; + int error; + + if (git_reference_iterator_glob_new(&iter, repo, glob) < 0) + return -1; + + while ((error = git_reference_next_name(&refname, iter)) == 0) { + if (callback(refname, payload)) { error = GIT_EUSER; goto out; } @@ -651,22 +710,28 @@ int git_reference_iterator_new(git_reference_iterator **out, git_repository *rep if (git_repository_refdb__weakptr(&refdb, repo) < 0) return -1; - return git_refdb_iterator(out, refdb); + return git_refdb_iterator(out, refdb, NULL); } -int git_reference_iterator_glob_new(git_reference_iterator **out, git_repository *repo, const char *glob) +int git_reference_iterator_glob_new( + git_reference_iterator **out, git_repository *repo, const char *glob) { git_refdb *refdb; if (git_repository_refdb__weakptr(&refdb, repo) < 0) return -1; - return git_refdb_iterator_glob(out, refdb, glob); + return git_refdb_iterator(out, refdb, glob); } int git_reference_next(git_reference **out, git_reference_iterator *iter) { - return git_refdb_next(out, iter); + return git_refdb_iterator_next(out, iter); +} + +int git_reference_next_name(const char **out, git_reference_iterator *iter) +{ + return git_refdb_iterator_next_name(out, iter); } void git_reference_iterator_free(git_reference_iterator *iter) @@ -693,7 +758,7 @@ int git_reference_list( if (git_vector_init(&ref_list, 8, NULL) < 0) return -1; - if (git_reference_foreach( + if (git_reference_foreach_name( repo, &cb__reflist_add, (void *)&ref_list) < 0) { git_vector_free(&ref_list); return -1; @@ -991,34 +1056,6 @@ int git_reference__update_terminal( return reference__update_terminal(repo, ref_name, oid, 0); } -int git_reference_foreach_glob( - git_repository *repo, - const char *glob, - git_reference_foreach_cb callback, - void *payload) -{ - git_reference_iterator *iter; - const char *name; - int error; - - if (git_reference_iterator_glob_new(&iter, repo, glob) < 0) - return -1; - - while ((error = git_reference_next(&name, iter)) == 0) { - if (callback(name, payload)) { - error = GIT_EUSER; - goto out; - } - } - - if (error == GIT_ITEROVER) - error = 0; - -out: - git_reference_iterator_free(iter); - return error; -} - int git_reference_has_log( git_reference *ref) { diff --git a/src/repository.c b/src/repository.c index 28505e822..2e7a334c9 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1473,12 +1473,14 @@ static int at_least_one_cb(const char *refname, void *payload) static int repo_contains_no_reference(git_repository *repo) { - int error = git_reference_foreach(repo, at_least_one_cb, NULL); + int error = git_reference_foreach_name(repo, &at_least_one_cb, NULL); if (error == GIT_EUSER) return 0; + if (!error) return 1; + return error; } diff --git a/src/tag.c b/src/tag.c index ecf3876cb..71f4c1eb1 100644 --- a/src/tag.c +++ b/src/tag.c @@ -440,7 +440,7 @@ int git_tag_foreach(git_repository *repo, git_tag_foreach_cb cb, void *cb_data) data.cb_data = cb_data; data.repo = repo; - return git_reference_foreach(repo, &tags_cb, &data); + return git_reference_foreach_name(repo, &tags_cb, &data); } typedef struct { diff --git a/tests-clar/refdb/inmemory.c b/tests-clar/refdb/inmemory.c deleted file mode 100644 index d2594cd6d..000000000 --- a/tests-clar/refdb/inmemory.c +++ /dev/null @@ -1,217 +0,0 @@ -#include "clar_libgit2.h" - -#include "buffer.h" -#include "posix.h" -#include "path.h" -#include "refs.h" - -#include "testdb.h" - -#define TEST_REPO_PATH "testrepo" - -static git_repository *repo; - -int unlink_ref(void *payload, git_buf *file) -{ - GIT_UNUSED(payload); - return p_unlink(git_buf_cstr(file)); -} - -int empty(void *payload, git_buf *file) -{ - GIT_UNUSED(payload); - GIT_UNUSED(file); - return -1; -} - -int ref_file_foreach(git_repository *repo, int (* cb)(void *payload, git_buf *filename)) -{ - const char *repo_path; - git_buf repo_refs_dir = GIT_BUF_INIT; - int error = 0; - - repo_path = git_repository_path(repo); - - git_buf_joinpath(&repo_refs_dir, repo_path, "HEAD"); - if (git_path_exists(git_buf_cstr(&repo_refs_dir)) && - cb(NULL, &repo_refs_dir) < 0) - return -1; - - git_buf_joinpath(&repo_refs_dir, repo_path, "refs"); - git_buf_joinpath(&repo_refs_dir, git_buf_cstr(&repo_refs_dir), "heads"); - if (git_path_direach(&repo_refs_dir, cb, NULL) != 0) - return -1; - - git_buf_joinpath(&repo_refs_dir, repo_path, "packed-refs"); - if (git_path_exists(git_buf_cstr(&repo_refs_dir)) && - cb(NULL, &repo_refs_dir) < 0) - return -1; - - git_buf_free(&repo_refs_dir); - - return error; -} - -void test_refdb_inmemory__initialize(void) -{ - git_buf repo_refs_dir = GIT_BUF_INIT; - git_refdb *refdb; - git_refdb_backend *refdb_backend; - - repo = cl_git_sandbox_init(TEST_REPO_PATH); - - cl_git_pass(git_repository_refdb(&refdb, repo)); - cl_git_pass(refdb_backend_test(&refdb_backend, repo)); - cl_git_pass(git_refdb_set_backend(refdb, refdb_backend)); - - ref_file_foreach(repo, unlink_ref); - - git_buf_free(&repo_refs_dir); - git_refdb_free(refdb); -} - -void test_refdb_inmemory__cleanup(void) -{ - cl_git_sandbox_cleanup(); -} - -void test_refdb_inmemory__doesnt_write_ref_file(void) -{ - git_reference *ref; - git_oid oid; - - cl_git_pass(git_oid_fromstr(&oid, "c47800c7266a2be04c571c04d5a6614691ea99bd")); - cl_git_pass(git_reference_create(&ref, repo, GIT_REFS_HEADS_DIR "test1", &oid, 0)); - - ref_file_foreach(repo, empty); - - git_reference_free(ref); -} - -void test_refdb_inmemory__read(void) -{ - git_reference *write1, *write2, *write3, *read1, *read2, *read3; - git_oid oid1, oid2, oid3; - - cl_git_pass(git_oid_fromstr(&oid1, "c47800c7266a2be04c571c04d5a6614691ea99bd")); - cl_git_pass(git_reference_create(&write1, repo, GIT_REFS_HEADS_DIR "test1", &oid1, 0)); - - cl_git_pass(git_oid_fromstr(&oid2, "e90810b8df3e80c413d903f631643c716887138d")); - cl_git_pass(git_reference_create(&write2, repo, GIT_REFS_HEADS_DIR "test2", &oid2, 0)); - - cl_git_pass(git_oid_fromstr(&oid3, "763d71aadf09a7951596c9746c024e7eece7c7af")); - cl_git_pass(git_reference_create(&write3, repo, GIT_REFS_HEADS_DIR "test3", &oid3, 0)); - - - cl_git_pass(git_reference_lookup(&read1, repo, GIT_REFS_HEADS_DIR "test1")); - cl_assert(strcmp(git_reference_name(read1), git_reference_name(write1)) == 0); - cl_assert(git_oid_cmp(git_reference_target(read1), git_reference_target(write1)) == 0); - - cl_git_pass(git_reference_lookup(&read2, repo, GIT_REFS_HEADS_DIR "test2")); - cl_assert(strcmp(git_reference_name(read2), git_reference_name(write2)) == 0); - cl_assert(git_oid_cmp(git_reference_target(read2), git_reference_target(write2)) == 0); - - cl_git_pass(git_reference_lookup(&read3, repo, GIT_REFS_HEADS_DIR "test3")); - cl_assert(strcmp(git_reference_name(read3), git_reference_name(write3)) == 0); - cl_assert(git_oid_cmp(git_reference_target(read3), git_reference_target(write3)) == 0); - - git_reference_free(write1); - git_reference_free(write2); - git_reference_free(write3); - - git_reference_free(read1); - git_reference_free(read2); - git_reference_free(read3); -} - -int foreach_test(const char *ref_name, void *payload) -{ - git_reference *ref; - git_oid expected; - size_t *i = payload; - - cl_git_pass(git_reference_lookup(&ref, repo, ref_name)); - - if (*i == 0) - cl_git_pass(git_oid_fromstr(&expected, "c47800c7266a2be04c571c04d5a6614691ea99bd")); - else if (*i == 1) - cl_git_pass(git_oid_fromstr(&expected, "e90810b8df3e80c413d903f631643c716887138d")); - else if (*i == 2) - cl_git_pass(git_oid_fromstr(&expected, "763d71aadf09a7951596c9746c024e7eece7c7af")); - - cl_assert(git_oid_cmp(&expected, git_reference_target(ref)) == 0); - - ++(*i); - - git_reference_free(ref); - - return 0; -} - -void test_refdb_inmemory__foreach(void) -{ - git_reference *write1, *write2, *write3; - git_oid oid1, oid2, oid3; - size_t i = 0; - - cl_git_pass(git_oid_fromstr(&oid1, "c47800c7266a2be04c571c04d5a6614691ea99bd")); - cl_git_pass(git_reference_create(&write1, repo, GIT_REFS_HEADS_DIR "test1", &oid1, 0)); - - cl_git_pass(git_oid_fromstr(&oid2, "e90810b8df3e80c413d903f631643c716887138d")); - cl_git_pass(git_reference_create(&write2, repo, GIT_REFS_HEADS_DIR "test2", &oid2, 0)); - - cl_git_pass(git_oid_fromstr(&oid3, "763d71aadf09a7951596c9746c024e7eece7c7af")); - cl_git_pass(git_reference_create(&write3, repo, GIT_REFS_HEADS_DIR "test3", &oid3, 0)); - - cl_git_pass(git_reference_foreach(repo,foreach_test, &i)); - cl_assert_equal_i(3, (int)i); - - git_reference_free(write1); - git_reference_free(write2); - git_reference_free(write3); -} - -int delete_test(const char *ref_name, void *payload) -{ - git_reference *ref; - git_oid expected; - size_t *i = payload; - - cl_git_pass(git_reference_lookup(&ref, repo, ref_name)); - - cl_git_pass(git_oid_fromstr(&expected, "e90810b8df3e80c413d903f631643c716887138d")); - cl_assert(git_oid_cmp(&expected, git_reference_target(ref)) == 0); - - ++(*i); - - git_reference_free(ref); - - return 0; -} - -void test_refdb_inmemory__delete(void) -{ - git_reference *write1, *write2, *write3; - git_oid oid1, oid2, oid3; - size_t i = 0; - - cl_git_pass(git_oid_fromstr(&oid1, "c47800c7266a2be04c571c04d5a6614691ea99bd")); - cl_git_pass(git_reference_create(&write1, repo, GIT_REFS_HEADS_DIR "test1", &oid1, 0)); - - cl_git_pass(git_oid_fromstr(&oid2, "e90810b8df3e80c413d903f631643c716887138d")); - cl_git_pass(git_reference_create(&write2, repo, GIT_REFS_HEADS_DIR "test2", &oid2, 0)); - - cl_git_pass(git_oid_fromstr(&oid3, "763d71aadf09a7951596c9746c024e7eece7c7af")); - cl_git_pass(git_reference_create(&write3, repo, GIT_REFS_HEADS_DIR "test3", &oid3, 0)); - - git_reference_delete(write1); - git_reference_free(write1); - - git_reference_delete(write3); - git_reference_free(write3); - - cl_git_pass(git_reference_foreach(repo, delete_test, &i)); - cl_assert_equal_i(1, (int)i); - - git_reference_free(write2); -} diff --git a/tests-clar/refdb/testdb.c b/tests-clar/refdb/testdb.c index c89bcce9f..6509ae9a2 100644 --- a/tests-clar/refdb/testdb.c +++ b/tests-clar/refdb/testdb.c @@ -112,55 +112,6 @@ static int refdb_test_backend__lookup( return GIT_ENOTFOUND; } -typedef struct { - git_reference_iterator parent; - size_t i; -} refdb_test_iter; - -static int refdb_test_backend__iterator(git_reference_iterator **out, git_refdb_backend *_backend) -{ - refdb_test_iter *iter; - - GIT_UNUSED(_backend); - - iter = git__calloc(1, sizeof(refdb_test_iter)); - GITERR_CHECK_ALLOC(iter); - - iter->parent.backend = _backend; - iter->i = 0; - - *out = (git_reference_iterator *) iter; - - return 0; -} - -static int refdb_test_backend__next(git_reference **out, git_reference_iterator *_iter) -{ - refdb_test_entry *entry; - refdb_test_backend *backend = (refdb_test_backend *) _iter->backend; - refdb_test_iter *iter = (refdb_test_iter *) _iter; - - entry = git_vector_get(&backend->refs, iter->i); - if (!entry) - return GIT_ITEROVER; - - if (entry->type == GIT_REF_OID) { - *out = git_reference__alloc(entry->name, &entry->target.oid, NULL); - } else if (entry->type == GIT_REF_SYMBOLIC) { - *out = git_reference__alloc_symbolic(entry->name, entry->target.symbolic); - } else { - return -1; - } - - iter->i++; - return 0; -} - -static void refdb_test_backend__iterator_free(git_reference_iterator *iter) -{ - git__free(iter); -} - static void refdb_test_entry_free(refdb_test_entry *entry) { if (entry->type == GIT_REF_SYMBOLIC) @@ -222,9 +173,6 @@ int refdb_backend_test( backend->parent.exists = &refdb_test_backend__exists; backend->parent.lookup = &refdb_test_backend__lookup; - backend->parent.iterator = &refdb_test_backend__iterator; - backend->parent.next = &refdb_test_backend__next; - backend->parent.iterator_free = &refdb_test_backend__iterator_free; backend->parent.write = &refdb_test_backend__write; backend->parent.delete = &refdb_test_backend__delete; backend->parent.free = &refdb_test_backend__free; From 4e6e2ff26f5a04a4628aa0d81e5d5d73acf28ec4 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 30 May 2013 03:47:10 +0200 Subject: [PATCH 4/7] ...Aaaand this works --- include/git2/refs.h | 7 +- include/git2/sys/refdb_backend.h | 9 +- src/branch.c | 13 ++- src/refdb.c | 34 +++++- src/refdb.h | 11 +- src/refdb_fs.c | 136 ++++++++++++++++++---- src/refs.c | 192 ++++--------------------------- src/remote.c | 5 +- tests-clar/refdb/testdb.c | 182 ----------------------------- tests-clar/refdb/testdb.h | 9 -- 10 files changed, 194 insertions(+), 404 deletions(-) delete mode 100644 tests-clar/refdb/testdb.c delete mode 100644 tests-clar/refdb/testdb.h diff --git a/include/git2/refs.h b/include/git2/refs.h index 44b658d5b..cec830653 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -257,11 +257,6 @@ GIT_EXTERN(int) git_reference_set_target( * The new name will be checked for validity. * See `git_reference_create_symbolic()` for rules about valid names. * - * On success, the given git_reference will be deleted from disk and a - * new `git_reference` will be returned. - * - * The reference will be immediately renamed in-memory and on disk. - * * If the `force` flag is not enabled, and there's already * a reference with the given name, the renaming will fail. * @@ -277,7 +272,7 @@ GIT_EXTERN(int) git_reference_set_target( * */ GIT_EXTERN(int) git_reference_rename( - git_reference **out, + git_reference **new_ref, git_reference *ref, const char *new_name, int force); diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index a78d22658..9b457b074 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -92,13 +92,18 @@ struct git_refdb_backend { * Writes the given reference to the refdb. A refdb implementation * must provide this function. */ - int (*write)(git_refdb_backend *backend, const git_reference *ref); + int (*write)(git_refdb_backend *backend, + const git_reference *ref, int force); + + int (*rename)( + git_reference **out, git_refdb_backend *backend, + const char *old_name, const char *new_name, int force); /** * Deletes the given reference from the refdb. A refdb implementation * must provide this function. */ - int (*delete)(git_refdb_backend *backend, const git_reference *ref); + int (*delete)(git_refdb_backend *backend, const char *ref_name); /** * Suggests that the given refdb compress or optimize its references. diff --git a/src/branch.c b/src/branch.c index 84efadae1..de38e3355 100644 --- a/src/branch.c +++ b/src/branch.c @@ -182,18 +182,21 @@ int git_branch_move( if (!git_reference_is_branch(branch)) return not_a_local_branch(git_reference_name(branch)); - if ((error = git_buf_joinpath(&new_reference_name, GIT_REFS_HEADS_DIR, new_branch_name)) < 0 || - (error = git_buf_printf(&old_config_section, "branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR))) < 0 || - (error = git_buf_printf(&new_config_section, "branch.%s", new_branch_name)) < 0) + error = git_buf_joinpath(&new_reference_name, GIT_REFS_HEADS_DIR, new_branch_name); + if (error < 0) goto done; + git_buf_printf(&old_config_section, + "branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR)); + + git_buf_printf(&new_config_section, "branch.%s", new_branch_name); + if ((error = git_config_rename_section(git_reference_owner(branch), git_buf_cstr(&old_config_section), git_buf_cstr(&new_config_section))) < 0) goto done; - if ((error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force)) < 0) - goto done; + error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force); done: git_buf_free(&new_reference_name); diff --git a/src/refdb.c b/src/refdb.c index e0701d347..d8daa7773 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -165,14 +165,40 @@ void git_refdb_iterator_free(git_reference_iterator *iter) iter->free(iter); } -int git_refdb_write(git_refdb *db, const git_reference *ref) +int git_refdb_write(git_refdb *db, git_reference *ref, int force) { assert(db && db->backend); - return db->backend->write(db->backend, ref); + + GIT_REFCOUNT_INC(db); + ref->db = db; + + return db->backend->write(db->backend, ref, force); } -int git_refdb_delete(struct git_refdb *db, const git_reference *ref) +int git_refdb_rename( + git_reference **out, + git_refdb *db, + const char *old_name, + const char *new_name, + int force) +{ + int error; + + assert(db && db->backend); + error = db->backend->rename(out, db->backend, old_name, new_name, force); + if (error < 0) + return error; + + if (out) { + GIT_REFCOUNT_INC(db); + (*out)->db = db; + } + + return 0; +} + +int git_refdb_delete(struct git_refdb *db, const char *ref_name) { assert(db && db->backend); - return db->backend->delete(db->backend, ref); + return db->backend->delete(db->backend, ref_name); } diff --git a/src/refdb.h b/src/refdb.h index 1dcd70da8..62068db39 100644 --- a/src/refdb.h +++ b/src/refdb.h @@ -26,12 +26,19 @@ int git_refdb_lookup( git_refdb *refdb, const char *ref_name); +int git_refdb_rename( + git_reference **out, + git_refdb *db, + const char *old_name, + const char *new_name, + int force); + int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob); int git_refdb_iterator_next(git_reference **out, git_reference_iterator *iter); int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter); void git_refdb_iterator_free(git_reference_iterator *iter); -int git_refdb_write(git_refdb *refdb, const git_reference *ref); -int git_refdb_delete(git_refdb *refdb, const git_reference *ref); +int git_refdb_write(git_refdb *refdb, git_reference *ref, int force); +int git_refdb_delete(git_refdb *refdb, const char *ref_name); #endif diff --git a/src/refdb_fs.c b/src/refdb_fs.c index ecd033de0..c40c52438 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -116,11 +116,8 @@ static int packed_parse_oid( git_oid_cpy(&ref->oid, &id); - ref->flags = 0; - *ref_out = ref; *buffer_out = refname_end + 1; - return 0; corrupt: @@ -735,6 +732,58 @@ static int refdb_fs_backend__iterator( return 0; } +static bool ref_is_available( + const char *old_ref, const char *new_ref, const char *this_ref) +{ + if (old_ref == NULL || strcmp(old_ref, this_ref)) { + size_t reflen = strlen(this_ref); + size_t newlen = strlen(new_ref); + size_t cmplen = reflen < newlen ? reflen : newlen; + const char *lead = reflen < newlen ? new_ref : this_ref; + + if (!strncmp(new_ref, this_ref, cmplen) && lead[cmplen] == '/') { + return false; + } + } + + return true; +} + +static int reference_path_available( + refdb_fs_backend *backend, + const char *new_ref, + const char* old_ref, + int force) +{ + struct packref *this_ref; + + if (packed_load(backend) < 0) + return -1; + + if (!force) { + int exists; + + if (refdb_fs_backend__exists(&exists, (git_refdb_backend *)backend, new_ref) < 0) + return -1; + + if (exists) { + giterr_set(GITERR_REFERENCE, + "Failed to write reference '%s': a reference with " + " that name already exists.", new_ref); + return GIT_EEXISTS; + } + } + + git_strmap_foreach_value(backend->refcache.packfile, this_ref, { + if (!ref_is_available(old_ref, new_ref, this_ref->name)) { + giterr_set(GITERR_REFERENCE, + "The path to reference '%s' collides with an existing one", new_ref); + return -1; + } + }); + + return 0; +} static int loose_write(refdb_fs_backend *backend, const git_reference *ref) { @@ -744,8 +793,7 @@ static int loose_write(refdb_fs_backend *backend, const git_reference *ref) /* Remove a possibly existing empty directory hierarchy * which name would collide with the reference name */ - if (git_futils_rmdir_r(ref->name, backend->path, - GIT_RMDIR_SKIP_NONEMPTY) < 0) + if (git_futils_rmdir_r(ref->name, backend->path, GIT_RMDIR_SKIP_NONEMPTY) < 0) return -1; if (git_buf_joinpath(&ref_path, backend->path, ref->name) < 0) @@ -1005,37 +1053,40 @@ cleanup_memory: static int refdb_fs_backend__write( git_refdb_backend *_backend, - const git_reference *ref) + const git_reference *ref, + int force) { refdb_fs_backend *backend; + int error; assert(_backend); backend = (refdb_fs_backend *)_backend; + error = reference_path_available(backend, ref->name, NULL, force); + if (error < 0) + return error; + return loose_write(backend, ref); } static int refdb_fs_backend__delete( git_refdb_backend *_backend, - const git_reference *ref) + const char *ref_name) { refdb_fs_backend *backend; - git_repository *repo; git_buf loose_path = GIT_BUF_INIT; struct packref *pack_ref; khiter_t pack_ref_pos; - int error = 0, pack_error; + int error = 0; bool loose_deleted = 0; assert(_backend); - assert(ref); + assert(ref_name); backend = (refdb_fs_backend *)_backend; - repo = backend->repo; /* If a loose reference exists, remove it from the filesystem */ - - if (git_buf_joinpath(&loose_path, repo->path_repository, ref->name) < 0) + if (git_buf_joinpath(&loose_path, backend->path, ref_name) < 0) return -1; if (git_path_isfile(loose_path.ptr)) { @@ -1049,22 +1100,66 @@ static int refdb_fs_backend__delete( return error; /* If a packed reference exists, remove it from the packfile and repack */ + error = packed_map_entry(&pack_ref, &pack_ref_pos, backend, ref_name); - if ((pack_error = packed_map_entry(&pack_ref, &pack_ref_pos, backend, ref->name)) == 0) { + if (error == GIT_ENOTFOUND) + return loose_deleted ? 0 : GIT_ENOTFOUND; + + if (error == 0) { git_strmap_delete_at(backend->refcache.packfile, pack_ref_pos); git__free(pack_ref); - error = packed_write(backend); } - if (pack_error == GIT_ENOTFOUND) - error = loose_deleted ? 0 : GIT_ENOTFOUND; - else - error = pack_error; - return error; } +static int refdb_fs_backend__rename( + git_reference **out, + git_refdb_backend *_backend, + const char *old_name, + const char *new_name, + int force) +{ + refdb_fs_backend *backend; + git_reference *old, *new; + int error; + + assert(_backend); + backend = (refdb_fs_backend *)_backend; + + error = reference_path_available(backend, new_name, old_name, force); + if (error < 0) + return error; + + error = refdb_fs_backend__lookup(&old, _backend, old_name); + if (error < 0) + return error; + + error = refdb_fs_backend__delete(_backend, old_name); + if (error < 0) { + git_reference_free(old); + return error; + } + + new = realloc(old, sizeof(git_reference) + strlen(new_name) + 1); + memcpy(new->name, new_name, strlen(new_name) + 1); + + error = loose_write(backend, new); + if (error < 0) { + git_reference_free(new); + return error; + } + + if (out) { + *out = new; + } else { + git_reference_free(new); + } + + return 0; +} + static int refdb_fs_backend__compress(git_refdb_backend *_backend) { refdb_fs_backend *backend; @@ -1172,6 +1267,7 @@ int git_refdb_backend_fs( backend->parent.iterator = &refdb_fs_backend__iterator; backend->parent.write = &refdb_fs_backend__write; backend->parent.delete = &refdb_fs_backend__delete; + backend->parent.rename = &refdb_fs_backend__rename; backend->parent.compress = &refdb_fs_backend__compress; backend->parent.free = &refdb_fs_backend__free; diff --git a/src/refs.c b/src/refs.c index a8583de19..c60e042d9 100644 --- a/src/refs.c +++ b/src/refs.c @@ -98,122 +98,9 @@ void git_reference_free(git_reference *reference) git__free(reference); } -struct reference_available_t { - const char *new_ref; - const char *old_ref; - int available; -}; - -static int _reference_available_cb(const char *refname, void *data) -{ - struct reference_available_t *d; - - assert(refname && data); - d = (struct reference_available_t *)data; - - if (!d->old_ref || strcmp(d->old_ref, refname)) { - size_t reflen = strlen(refname); - size_t newlen = strlen(d->new_ref); - size_t cmplen = reflen < newlen ? reflen : newlen; - const char *lead = reflen < newlen ? d->new_ref : refname; - - if (!strncmp(d->new_ref, refname, cmplen) && lead[cmplen] == '/') { - d->available = 0; - return -1; - } - } - - return 0; -} - -/** - * TODO: this should be part of the FS backend - */ -static int reference_path_available( - git_repository *repo, - const char *ref, - const char* old_ref) -{ - int error; - struct reference_available_t data; - - data.new_ref = ref; - data.old_ref = old_ref; - data.available = 1; - - error = git_reference_foreach_name(repo, _reference_available_cb, (void *)&data); - if (error < 0) - return error; - - if (!data.available) { - giterr_set(GITERR_REFERENCE, - "The path to reference '%s' collides with an existing one", ref); - return -1; - } - - return 0; -} - -/* - * Check if a reference could be written to disk, based on: - * - * - Whether a reference with the same name already exists, - * and we are allowing or disallowing overwrites - * - * - Whether the name of the reference would collide with - * an existing path - */ -static int reference_can_write( - git_repository *repo, - const char *refname, - const char *previous_name, - int force) -{ - git_refdb *refdb; - - if (git_repository_refdb__weakptr(&refdb, repo) < 0) - return -1; - - /* see if the reference shares a path with an existing reference; - * if a path is shared, we cannot create the reference, even when forcing */ - if (reference_path_available(repo, refname, previous_name) < 0) - return -1; - - /* check if the reference actually exists, but only if we are not forcing - * the rename. If we are forcing, it's OK to overwrite */ - if (!force) { - int exists; - - if (git_refdb_exists(&exists, refdb, refname) < 0) - return -1; - - /* We cannot proceed if the reference already exists and we're not forcing - * the rename; the existing one would be overwritten */ - if (exists) { - giterr_set(GITERR_REFERENCE, - "A reference with that name (%s) already exists", refname); - return GIT_EEXISTS; - } - } - - /* FIXME: if the reference exists and we are forcing, do we really need to - * remove the reference first? - * - * Two cases: - * - * - the reference already exists and is loose: not a problem, the file - * gets overwritten on disk - * - * - the reference already exists and is packed: we write a new one as - * loose, which by all means renders the packed one useless - */ - - return 0; -} - int git_reference_delete(git_reference *ref) { - return git_refdb_delete(ref->db, ref); + return git_refdb_delete(ref->db, ref->name); } int git_reference_lookup(git_reference **ref_out, @@ -420,23 +307,24 @@ static int reference__create( if (ref_out) *ref_out = NULL; - if ((error = git_reference__normalize_name_lax(normalized, sizeof(normalized), name)) < 0 || - (error = reference_can_write(repo, normalized, NULL, force)) < 0 || - (error = git_repository_refdb__weakptr(&refdb, repo)) < 0) + error = git_reference__normalize_name_lax(normalized, sizeof(normalized), name); + if (error < 0) + return error; + + error = git_repository_refdb__weakptr(&refdb, repo); + if (error < 0) return error; if (oid != NULL) { assert(symbolic == NULL); - ref = git_reference__alloc(name, oid, NULL); + ref = git_reference__alloc(normalized, oid, NULL); } else { - ref = git_reference__alloc_symbolic(name, symbolic); + ref = git_reference__alloc_symbolic(normalized, symbolic); } - /* TODO: this needs to be written more explicitly */ GITERR_CHECK_ALLOC(ref); - ref->db = refdb; - if ((error = git_refdb_write(refdb, ref)) < 0) { + if ((error = git_refdb_write(refdb, ref, force)) < 0) { git_reference_free(ref); return error; } @@ -533,77 +421,41 @@ int git_reference_rename( unsigned int normalization_flags; char normalized[GIT_REFNAME_MAX]; bool should_head_be_updated = false; - git_reference *result = NULL; int error = 0; int reference_has_log; - *out = NULL; - normalization_flags = ref->type == GIT_REF_SYMBOLIC ? GIT_REF_FORMAT_ALLOW_ONELEVEL : GIT_REF_FORMAT_NORMAL; if ((error = git_reference_normalize_name( - normalized, sizeof(normalized), new_name, normalization_flags)) < 0 || - (error = reference_can_write(ref->db->repo, normalized, ref->name, force)) < 0) + normalized, sizeof(normalized), new_name, normalization_flags)) < 0) return error; - /* - * Create the new reference. - */ - if (ref->type == GIT_REF_OID) { - result = git_reference__alloc(new_name, &ref->target.oid, &ref->peel); - } else if (ref->type == GIT_REF_SYMBOLIC) { - result = git_reference__alloc_symbolic(new_name, ref->target.symbolic); - } else { - assert(0); - } - - if (result == NULL) - return -1; - - /* TODO: this is bad */ - result->db = ref->db; - /* Check if we have to update HEAD. */ if ((error = git_branch_is_head(ref)) < 0) - goto on_error; + return error; should_head_be_updated = (error > 0); - /* Now delete the old ref and save the new one. */ - if ((error = git_refdb_delete(ref->db, ref)) < 0) - goto on_error; - - /* Save the new reference. */ - if ((error = git_refdb_write(ref->db, result)) < 0) - goto rollback; + if ((error = git_refdb_rename(out, ref->db, ref->name, new_name, force)) < 0) + return error; /* Update HEAD it was poiting to the reference being renamed. */ - if (should_head_be_updated && (error = git_repository_set_head(ref->db->repo, new_name)) < 0) { + if (should_head_be_updated && + (error = git_repository_set_head(ref->db->repo, new_name)) < 0) { giterr_set(GITERR_REFERENCE, "Failed to update HEAD after renaming reference"); - goto on_error; + return error; } /* Rename the reflog file, if it exists. */ reference_has_log = git_reference_has_log(ref); - if (reference_has_log < 0) { - error = reference_has_log; - goto on_error; - } + if (reference_has_log < 0) + return reference_has_log; + if (reference_has_log && (error = git_reflog_rename(ref, new_name)) < 0) - goto on_error; + return error; - *out = result; - - return error; - -rollback: - git_refdb_write(ref->db, ref); - -on_error: - git_reference_free(result); - - return error; + return 0; } int git_reference_resolve(git_reference **ref_out, const git_reference *ref) diff --git a/src/remote.c b/src/remote.c index 266a3d914..674c2776c 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1245,7 +1245,6 @@ static int rename_one_remote_reference( { int error = -1; git_buf new_name = GIT_BUF_INIT; - git_reference *newref = NULL; if (git_buf_printf( &new_name, @@ -1254,11 +1253,9 @@ static int rename_one_remote_reference( reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name)) < 0) return -1; - /* TODO: can we make this NULL? */ - error = git_reference_rename(&newref, reference, git_buf_cstr(&new_name), 0); + error = git_reference_rename(NULL, reference, git_buf_cstr(&new_name), 0); git_reference_free(reference); - git_reference_free(newref); git_buf_free(&new_name); return error; } diff --git a/tests-clar/refdb/testdb.c b/tests-clar/refdb/testdb.c deleted file mode 100644 index 6509ae9a2..000000000 --- a/tests-clar/refdb/testdb.c +++ /dev/null @@ -1,182 +0,0 @@ -#include "vector.h" -#include "util.h" -#include "testdb.h" - -typedef struct refdb_test_backend { - git_refdb_backend parent; - - git_repository *repo; - git_vector refs; -} refdb_test_backend; - -typedef struct refdb_test_entry { - char *name; - git_ref_t type; - - union { - git_oid oid; - char *symbolic; - } target; -} refdb_test_entry; - -static int ref_name_cmp(const void *a, const void *b) -{ - return strcmp(git_reference_name((git_reference *)a), - git_reference_name((git_reference *)b)); -} - -static int refdb_test_backend__exists( - int *exists, - git_refdb_backend *_backend, - const char *ref_name) -{ - refdb_test_backend *backend; - refdb_test_entry *entry; - size_t i; - - assert(_backend); - backend = (refdb_test_backend *)_backend; - - *exists = 0; - - git_vector_foreach(&backend->refs, i, entry) { - if (strcmp(entry->name, ref_name) == 0) { - *exists = 1; - break; - } - } - - return 0; -} - -static int refdb_test_backend__write( - git_refdb_backend *_backend, - const git_reference *ref) -{ - refdb_test_backend *backend; - refdb_test_entry *entry; - - assert(_backend); - backend = (refdb_test_backend *)_backend; - - entry = git__calloc(1, sizeof(refdb_test_entry)); - GITERR_CHECK_ALLOC(entry); - - entry->name = git__strdup(git_reference_name(ref)); - GITERR_CHECK_ALLOC(entry->name); - - entry->type = git_reference_type(ref); - - if (entry->type == GIT_REF_OID) - git_oid_cpy(&entry->target.oid, git_reference_target(ref)); - else { - entry->target.symbolic = git__strdup(git_reference_symbolic_target(ref)); - GITERR_CHECK_ALLOC(entry->target.symbolic); - } - - git_vector_insert(&backend->refs, entry); - - return 0; -} - -static int refdb_test_backend__lookup( - git_reference **out, - git_refdb_backend *_backend, - const char *ref_name) -{ - refdb_test_backend *backend; - refdb_test_entry *entry; - size_t i; - - assert(_backend); - backend = (refdb_test_backend *)_backend; - - git_vector_foreach(&backend->refs, i, entry) { - if (strcmp(entry->name, ref_name) == 0) { - - if (entry->type == GIT_REF_OID) { - *out = git_reference__alloc(ref_name, - &entry->target.oid, NULL); - } else if (entry->type == GIT_REF_SYMBOLIC) { - *out = git_reference__alloc_symbolic(ref_name, - entry->target.symbolic); - } - - if (*out == NULL) - return -1; - - return 0; - } - } - - return GIT_ENOTFOUND; -} - -static void refdb_test_entry_free(refdb_test_entry *entry) -{ - if (entry->type == GIT_REF_SYMBOLIC) - git__free(entry->target.symbolic); - - git__free(entry->name); - git__free(entry); -} - -static int refdb_test_backend__delete( - git_refdb_backend *_backend, - const git_reference *ref) -{ - refdb_test_backend *backend; - refdb_test_entry *entry; - size_t i; - - assert(_backend); - backend = (refdb_test_backend *)_backend; - - git_vector_foreach(&backend->refs, i, entry) { - if (strcmp(entry->name, git_reference_name(ref)) == 0) { - git_vector_remove(&backend->refs, i); - refdb_test_entry_free(entry); - } - } - - return GIT_ENOTFOUND; -} - -static void refdb_test_backend__free(git_refdb_backend *_backend) -{ - refdb_test_backend *backend; - refdb_test_entry *entry; - size_t i; - - assert(_backend); - backend = (refdb_test_backend *)_backend; - - git_vector_foreach(&backend->refs, i, entry) - refdb_test_entry_free(entry); - - git_vector_free(&backend->refs); - git__free(backend); -} - -int refdb_backend_test( - git_refdb_backend **backend_out, - git_repository *repo) -{ - refdb_test_backend *backend; - - backend = git__calloc(1, sizeof(refdb_test_backend)); - GITERR_CHECK_ALLOC(backend); - - git_vector_init(&backend->refs, 0, ref_name_cmp); - - backend->repo = repo; - - backend->parent.exists = &refdb_test_backend__exists; - backend->parent.lookup = &refdb_test_backend__lookup; - backend->parent.write = &refdb_test_backend__write; - backend->parent.delete = &refdb_test_backend__delete; - backend->parent.free = &refdb_test_backend__free; - - *backend_out = (git_refdb_backend *)backend; - return 0; -} diff --git a/tests-clar/refdb/testdb.h b/tests-clar/refdb/testdb.h deleted file mode 100644 index a0d1bbc48..000000000 --- a/tests-clar/refdb/testdb.h +++ /dev/null @@ -1,9 +0,0 @@ -#include -#include -#include -#include -#include - -int refdb_backend_test( - git_refdb_backend **backend_out, - git_repository *repo); From 979f75d8e1df7e8e43797822d5a55a8eff74fa74 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 30 May 2013 17:19:43 +0200 Subject: [PATCH 5/7] Refcounting --- src/refdb.c | 6 +++--- src/refdb.h | 2 ++ src/refs.c | 3 +++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/refdb.c b/src/refdb.c index d8daa7773..359842e44 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -86,7 +86,7 @@ int git_refdb_compress(git_refdb *db) return 0; } -static void refdb_free(git_refdb *db) +void git_refdb__free(git_refdb *db) { refdb_free_backend(db); git__free(db); @@ -97,7 +97,7 @@ void git_refdb_free(git_refdb *db) if (db == NULL) return; - GIT_REFCOUNT_DEC(db, refdb_free); + GIT_REFCOUNT_DEC(db, git_refdb__free); } int git_refdb_exists(int *exists, git_refdb *refdb, const char *ref_name) @@ -161,7 +161,7 @@ int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter) void git_refdb_iterator_free(git_reference_iterator *iter) { - GIT_REFCOUNT_DEC(iter->db, refdb_free); + GIT_REFCOUNT_DEC(iter->db, git_refdb__free); iter->free(iter); } diff --git a/src/refdb.h b/src/refdb.h index 62068db39..3aea37b62 100644 --- a/src/refdb.h +++ b/src/refdb.h @@ -16,6 +16,8 @@ struct git_refdb { git_refdb_backend *backend; }; +void git_refdb__free(git_refdb *db); + int git_refdb_exists( int *exists, git_refdb *refdb, diff --git a/src/refs.c b/src/refs.c index c60e042d9..7103decbd 100644 --- a/src/refs.c +++ b/src/refs.c @@ -95,6 +95,9 @@ void git_reference_free(git_reference *reference) if (reference->type == GIT_REF_SYMBOLIC) git__free(reference->target.symbolic); + if (reference->db) + GIT_REFCOUNT_DEC(reference->db, git_refdb__free); + git__free(reference); } From 1ed356dcfef449313bac3ce795f26d19423c744c Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 30 May 2013 21:04:28 +0200 Subject: [PATCH 6/7] Frees --- src/refdb_fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index c40c52438..e7ffcd4a1 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -566,7 +566,7 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) size_t i; git_vector_foreach(&iter->loose, i, loose_path) { - free(loose_path); + git__free(loose_path); } git_vector_free(&iter->loose); @@ -1232,7 +1232,7 @@ static int setup_namespace(git_buf *path, git_repository *repo) } git_buf_printf(path, "refs/namespaces/%s/refs", end); - free(parts); + git__free(parts); /* Make sure that the folder with the namespace exists */ if (git_futils_mkdir_r(git_buf_cstr(path), repo->path_repository, 0777) < 0) From cee695ae6b9a9f586d32d0b9460a358bfdc4fe1b Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 31 May 2013 12:18:43 -0700 Subject: [PATCH 7/7] Make iterators use GIT_ITEROVER & smart advance 1. internal iterators now return GIT_ITEROVER when you go past the last item in the iteration. 2. git_iterator_advance will "advance" to the first item in the iteration if it is called immediately after creating the iterator, which allows a simpler idiom for basic iteration. 3. if git_iterator_advance encounters an error reading data (e.g. a missing tree or an unreadable file), it returns the error but also attempts to advance past the invalid data to prevent an infinite loop. Updated all tests and internal usage of iterators to account for these new behaviors. --- src/checkout.c | 31 ++--- src/diff.c | 34 ++++-- src/iterator.c | 214 +++++++++++++++++++++++----------- src/iterator.h | 38 +++++- src/merge.c | 15 ++- src/notes.c | 14 +-- src/refdb_fs.c | 10 +- src/submodule.c | 18 ++- tests-clar/diff/iterator.c | 116 +++++++++--------- tests-clar/repo/iterator.c | 21 ++-- tests-clar/submodule/status.c | 5 +- 11 files changed, 329 insertions(+), 187 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index c28fcdee0..7a2e68300 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -495,12 +495,9 @@ static int checkout_action( if (cmp == 0) { if (wd->mode == GIT_FILEMODE_TREE) { /* case 2 - entry prefixed by workdir tree */ - if ((error = git_iterator_advance_into(&wd, workdir)) < 0) { - if (error != GIT_ENOTFOUND || - git_iterator_advance(&wd, workdir) < 0) - goto fail; - } - + error = git_iterator_advance_into_or_over(&wd, workdir); + if (error && error != GIT_ITEROVER) + goto fail; *wditem_ptr = wd; continue; } @@ -515,8 +512,10 @@ static int checkout_action( } /* case 1 - handle wd item (if it matches pathspec) */ - if (checkout_action_wd_only(data, workdir, wd, pathspec) < 0 || - git_iterator_advance(&wd, workdir) < 0) + if (checkout_action_wd_only(data, workdir, wd, pathspec) < 0) + goto fail; + if ((error = git_iterator_advance(&wd, workdir)) < 0 && + error != GIT_ITEROVER) goto fail; *wditem_ptr = wd; @@ -539,8 +538,9 @@ static int checkout_action( if (delta->status == GIT_DELTA_TYPECHANGE) { if (delta->old_file.mode == GIT_FILEMODE_TREE) { act = checkout_action_with_wd(data, delta, wd); - if (git_iterator_advance_into(&wd, workdir) < 0) - wd = NULL; + if ((error = git_iterator_advance_into(&wd, workdir)) < 0 && + error != GIT_ENOTFOUND) + goto fail; *wditem_ptr = wd; return act; } @@ -550,8 +550,9 @@ static int checkout_action( delta->old_file.mode == GIT_FILEMODE_COMMIT) { act = checkout_action_with_wd(data, delta, wd); - if (git_iterator_advance(&wd, workdir) < 0) - wd = NULL; + if ((error = git_iterator_advance(&wd, workdir)) < 0 && + error != GIT_ITEROVER) + goto fail; *wditem_ptr = wd; return act; } @@ -582,6 +583,9 @@ static int checkout_remaining_wd_items( error = git_iterator_advance(&wd, workdir); } + if (error == GIT_ITEROVER) + error = 0; + return error; } @@ -603,7 +607,8 @@ static int checkout_get_actions( git_pathspec_init(&pathspec, &data->opts.paths, &pathpool) < 0) return -1; - if ((error = git_iterator_current(&wditem, workdir)) < 0) + if ((error = git_iterator_current(&wditem, workdir)) < 0 && + error != GIT_ITEROVER) goto fail; deltas = &data->diff->deltas; diff --git a/src/diff.c b/src/diff.c index b96ff4705..05ef4f16b 100644 --- a/src/diff.c +++ b/src/diff.c @@ -796,6 +796,9 @@ static int diff_scan_inside_untracked_dir( done: git_buf_free(&base); + if (error == GIT_ITEROVER) + error = 0; + return error; } @@ -981,8 +984,11 @@ static int handle_matched_item( { int error = 0; - if (!(error = maybe_modified(diff, info)) && - !(error = git_iterator_advance(&info->oitem, info->old_iter))) + if ((error = maybe_modified(diff, info)) < 0) + return error; + + if (!(error = git_iterator_advance(&info->oitem, info->old_iter)) || + error == GIT_ITEROVER) error = git_iterator_advance(&info->nitem, info->new_iter); return error; @@ -1011,15 +1017,22 @@ int git_diff__from_iterators( /* make iterators have matching icase behavior */ if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_DELTAS_ARE_ICASE)) { - if (!(error = git_iterator_set_ignore_case(old_iter, true))) - error = git_iterator_set_ignore_case(new_iter, true); + if ((error = git_iterator_set_ignore_case(old_iter, true)) < 0 || + (error = git_iterator_set_ignore_case(new_iter, true)) < 0) + goto cleanup; } /* finish initialization */ - if (!error && - !(error = diff_list_apply_options(diff, opts)) && - !(error = git_iterator_current(&info.oitem, old_iter))) - error = git_iterator_current(&info.nitem, new_iter); + if ((error = diff_list_apply_options(diff, opts)) < 0) + goto cleanup; + + if ((error = git_iterator_current(&info.oitem, old_iter)) < 0 && + error != GIT_ITEROVER) + goto cleanup; + if ((error = git_iterator_current(&info.nitem, new_iter)) < 0 && + error != GIT_ITEROVER) + goto cleanup; + error = 0; /* run iterators building diffs */ while (!error && (info.oitem || info.nitem)) { @@ -1041,8 +1054,13 @@ int git_diff__from_iterators( */ else error = handle_matched_item(diff, &info); + + /* because we are iterating over two lists, ignore ITEROVER */ + if (error == GIT_ITEROVER) + error = 0; } +cleanup: if (!error) *diff_ptr = diff; else diff --git a/src/iterator.c b/src/iterator.c index ff08c1ce0..4360b99ad 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -46,6 +46,9 @@ #define iterator__dont_autoexpand(I) iterator__flag(I,DONT_AUTOEXPAND) #define iterator__do_autoexpand(I) !iterator__flag(I,DONT_AUTOEXPAND) +#define GIT_ITERATOR_FIRST_ACCESS (1 << 15) +#define iterator__has_been_accessed(I) iterator__flag(I,FIRST_ACCESS) + #define iterator__end(I) ((git_iterator *)(I))->end #define iterator__past_end(I,PATH) \ (iterator__end(I) && ((git_iterator *)(I))->prefixcomp((PATH),iterator__end(I)) > 0) @@ -68,6 +71,8 @@ static int iterator__reset_range( GITERR_CHECK_ALLOC(iter->end); } + iter->flags &= ~GIT_ITERATOR_FIRST_ACCESS; + return 0; } @@ -109,7 +114,7 @@ static int empty_iterator__noop(const git_index_entry **e, git_iterator *i) { GIT_UNUSED(i); iterator__clear_entry(e); - return 0; + return GIT_ITEROVER; } static int empty_iterator__seek(git_iterator *i, const char *p) @@ -193,6 +198,7 @@ typedef struct { git_buf path; int path_ambiguities; bool path_has_filename; + bool entry_is_current; int (*strncomp)(const char *a, const char *b, size_t sz); } tree_iterator; @@ -266,9 +272,28 @@ static int tree_iterator__search_cmp(const void *key, const void *val, void *p) ((tree_iterator *)p)->strncomp); } +static bool tree_iterator__move_to_next( + tree_iterator *ti, tree_iterator_frame *tf) +{ + if (tf->next > tf->current + 1) + ti->path_ambiguities--; + + if (!tf->up) { /* at root */ + tf->current = tf->next; + return false; + } + + for (; tf->current < tf->next; tf->current++) { + git_tree_free(tf->entries[tf->current]->tree); + tf->entries[tf->current]->tree = NULL; + } + + return (tf->current < tf->n_entries); +} + static int tree_iterator__set_next(tree_iterator *ti, tree_iterator_frame *tf) { - int error; + int error = 0; const git_tree_entry *te, *last = NULL; tf->next = tf->current; @@ -279,18 +304,23 @@ static int tree_iterator__set_next(tree_iterator *ti, tree_iterator_frame *tf) if (last && tree_iterator__te_cmp(last, te, ti->strncomp)) break; - /* load trees for items in [current,next) range */ - if (git_tree_entry__is_tree(te) && - (error = git_tree_lookup( - &tf->entries[tf->next]->tree, ti->base.repo, &te->oid)) < 0) - return error; + /* try to load trees for items in [current,next) range */ + if (!error && git_tree_entry__is_tree(te)) + error = git_tree_lookup( + &tf->entries[tf->next]->tree, ti->base.repo, &te->oid); } if (tf->next > tf->current + 1) ti->path_ambiguities++; + /* if a tree lookup failed, advance over this span and return failure */ + if (error < 0) { + tree_iterator__move_to_next(ti, tf); + return error; + } + if (last && !tree_iterator__current_filename(ti, last)) - return -1; + return -1; /* must have been allocation failure */ return 0; } @@ -308,7 +338,7 @@ static int tree_iterator__push_frame(tree_iterator *ti) size_t i, n_entries = 0; if (head->current >= head->n_entries || !head->entries[head->current]->tree) - return 0; + return GIT_ITEROVER; for (i = head->current; i < head->next; ++i) n_entries += git_tree_entrycount(head->entries[i]->tree); @@ -359,7 +389,7 @@ static int tree_iterator__push_frame(tree_iterator *ti) } } - ti->path_has_filename = false; + ti->path_has_filename = ti->entry_is_current = false; if ((error = tree_iterator__set_next(ti, tf)) < 0) return error; @@ -371,25 +401,6 @@ static int tree_iterator__push_frame(tree_iterator *ti) return 0; } -static bool tree_iterator__move_to_next( - tree_iterator *ti, tree_iterator_frame *tf) -{ - if (tf->next > tf->current + 1) - ti->path_ambiguities--; - - if (!tf->up) { /* at root */ - tf->current = tf->next; - return false; - } - - for (; tf->current < tf->next; tf->current++) { - git_tree_free(tf->entries[tf->current]->tree); - tf->entries[tf->current]->tree = NULL; - } - - return (tf->current < tf->n_entries); -} - static bool tree_iterator__pop_frame(tree_iterator *ti, bool final) { tree_iterator_frame *tf = ti->head; @@ -412,7 +423,7 @@ static bool tree_iterator__pop_frame(tree_iterator *ti, bool final) return true; } -static int tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final) +static void tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final) { while (tree_iterator__pop_frame(ti, final)) /* pop to root */; @@ -421,22 +432,18 @@ static int tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final) ti->path_ambiguities = 0; git_buf_clear(&ti->path); } - - return 0; } -static int tree_iterator__current( - const git_index_entry **entry, git_iterator *self) +static int tree_iterator__update_entry(tree_iterator *ti) { - tree_iterator *ti = (tree_iterator *)self; - tree_iterator_frame *tf = ti->head; - const git_tree_entry *te; + tree_iterator_frame *tf; + const git_tree_entry *te; - iterator__clear_entry(entry); + if (ti->entry_is_current) + return 0; - if (tf->current >= tf->n_entries) - return 0; - te = tf->entries[tf->current]->te; + tf = ti->head; + te = tf->entries[tf->current]->te; ti->entry.mode = te->attr; git_oid_cpy(&ti->entry.oid, &te->oid); @@ -447,12 +454,36 @@ static int tree_iterator__current( if (ti->path_ambiguities > 0) tree_iterator__rewrite_filename(ti); - if (iterator__past_end(ti, ti->entry.path)) - return tree_iterator__pop_all(ti, true, false); + if (iterator__past_end(ti, ti->entry.path)) { + tree_iterator__pop_all(ti, true, false); + return GIT_ITEROVER; + } + + ti->entry_is_current = true; + + return 0; +} + +static int tree_iterator__current( + const git_index_entry **entry, git_iterator *self) +{ + int error; + tree_iterator *ti = (tree_iterator *)self; + tree_iterator_frame *tf = ti->head; + + iterator__clear_entry(entry); + + if (tf->current >= tf->n_entries) + return GIT_ITEROVER; + + if ((error = tree_iterator__update_entry(ti)) < 0) + return error; if (entry) *entry = &ti->entry; + ti->base.flags |= GIT_ITERATOR_FIRST_ACCESS; + return 0; } @@ -464,8 +495,10 @@ static int tree_iterator__advance_into( iterator__clear_entry(entry); - if (tree_iterator__at_tree(ti) && - !(error = tree_iterator__push_frame(ti))) + if (tree_iterator__at_tree(ti)) + error = tree_iterator__push_frame(ti); + + if (!error && entry) error = tree_iterator__current(entry, self); return error; @@ -480,8 +513,11 @@ static int tree_iterator__advance( iterator__clear_entry(entry); - if (tf->current > tf->n_entries) - return 0; + if (tf->current >= tf->n_entries) + return GIT_ITEROVER; + + if (!iterator__has_been_accessed(ti)) + return tree_iterator__current(entry, self); if (iterator__do_autoexpand(ti) && iterator__include_trees(ti) && tree_iterator__at_tree(ti)) @@ -489,7 +525,7 @@ static int tree_iterator__advance( if (ti->path_has_filename) { git_buf_rtruncate_at_char(&ti->path, '/'); - ti->path_has_filename = false; + ti->path_has_filename = ti->entry_is_current = false; } /* scan forward and up, advancing in frame or popping frame when done */ @@ -699,7 +735,9 @@ static int index_iterator__current( if (entry) *entry = ie; - return 0; + ii->base.flags |= GIT_ITERATOR_FIRST_ACCESS; + + return (ie != NULL) ? 0 : GIT_ITEROVER; } static int index_iterator__at_end(git_iterator *self) @@ -715,6 +753,9 @@ static int index_iterator__advance( size_t entrycount = git_index_entrycount(ii->index); const git_index_entry *ie; + if (!iterator__has_been_accessed(ii)) + return index_iterator__current(entry, self); + if (index_iterator__at_tree(ii)) { if (iterator__do_autoexpand(ii)) { ii->partial.ptr[ii->partial_pos] = ii->restore_terminator; @@ -906,6 +947,8 @@ static void fs_iterator__pop_frame( } static int fs_iterator__update_entry(fs_iterator *fi); +static int fs_iterator__advance_over( + const git_index_entry **entry, git_iterator *self); static int fs_iterator__entry_cmp(const void *i, const void *item) { @@ -945,7 +988,13 @@ static int fs_iterator__expand_dir(fs_iterator *fi) fi->path.ptr, fi->root_len, iterator__ignore_case(fi), fi->base.start, fi->base.end, &ff->entries); - if (error < 0 || ff->entries.length == 0) { + if (error < 0) { + fs_iterator__free_frame(ff); + fs_iterator__advance_over(NULL, (git_iterator *)fi); + return error; + } + + if (ff->entries.length == 0) { fs_iterator__free_frame(ff); return GIT_ENOTFOUND; } @@ -966,9 +1015,14 @@ static int fs_iterator__current( const git_index_entry **entry, git_iterator *self) { fs_iterator *fi = (fs_iterator *)self; + const git_index_entry *fe = (fi->entry.path == NULL) ? NULL : &fi->entry; + if (entry) - *entry = (fi->entry.path == NULL) ? NULL : &fi->entry; - return 0; + *entry = fe; + + fi->base.flags |= GIT_ITERATOR_FIRST_ACCESS; + + return (fe != NULL) ? 0 : GIT_ITEROVER; } static int fs_iterator__at_end(git_iterator *self) @@ -998,6 +1052,9 @@ static int fs_iterator__advance_into( if (!error && entry) error = fs_iterator__current(entry, iter); + if (!error && !fi->entry.path) + error = GIT_ITEROVER; + return error; } @@ -1035,6 +1092,9 @@ static int fs_iterator__advance( { fs_iterator *fi = (fs_iterator *)self; + if (!iterator__has_been_accessed(fi)) + return fs_iterator__current(entry, self); + /* given include_trees & autoexpand, we might have to go into a tree */ if (iterator__do_autoexpand(fi) && fi->entry.path != NULL && @@ -1063,18 +1123,23 @@ static int fs_iterator__seek(git_iterator *self, const char *prefix) static int fs_iterator__reset( git_iterator *self, const char *start, const char *end) { + int error; fs_iterator *fi = (fs_iterator *)self; while (fi->stack != NULL && fi->stack->next != NULL) fs_iterator__pop_frame(fi, fi->stack, false); fi->depth = 0; - if (iterator__reset_range(self, start, end) < 0) - return -1; + if ((error = iterator__reset_range(self, start, end)) < 0) + return error; fs_iterator__seek_frame_start(fi, fi->stack); - return fs_iterator__update_entry(fi); + error = fs_iterator__update_entry(fi); + if (error == GIT_ITEROVER) + error = 0; + + return error; } static void fs_iterator__free(git_iterator *self) @@ -1089,18 +1154,23 @@ static void fs_iterator__free(git_iterator *self) static int fs_iterator__update_entry(fs_iterator *fi) { - git_path_with_stat *ps = - git_vector_get(&fi->stack->entries, fi->stack->index); + git_path_with_stat *ps; - git_buf_truncate(&fi->path, fi->root_len); memset(&fi->entry, 0, sizeof(fi->entry)); + if (!fi->stack) + return GIT_ITEROVER; + + ps = git_vector_get(&fi->stack->entries, fi->stack->index); if (!ps) - return 0; + return GIT_ITEROVER; + + git_buf_truncate(&fi->path, fi->root_len); if (git_buf_put(&fi->path, ps->path, ps->path_len) < 0) return -1; + if (iterator__past_end(fi, fi->path.ptr + fi->root_len)) - return 0; + return GIT_ITEROVER; fi->entry.path = ps->path; git_index_entry__init_from_stat(&fi->entry, &ps->st); @@ -1114,8 +1184,13 @@ static int fs_iterator__update_entry(fs_iterator *fi) return fs_iterator__advance_over(NULL, (git_iterator *)fi); /* if this is a tree and trees aren't included, then skip */ - if (fi->entry.mode == GIT_FILEMODE_TREE && !iterator__include_trees(fi)) - return git_iterator_advance(NULL, (git_iterator *)fi); + if (fi->entry.mode == GIT_FILEMODE_TREE && !iterator__include_trees(fi)) { + int error = fs_iterator__advance_into(NULL, (git_iterator *)fi); + if (error != GIT_ENOTFOUND) + return error; + giterr_clear(); + return fs_iterator__advance_over(NULL, (git_iterator *)fi); + } return 0; } @@ -1131,13 +1206,14 @@ static int fs_iterator__initialize( } fi->root_len = fi->path.size; - if ((error = fs_iterator__expand_dir(fi)) == GIT_ENOTFOUND) { - giterr_clear(); - error = 0; - } - if (error) { - git_iterator_free((git_iterator *)fi); - fi = NULL; + if ((error = fs_iterator__expand_dir(fi)) < 0) { + if (error == GIT_ENOTFOUND || error == GIT_ITEROVER) { + giterr_clear(); + error = 0; + } else { + git_iterator_free((git_iterator *)fi); + fi = NULL; + } } *out = (git_iterator *)fi; diff --git a/src/iterator.h b/src/iterator.h index 7998f7c6b..493ff4b2a 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -142,9 +142,9 @@ GIT_INLINE(int) git_iterator_advance( * * If the current item is not a tree, this is a no-op. * - * For working directory iterators only, a tree (i.e. directory) can be - * empty. In that case, this function returns GIT_ENOTFOUND and does not - * advance. That can't happen for tree and index iterators. + * For filesystem and working directory iterators, a tree (i.e. directory) + * can be empty. In that case, this function returns GIT_ENOTFOUND and + * does not advance. That can't happen for tree and index iterators. */ GIT_INLINE(int) git_iterator_advance_into( const git_index_entry **entry, git_iterator *iter) @@ -152,18 +152,50 @@ GIT_INLINE(int) git_iterator_advance_into( return iter->cb->advance_into(entry, iter); } +/** + * Advance into a tree or skip over it if it is empty. + * + * Because `git_iterator_advance_into` may return GIT_ENOTFOUND if the + * directory is empty (only with filesystem and working directory + * iterators) and a common response is to just call `git_iterator_advance` + * when that happens, this bundles the two into a single simple call. + */ +GIT_INLINE(int) git_iterator_advance_into_or_over( + const git_index_entry **entry, git_iterator *iter) +{ + int error = iter->cb->advance_into(entry, iter); + if (error == GIT_ENOTFOUND) { + giterr_clear(); + error = iter->cb->advance(entry, iter); + } + return error; +} + +/* Seek is currently unimplemented */ GIT_INLINE(int) git_iterator_seek( git_iterator *iter, const char *prefix) { return iter->cb->seek(iter, prefix); } +/** + * Go back to the start of the iteration. + * + * This resets the iterator to the start of the iteration. It also allows + * you to reset the `start` and `end` pathname boundaries of the iteration + * when doing so. + */ GIT_INLINE(int) git_iterator_reset( git_iterator *iter, const char *start, const char *end) { return iter->cb->reset(iter, start, end); } +/** + * Check if the iterator is at the end + * + * @return 0 if not at end, >0 if at end + */ GIT_INLINE(int) git_iterator_at_end(git_iterator *iter) { return iter->cb->at_end(iter); diff --git a/src/merge.c b/src/merge.c index 11345587c..047d96013 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1259,7 +1259,8 @@ int git_merge_diff_list__find_differences( /* Set up the iterators */ for (i = 0; i < 3; i++) { - if ((error = git_iterator_current(&items[i], iterators[i])) < 0) + error = git_iterator_current(&items[i], iterators[i]); + if (error < 0 && error != GIT_ITEROVER) goto done; } @@ -1313,11 +1314,16 @@ int git_merge_diff_list__find_differences( error = merge_index_insert_conflict(diff_list, &df_data, cur_items); else error = merge_index_insert_unmodified(diff_list, cur_items); + if (error < 0) + goto done; /* Advance each iterator that participated */ for (i = 0; i < 3; i++) { - if (cur_items[i] != NULL && - (error = git_iterator_advance(&items[i], iterators[i])) < 0) + if (cur_items[i] == NULL) + continue; + + error = git_iterator_advance(&items[i], iterators[i]); + if (error < 0 && error != GIT_ITEROVER) goto done; } } @@ -1326,6 +1332,9 @@ done: for (i = 0; i < 3; i++) git_iterator_free(iterators[i]); + if (error == GIT_ITEROVER) + error = 0; + return error; } diff --git a/src/notes.c b/src/notes.c index 3e3db58db..beace1b50 100644 --- a/src/notes.c +++ b/src/notes.c @@ -647,18 +647,12 @@ int git_note_next( const git_index_entry *item; if ((error = git_iterator_current(&item, it)) < 0) - goto exit; + return error; - if (item != NULL) { - git_oid_cpy(note_id, &item->oid); - error = process_entry_path(item->path, annotated_id); + git_oid_cpy(note_id, &item->oid); - if (error >= 0) - error = git_iterator_advance(NULL, it); - } else { - error = GIT_ITEROVER; - } + if (!(error = process_entry_path(item->path, annotated_id))) + git_iterator_advance(NULL, it); -exit: return error; } diff --git a/src/refdb_fs.c b/src/refdb_fs.c index e7ffcd4a1..4083ba9e5 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -582,6 +582,9 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) git_iterator *fsit; const git_index_entry *entry = NULL; + if (!backend->path) /* do nothing if no path for loose refs */ + return 0; + if (git_buf_printf(&path, "%s/refs", backend->path) < 0) return -1; @@ -591,7 +594,7 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) git_vector_init(&iter->loose, 8, NULL); git_buf_sets(&path, GIT_REFS_DIR); - while (!git_iterator_current(&entry, fsit) && entry) { + while (!git_iterator_advance(&entry, fsit)) { const char *ref_name; khiter_t pos; @@ -600,10 +603,8 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) ref_name = git_buf_cstr(&path); if (git__suffixcmp(ref_name, ".lock") == 0 || - (iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) { - git_iterator_advance(NULL, fsit); + (iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) continue; - } pos = git_strmap_lookup_index(packfile, ref_name); if (git_strmap_valid_index(packfile, pos)) { @@ -612,7 +613,6 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) } git_vector_insert(&iter->loose, git__strdup(ref_name)); - git_iterator_advance(NULL, fsit); } git_iterator_free(fsit); diff --git a/src/submodule.c b/src/submodule.c index f4fbcd35a..16114d8ac 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1143,9 +1143,7 @@ static int load_submodule_config_from_index( (error = git_iterator_for_index(&i, index, 0, NULL, NULL)) < 0) return error; - error = git_iterator_current(&entry, i); - - while (!error && entry != NULL) { + while (!(error = git_iterator_advance(&entry, i))) { if (S_ISGITLINK(entry->mode)) { error = submodule_load_from_index(repo, entry); @@ -1158,10 +1156,11 @@ static int load_submodule_config_from_index( if (strcmp(entry->path, GIT_MODULES_FILE) == 0) git_oid_cpy(gitmodules_oid, &entry->oid); } - - error = git_iterator_advance(&entry, i); } + if (error == GIT_ITEROVER) + error = 0; + git_iterator_free(i); return error; @@ -1183,9 +1182,7 @@ static int load_submodule_config_from_head( return error; } - error = git_iterator_current(&entry, i); - - while (!error && entry != NULL) { + while (!(error = git_iterator_advance(&entry, i))) { if (S_ISGITLINK(entry->mode)) { error = submodule_load_from_head(repo, entry->path, &entry->oid); @@ -1199,10 +1196,11 @@ static int load_submodule_config_from_head( git_oid_iszero(gitmodules_oid)) git_oid_cpy(gitmodules_oid, &entry->oid); } - - error = git_iterator_advance(&entry, i); } + if (error == GIT_ITEROVER) + error = 0; + git_iterator_free(i); git_tree_free(head); diff --git a/tests-clar/diff/iterator.c b/tests-clar/diff/iterator.c index 15b10465a..bbdae8ad1 100644 --- a/tests-clar/diff/iterator.c +++ b/tests-clar/diff/iterator.c @@ -31,7 +31,7 @@ static void tree_iterator_test( git_tree *t; git_iterator *i; const git_index_entry *entry; - int count = 0, count_post_reset = 0; + int error, count = 0, count_post_reset = 0; git_repository *repo = cl_git_sandbox_init(sandbox); cl_assert(t = resolve_commit_oid_to_tree(repo, treeish)); @@ -39,29 +39,30 @@ static void tree_iterator_test( &i, t, GIT_ITERATOR_DONT_IGNORE_CASE, start, end)); /* test loop */ - cl_git_pass(git_iterator_current(&entry, i)); - while (entry != NULL) { + while (!(error = git_iterator_advance(&entry, i))) { + cl_assert(entry); if (expected_values != NULL) cl_assert_equal_s(expected_values[count], entry->path); count++; - cl_git_pass(git_iterator_advance(&entry, i)); } + cl_assert_equal_i(GIT_ITEROVER, error); + cl_assert(!entry); + cl_assert_equal_i(expected_count, count); /* test reset */ cl_git_pass(git_iterator_reset(i, NULL, NULL)); - cl_git_pass(git_iterator_current(&entry, i)); - while (entry != NULL) { + + while (!(error = git_iterator_advance(&entry, i))) { + cl_assert(entry); if (expected_values != NULL) cl_assert_equal_s(expected_values[count_post_reset], entry->path); count_post_reset++; - cl_git_pass(git_iterator_advance(&entry, i)); } - - git_iterator_free(i); - - cl_assert_equal_i(expected_count, count); + cl_assert_equal_i(GIT_ITEROVER, error); + cl_assert(!entry); cl_assert_equal_i(count, count_post_reset); + git_iterator_free(i); git_tree_free(t); } @@ -298,7 +299,7 @@ void test_diff_iterator__tree_special_functions(void) git_iterator *i; const git_index_entry *entry; git_repository *repo = cl_git_sandbox_init("attr"); - int cases = 0; + int error, cases = 0; const char *rootoid = "ce39a97a7fb1fa90bcf5e711249c1e507476ae0e"; t = resolve_commit_oid_to_tree( @@ -307,9 +308,10 @@ void test_diff_iterator__tree_special_functions(void) cl_git_pass(git_iterator_for_tree( &i, t, GIT_ITERATOR_DONT_IGNORE_CASE, NULL, NULL)); - cl_git_pass(git_iterator_current(&entry, i)); - while (entry != NULL) { + while (!(error = git_iterator_advance(&entry, i))) { + cl_assert(entry); + if (strcmp(entry->path, "sub/file") == 0) { cases++; check_tree_entry( @@ -338,11 +340,11 @@ void test_diff_iterator__tree_special_functions(void) "2929de282ce999e95183aedac6451d3384559c4b", rootoid, NULL); } - - cl_git_pass(git_iterator_advance(&entry, i)); } - + cl_assert_equal_i(GIT_ITEROVER, error); + cl_assert(!entry); cl_assert_equal_i(4, cases); + git_iterator_free(i); git_tree_free(t); } @@ -360,14 +362,15 @@ static void index_iterator_test( git_index *index; git_iterator *i; const git_index_entry *entry; - int count = 0; + int error, count = 0; git_repository *repo = cl_git_sandbox_init(sandbox); cl_git_pass(git_repository_index(&index, repo)); cl_git_pass(git_iterator_for_index(&i, index, 0, start, end)); - cl_git_pass(git_iterator_current(&entry, i)); - while (entry != NULL) { + while (!(error = git_iterator_advance(&entry, i))) { + cl_assert(entry); + if (expected_names != NULL) cl_assert_equal_s(expected_names[count], entry->path); @@ -378,13 +381,14 @@ static void index_iterator_test( } count++; - cl_git_pass(git_iterator_advance(&entry, i)); } + cl_assert_equal_i(GIT_ITEROVER, error); + cl_assert(!entry); + cl_assert_equal_i(expected_count, count); + git_iterator_free(i); git_index_free(index); - - cl_assert_equal_i(expected_count, count); } static const char *expected_index_0[] = { @@ -535,12 +539,15 @@ static void workdir_iterator_test( { git_iterator *i; const git_index_entry *entry; - int count = 0, count_all = 0, count_all_post_reset = 0; + int error, count = 0, count_all = 0, count_all_post_reset = 0; git_repository *repo = cl_git_sandbox_init(sandbox); cl_git_pass(git_iterator_for_workdir( &i, repo, GIT_ITERATOR_DONT_AUTOEXPAND, start, end)); - cl_git_pass(git_iterator_current(&entry, i)); + + error = git_iterator_current(&entry, i); + cl_assert((error == 0 && entry != NULL) || + (error == GIT_ITEROVER && entry == NULL)); while (entry != NULL) { int ignored = git_iterator_current_is_ignored(i); @@ -560,29 +567,39 @@ static void workdir_iterator_test( count++; count_all++; - cl_git_pass(git_iterator_advance(&entry, i)); + error = git_iterator_advance(&entry, i); + + cl_assert((error == 0 && entry != NULL) || + (error == GIT_ITEROVER && entry == NULL)); } + cl_assert_equal_i(expected_count, count); + cl_assert_equal_i(expected_count + expected_ignores, count_all); + cl_git_pass(git_iterator_reset(i, NULL, NULL)); - cl_git_pass(git_iterator_current(&entry, i)); + + error = git_iterator_current(&entry, i); + cl_assert((error == 0 && entry != NULL) || + (error == GIT_ITEROVER && entry == NULL)); while (entry != NULL) { if (S_ISDIR(entry->mode)) { cl_git_pass(git_iterator_advance_into(&entry, i)); continue; } + if (expected_names != NULL) cl_assert_equal_s( expected_names[count_all_post_reset], entry->path); count_all_post_reset++; - cl_git_pass(git_iterator_advance(&entry, i)); + + error = git_iterator_advance(&entry, i); + cl_assert(error == 0 || error == GIT_ITEROVER); } - git_iterator_free(i); - - cl_assert_equal_i(expected_count, count); - cl_assert_equal_i(expected_count + expected_ignores, count_all); cl_assert_equal_i(count_all, count_all_post_reset); + + git_iterator_free(i); } void test_diff_iterator__workdir_0(void) @@ -752,8 +769,10 @@ void test_diff_iterator__workdir_builtin_ignores(void) { /* it is possible to advance "into" a submodule */ cl_git_pass(git_iterator_advance_into(&entry, i)); - } else - cl_git_pass(git_iterator_advance(&entry, i)); + } else { + int error = git_iterator_advance(&entry, i); + cl_assert(!error || error == GIT_ITEROVER); + } } cl_assert(expected[idx].path == NULL); @@ -766,7 +785,7 @@ static void check_wd_first_through_third_range( { git_iterator *i; const git_index_entry *entry; - int idx; + int error, idx; static const char *expected[] = { "FIRST", "second", "THIRD", NULL }; cl_git_pass(git_iterator_for_workdir( @@ -776,7 +795,8 @@ static void check_wd_first_through_third_range( for (idx = 0; entry != NULL; ++idx) { cl_assert_equal_s(expected[idx], entry->path); - cl_git_pass(git_iterator_advance(&entry, i)); + error = git_iterator_advance(&entry, i); + cl_assert(!error || error == GIT_ITEROVER); } cl_assert(expected[idx] == NULL); @@ -814,8 +834,7 @@ static void check_tree_range( { git_tree *head; git_iterator *i; - const git_index_entry *entry; - int count; + int error, count; cl_git_pass(git_repository_head_tree(&head, repo)); @@ -824,13 +843,10 @@ static void check_tree_range( ignore_case ? GIT_ITERATOR_IGNORE_CASE : GIT_ITERATOR_DONT_IGNORE_CASE, start, end)); - cl_git_pass(git_iterator_current(&entry, i)); - - for (count = 0; entry != NULL; ) { - ++count; - cl_git_pass(git_iterator_advance(&entry, i)); - } + for (count = 0; !(error = git_iterator_advance(NULL, i)); ++count) + /* count em up */; + cl_assert_equal_i(GIT_ITEROVER, error); cl_assert_equal_i(expected_count, count); git_iterator_free(i); @@ -872,8 +888,7 @@ static void check_index_range( { git_index *index; git_iterator *i; - const git_index_entry *entry; - int count, caps; + int error, count, caps; bool is_ignoring_case; cl_git_pass(git_repository_index(&index, repo)); @@ -888,13 +903,10 @@ static void check_index_range( cl_assert(git_iterator_ignore_case(i) == ignore_case); - cl_git_pass(git_iterator_current(&entry, i)); - - for (count = 0; entry != NULL; ) { - ++count; - cl_git_pass(git_iterator_advance(&entry, i)); - } + for (count = 0; !(error = git_iterator_advance(NULL, i)); ++count) + /* count em up */; + cl_assert_equal_i(GIT_ITEROVER, error); cl_assert_equal_i(expected_count, count); git_iterator_free(i); diff --git a/tests-clar/repo/iterator.c b/tests-clar/repo/iterator.c index ab460735c..11a7d2a23 100644 --- a/tests-clar/repo/iterator.c +++ b/tests-clar/repo/iterator.c @@ -31,12 +31,11 @@ static void expect_iterator_items( if (expected_flat < 0) { v = true; expected_flat = -expected_flat; } if (expected_total < 0) { v = true; expected_total = -expected_total; } - count = 0; - cl_git_pass(git_iterator_current(&entry, i)); - if (v) fprintf(stderr, "== %s ==\n", no_trees ? "notrees" : "trees"); - while (entry != NULL) { + count = 0; + + while (!git_iterator_advance(&entry, i)) { if (v) fprintf(stderr, " %s %07o\n", entry->path, (int)entry->mode); if (no_trees) @@ -54,8 +53,6 @@ static void expect_iterator_items( cl_assert(entry->mode != GIT_FILEMODE_TREE); } - cl_git_pass(git_iterator_advance(&entry, i)); - if (++count > expected_flat) break; } @@ -93,10 +90,14 @@ static void expect_iterator_items( /* could return NOTFOUND if directory is empty */ cl_assert(!error || error == GIT_ENOTFOUND); - if (error == GIT_ENOTFOUND) - cl_git_pass(git_iterator_advance(&entry, i)); - } else - cl_git_pass(git_iterator_advance(&entry, i)); + if (error == GIT_ENOTFOUND) { + error = git_iterator_advance(&entry, i); + cl_assert(!error || error == GIT_ITEROVER); + } + } else { + error = git_iterator_advance(&entry, i); + cl_assert(!error || error == GIT_ITEROVER); + } if (++count > expected_total) break; diff --git a/tests-clar/submodule/status.c b/tests-clar/submodule/status.c index 88f388052..39c83a0b7 100644 --- a/tests-clar/submodule/status.c +++ b/tests-clar/submodule/status.c @@ -370,12 +370,9 @@ void test_submodule_status__iterator(void) cl_git_pass(git_iterator_for_workdir(&iter, g_repo, GIT_ITERATOR_IGNORE_CASE | GIT_ITERATOR_INCLUDE_TREES, NULL, NULL)); - cl_git_pass(git_iterator_current(&entry, iter)); - for (i = 0; entry; ++i) { + for (i = 0; !git_iterator_advance(&entry, iter); ++i) cl_assert_equal_s(expected[i], entry->path); - cl_git_pass(git_iterator_advance(&entry, iter)); - } git_iterator_free(iter);