Commit efd5a4e1 by Vicent Martí

Merge pull request #1627 from arrbee/iterator-api-improvements

Make iterators use GIT_ITEROVER & smart advance
parents 1ed356dc cee695ae
......@@ -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;
......
......@@ -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
......
......@@ -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);
......
......@@ -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;
}
......
......@@ -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;
}
......@@ -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);
......
......@@ -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);
......
......@@ -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));
}
git_iterator_free(i);
error = git_iterator_advance(&entry, i);
cl_assert(error == 0 || error == GIT_ITEROVER);
}
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);
......
......@@ -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;
......
......@@ -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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment