From 3b4c401a38ce912d5be8c9bf4ab1c4912a4f08bd Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 10 Feb 2014 13:20:08 -0800 Subject: [PATCH] Decouple index iterator sort from index This makes the index iterator honor the GIT_ITERATOR_IGNORE_CASE and GIT_ITERATOR_DONT_IGNORE_CASE flags without modifying the index data itself. To take advantage of this, I had to export a number of the internal index entry comparison functions. I also wrote some new tests to exercise the capability. --- src/diff.c | 79 +++++++++++++++++++++---------------------- src/index.c | 73 +++++++++++++++------------------------ src/index.h | 11 ++++-- src/iterator.c | 14 +++++--- src/merge.c | 2 +- src/util.c | 8 +++-- src/util.h | 1 + src/vector.c | 3 +- tests/diff/iterator.c | 58 +++++++++++++++++++++++++++---- 9 files changed, 142 insertions(+), 107 deletions(-) diff --git a/src/diff.c b/src/diff.c index cb05a5faf..0d1aed4ad 100644 --- a/src/diff.c +++ b/src/diff.c @@ -318,6 +318,31 @@ static const char *diff_mnemonic_prefix( return pfx; } +static void diff_set_ignore_case(git_diff *diff, bool ignore_case) +{ + if (!ignore_case) { + diff->opts.flags &= ~GIT_DIFF_IGNORE_CASE; + + diff->strcomp = git__strcmp; + diff->strncomp = git__strncmp; + diff->pfxcomp = git__prefixcmp; + diff->entrycomp = git_index_entry_cmp; + + git_vector_set_cmp(&diff->deltas, git_diff_delta__cmp); + } else { + diff->opts.flags |= GIT_DIFF_IGNORE_CASE; + + diff->strcomp = git__strcasecmp; + diff->strncomp = git__strncasecmp; + diff->pfxcomp = git__prefixcmp_icase; + diff->entrycomp = git_index_entry_icmp; + + git_vector_set_cmp(&diff->deltas, git_diff_delta__casecmp); + } + + git_vector_sort(&diff->deltas); +} + static git_diff *diff_list_alloc( git_repository *repo, git_iterator *old_iter, @@ -344,24 +369,10 @@ static git_diff *diff_list_alloc( /* Use case-insensitive compare if either iterator has * the ignore_case bit set */ - if (!git_iterator_ignore_case(old_iter) && - !git_iterator_ignore_case(new_iter)) { - diff->opts.flags &= ~GIT_DIFF_IGNORE_CASE; - - diff->strcomp = git__strcmp; - diff->strncomp = git__strncmp; - diff->pfxcomp = git__prefixcmp; - diff->entrycomp = git_index_entry__cmp; - } else { - diff->opts.flags |= GIT_DIFF_IGNORE_CASE; - - diff->strcomp = git__strcasecmp; - diff->strncomp = git__strncasecmp; - diff->pfxcomp = git__prefixcmp_icase; - diff->entrycomp = git_index_entry__cmp_icase; - - git_vector_set_cmp(&diff->deltas, git_diff_delta__casecmp); - } + diff_set_ignore_case( + diff, + git_iterator_ignore_case(old_iter) || + git_iterator_ignore_case(new_iter)); return diff; } @@ -1183,39 +1194,25 @@ int git_diff_tree_to_index( const git_diff_options *opts) { int error = 0; - bool reset_index_ignore_case = false; + bool index_ignore_case = false; assert(diff && repo); if (!index && (error = diff_load_index(&index, repo)) < 0) return error; - if (index->ignore_case) { - git_index__set_ignore_case(index, false); - reset_index_ignore_case = true; - } + index_ignore_case = index->ignore_case; DIFF_FROM_ITERATORS( - git_iterator_for_tree(&a, old_tree, 0, pfx, pfx), - git_iterator_for_index(&b, index, 0, pfx, pfx) + git_iterator_for_tree( + &a, old_tree, GIT_ITERATOR_DONT_IGNORE_CASE, pfx, pfx), + git_iterator_for_index( + &b, index, GIT_ITERATOR_DONT_IGNORE_CASE, pfx, pfx) ); - if (reset_index_ignore_case) { - git_index__set_ignore_case(index, true); - - if (!error) { - git_diff *d = *diff; - - d->opts.flags |= GIT_DIFF_IGNORE_CASE; - d->strcomp = git__strcasecmp; - d->strncomp = git__strncasecmp; - d->pfxcomp = git__prefixcmp_icase; - d->entrycomp = git_index_entry__cmp_icase; - - git_vector_set_cmp(&d->deltas, git_diff_delta__casecmp); - git_vector_sort(&d->deltas); - } - } + /* if index is in case-insensitive order, re-sort deltas to match */ + if (!error && index_ignore_case) + diff_set_ignore_case(*diff, true); return error; } diff --git a/src/index.c b/src/index.c index f28cbef27..d733e4e48 100644 --- a/src/index.c +++ b/src/index.c @@ -106,7 +106,7 @@ static int write_index(git_index *index, git_filebuf *file); static void index_entry_free(git_index_entry *entry); static void index_entry_reuc_free(git_index_reuc_entry *reuc); -static int index_srch(const void *key, const void *array_member) +int git_index_entry_srch(const void *key, const void *array_member) { const struct entry_srch_key *srch_key = key; const git_index_entry *entry = array_member; @@ -131,7 +131,7 @@ static int index_srch(const void *key, const void *array_member) return 0; } -static int index_isrch(const void *key, const void *array_member) +int git_index_entry_isrch(const void *key, const void *array_member) { const struct entry_srch_key *srch_key = key; const git_index_entry *entry = array_member; @@ -157,31 +157,21 @@ static int index_isrch(const void *key, const void *array_member) return 0; } -static int index_cmp_path(const void *a, const void *b) -{ - return strcmp((const char *)a, (const char *)b); -} - -static int index_icmp_path(const void *a, const void *b) -{ - return strcasecmp((const char *)a, (const char *)b); -} - -static int index_srch_path(const void *path, const void *array_member) +static int index_entry_srch_path(const void *path, const void *array_member) { const git_index_entry *entry = array_member; return strcmp((const char *)path, entry->path); } -static int index_isrch_path(const void *path, const void *array_member) +static int index_entry_isrch_path(const void *path, const void *array_member) { const git_index_entry *entry = array_member; return strcasecmp((const char *)path, entry->path); } -static int index_cmp(const void *a, const void *b) +int git_index_entry_cmp(const void *a, const void *b) { int diff; const git_index_entry *entry_a = a; @@ -195,7 +185,7 @@ static int index_cmp(const void *a, const void *b) return diff; } -static int index_icmp(const void *a, const void *b) +int git_index_entry_icmp(const void *a, const void *b) { int diff; const git_index_entry *entry_a = a; @@ -351,15 +341,22 @@ void git_index__set_ignore_case(git_index *index, bool ignore_case) { index->ignore_case = ignore_case; - index->entries_cmp_path = ignore_case ? index_icmp_path : index_cmp_path; - index->entries_search = ignore_case ? index_isrch : index_srch; - index->entries_search_path = ignore_case ? index_isrch_path : index_srch_path; + if (ignore_case) { + index->entries_cmp_path = git__strcasecmp_cb; + index->entries_search = git_index_entry_isrch; + index->entries_search_path = index_entry_isrch_path; + index->reuc_search = reuc_isrch; + } else { + index->entries_cmp_path = git__strcmp_cb; + index->entries_search = git_index_entry_srch; + index->entries_search_path = index_entry_srch_path; + index->reuc_search = reuc_srch; + } - git_vector_set_cmp(&index->entries, ignore_case ? index_icmp : index_cmp); + git_vector_set_cmp(&index->entries, + ignore_case ? git_index_entry_icmp : git_index_entry_cmp); index_sort_if_needed(index, true); - index->reuc_search = ignore_case ? reuc_isrch : reuc_srch; - git_vector_set_cmp(&index->reuc, ignore_case ? reuc_icmp : reuc_cmp); git_vector_sort(&index->reuc); } @@ -390,15 +387,15 @@ int git_index_open(git_index **index_out, const char *index_path) index->on_disk = 1; } - if (git_vector_init(&index->entries, 32, index_cmp) < 0 || + if (git_vector_init(&index->entries, 32, git_index_entry_cmp) < 0 || git_vector_init(&index->names, 32, conflict_name_cmp) < 0 || git_vector_init(&index->reuc, 32, reuc_cmp) < 0 || - git_vector_init(&index->deleted, 2, index_cmp) < 0) + git_vector_init(&index->deleted, 2, git_index_entry_cmp) < 0) goto fail; - index->entries_cmp_path = index_cmp_path; - index->entries_search = index_srch; - index->entries_search_path = index_srch_path; + index->entries_cmp_path = git__strcmp_cb; + index->entries_search = git_index_entry_srch; + index->entries_search_path = index_entry_srch_path; index->reuc_search = reuc_srch; if (index_path != NULL && (error = git_index_read(index, true)) < 0) @@ -727,22 +724,6 @@ void git_index_entry__init_from_stat( entry->file_size = st->st_size; } -int git_index_entry__cmp(const void *a, const void *b) -{ - const git_index_entry *entry_a = a; - const git_index_entry *entry_b = b; - - return strcmp(entry_a->path, entry_b->path); -} - -int git_index_entry__cmp_icase(const void *a, const void *b) -{ - const git_index_entry *entry_a = a; - const git_index_entry *entry_b = b; - - return strcasecmp(entry_a->path, entry_b->path); -} - static int index_entry_init( git_index_entry **entry_out, git_index *index, const char *rel_path) { @@ -1129,14 +1110,14 @@ int git_index_remove_directory(git_index *index, const char *dir, int stage) } int git_index__find_in_entries( - size_t *out, git_vector *entries, git_vector_cmp entry_cmp, + size_t *out, git_vector *entries, git_vector_cmp entry_srch, const char *path, size_t path_len, int stage) { struct entry_srch_key srch_key; srch_key.path = path; srch_key.path_len = !path_len ? strlen(path) : path_len; srch_key.stage = stage; - return git_vector_bsearch2(out, entries, entry_cmp, &srch_key); + return git_vector_bsearch2(out, entries, entry_srch, &srch_key); } int git_index__find( @@ -2040,7 +2021,7 @@ static int write_entries(git_index *index, git_filebuf *file) /* If index->entries is sorted case-insensitively, then we need * to re-sort it case-sensitively before writing */ if (index->ignore_case) { - git_vector_dup(&case_sorted, &index->entries, index_cmp); + git_vector_dup(&case_sorted, &index->entries, git_index_entry_cmp); git_vector_sort(&case_sorted); entries = &case_sorted; } else { diff --git a/src/index.h b/src/index.h index c2a932218..cb4425885 100644 --- a/src/index.h +++ b/src/index.h @@ -53,8 +53,13 @@ struct git_index_conflict_iterator { extern void git_index_entry__init_from_stat( git_index_entry *entry, struct stat *st, bool trust_mode); -extern int git_index_entry__cmp(const void *a, const void *b); -extern int git_index_entry__cmp_icase(const void *a, const void *b); +/* Index entry comparison functions for array sorting */ +extern int git_index_entry_cmp(const void *a, const void *b); +extern int git_index_entry_icmp(const void *a, const void *b); + +/* Index entry search functions for search using a search spec */ +extern int git_index_entry_srch(const void *a, const void *b); +extern int git_index_entry_isrch(const void *a, const void *b); /* Search index for `path`, returning GIT_ENOTFOUND if it does not exist. * `at_pos` is set to the position where it is or would be inserted. @@ -83,7 +88,7 @@ extern void git_index__release_snapshot(git_index *index); /* Allow searching in a snapshot; entries must already be sorted! */ extern int git_index__find_in_entries( size_t *at_pos, - git_vector *entries, git_vector_cmp entry_cmp, + git_vector *entries, git_vector_cmp entry_srch, const char *path, size_t path_len, int stage); #endif diff --git a/src/iterator.c b/src/iterator.c index a84f4d3db..53ef278d1 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -868,16 +868,20 @@ int git_iterator_for_index( return error; } ii->index = index; - ii->entry_srch = index->entries_search; ITERATOR_BASE_INIT(ii, index, INDEX, git_index_owner(index)); - if (index->ignore_case) { - ii->base.flags |= GIT_ITERATOR_IGNORE_CASE; - ii->base.prefixcomp = git__prefixcmp_icase; + if ((error = iterator__update_ignore_case((git_iterator *)ii, flags)) < 0) { + git_iterator_free((git_iterator *)ii); + return error; } - /* TODO: resort entries to match desired ignore_case behavior */ + ii->entry_srch = iterator__ignore_case(ii) ? + git_index_entry_isrch : git_index_entry_srch; + + git_vector_set_cmp(&ii->entries, iterator__ignore_case(ii) ? + git_index_entry_icmp : git_index_entry_cmp); + git_vector_sort(&ii->entries); git_buf_init(&ii->partial, 0); ii->tree_entry.mode = GIT_FILEMODE_TREE; diff --git a/src/merge.c b/src/merge.c index 9c4a07b58..68105d483 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1332,7 +1332,7 @@ int git_merge_diff_list__find_differences( { git_iterator *iterators[3] = {0}; const git_index_entry *items[3] = {0}, *best_cur_item, *cur_items[3]; - git_vector_cmp entry_compare = git_index_entry__cmp; + git_vector_cmp entry_compare = git_index_entry_cmp; struct merge_diff_df_data df_data = {0}; int cur_item_modified; size_t i, j; diff --git a/src/util.c b/src/util.c index 3767b890c..39858254f 100644 --- a/src/util.c +++ b/src/util.c @@ -542,10 +542,12 @@ int git__bsearch_r( */ int git__strcmp_cb(const void *a, const void *b) { - const char *stra = (const char *)a; - const char *strb = (const char *)b; + return strcmp((const char *)a, (const char *)b); +} - return strcmp(stra, strb); +int git__strcasecmp_cb(const void *a, const void *b) +{ + return strcasecmp((const char *)a, (const char *)b); } int git__parse_bool(int *out, const char *value) diff --git a/src/util.h b/src/util.h index 5c2c563d6..d94463c65 100644 --- a/src/util.h +++ b/src/util.h @@ -196,6 +196,7 @@ extern int git__bsearch_r( size_t *position); extern int git__strcmp_cb(const void *a, const void *b); +extern int git__strcasecmp_cb(const void *a, const void *b); extern int git__strcmp(const char *a, const char *b); extern int git__strcasecmp(const char *a, const char *b); diff --git a/src/vector.c b/src/vector.c index 9f7eed5a3..c769b696a 100644 --- a/src/vector.c +++ b/src/vector.c @@ -177,7 +177,8 @@ void git_vector_sort(git_vector *v) if (git_vector_is_sorted(v) || !v->_cmp) return; - git__tsort(v->contents, v->length, v->_cmp); + if (v->length > 1) + git__tsort(v->contents, v->length, v->_cmp); git_vector_set_sorted(v, 1); } diff --git a/tests/diff/iterator.c b/tests/diff/iterator.c index 891d8a6e5..19a9a0077 100644 --- a/tests/diff/iterator.c +++ b/tests/diff/iterator.c @@ -355,6 +355,7 @@ static void index_iterator_test( const char *sandbox, const char *start, const char *end, + git_iterator_flag_t flags, int expected_count, const char **expected_names, const char **expected_oids) @@ -364,9 +365,12 @@ static void index_iterator_test( const git_index_entry *entry; int error, count = 0; git_repository *repo = cl_git_sandbox_init(sandbox); + unsigned int caps; cl_git_pass(git_repository_index(&index, repo)); - cl_git_pass(git_iterator_for_index(&i, index, 0, start, end)); + caps = git_index_caps(index); + + cl_git_pass(git_iterator_for_index(&i, index, flags, start, end)); while (!(error = git_iterator_advance(&entry, i))) { cl_assert(entry); @@ -388,6 +392,8 @@ static void index_iterator_test( cl_assert_equal_i(expected_count, count); git_iterator_free(i); + + cl_assert(caps == git_index_caps(index)); git_index_free(index); } @@ -446,7 +452,8 @@ static const char *expected_index_oids_0[] = { void test_diff_iterator__index_0(void) { index_iterator_test( - "attr", NULL, NULL, 23, expected_index_0, expected_index_oids_0); + "attr", NULL, NULL, 0, ARRAY_SIZE(expected_index_0), + expected_index_0, expected_index_oids_0); } static const char *expected_index_range[] = { @@ -466,25 +473,26 @@ static const char *expected_index_oids_range[] = { void test_diff_iterator__index_range(void) { index_iterator_test( - "attr", "root", "root", 4, expected_index_range, expected_index_oids_range); + "attr", "root", "root", 0, ARRAY_SIZE(expected_index_range), + expected_index_range, expected_index_oids_range); } void test_diff_iterator__index_range_empty_0(void) { index_iterator_test( - "attr", "empty", "empty", 0, NULL, NULL); + "attr", "empty", "empty", 0, 0, NULL, NULL); } void test_diff_iterator__index_range_empty_1(void) { index_iterator_test( - "attr", "z_empty_after", NULL, 0, NULL, NULL); + "attr", "z_empty_after", NULL, 0, 0, NULL, NULL); } void test_diff_iterator__index_range_empty_2(void) { index_iterator_test( - "attr", NULL, ".aaa_empty_before", 0, NULL, NULL); + "attr", NULL, ".aaa_empty_before", 0, 0, NULL, NULL); } static const char *expected_index_1[] = { @@ -522,9 +530,45 @@ static const char* expected_index_oids_1[] = { void test_diff_iterator__index_1(void) { index_iterator_test( - "status", NULL, NULL, 13, expected_index_1, expected_index_oids_1); + "status", NULL, NULL, 0, ARRAY_SIZE(expected_index_1), + expected_index_1, expected_index_oids_1); } +static const char *expected_index_cs[] = { + "B", "D", "F", "H", "J", "L/1", "L/B", "L/D", "L/a", "L/c", + "a", "c", "e", "g", "i", "k/1", "k/B", "k/D", "k/a", "k/c", +}; + +static const char *expected_index_ci[] = { + "a", "B", "c", "D", "e", "F", "g", "H", "i", "J", + "k/1", "k/a", "k/B", "k/c", "k/D", "L/1", "L/a", "L/B", "L/c", "L/D", +}; + +void test_diff_iterator__index_case_folding(void) +{ + git_buf path = GIT_BUF_INIT; + int fs_is_ci = 0; + + cl_git_pass(git_buf_joinpath(&path, cl_fixture("icase"), ".gitted/CoNfIg")); + fs_is_ci = git_path_exists(path.ptr); + git_buf_free(&path); + + index_iterator_test( + "icase", NULL, NULL, 0, ARRAY_SIZE(expected_index_cs), + fs_is_ci ? expected_index_ci : expected_index_cs, NULL); + + cl_git_sandbox_cleanup(); + + index_iterator_test( + "icase", NULL, NULL, GIT_ITERATOR_IGNORE_CASE, + ARRAY_SIZE(expected_index_ci), expected_index_ci, NULL); + + cl_git_sandbox_cleanup(); + + index_iterator_test( + "icase", NULL, NULL, GIT_ITERATOR_DONT_IGNORE_CASE, + ARRAY_SIZE(expected_index_cs), expected_index_cs, NULL); +} /* -- WORKDIR ITERATOR TESTS -- */