From 2e0391f4f1d088a9d9748a1b3bcab8ac576d63de Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 2 Apr 2016 11:33:00 -0700 Subject: diff: test submodules are found with trailing `/` Test that submodules are found when the are included in a pathspec but have a trailing slash. --- tests/diff/submodules.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/diff/submodules.c b/tests/diff/submodules.c index 08682cd4b..eebfef3a2 100644 --- a/tests/diff/submodules.c +++ b/tests/diff/submodules.c @@ -493,3 +493,50 @@ void test_diff_submodules__skips_empty_includes_used(void) cl_assert_equal_i(1, exp.file_status[GIT_DELTA_UNTRACKED]); git_diff_free(diff); } + +static void ensure_submodules_found( + git_repository *repo, + const char **paths, + size_t cnt) +{ + git_diff *diff = NULL; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + const git_diff_delta *delta; + size_t i, pathlen; + + opts.pathspec.strings = (char **)paths; + opts.pathspec.count = cnt; + + git_diff_index_to_workdir(&diff, repo, NULL, &opts); + + cl_assert_equal_i(cnt, git_diff_num_deltas(diff)); + + for (i = 0; i < cnt; i++) { + delta = git_diff_get_delta(diff, i); + + /* ensure that the given path is returned w/o trailing slashes. */ + pathlen = strlen(opts.pathspec.strings[i]); + + while (pathlen && opts.pathspec.strings[i][pathlen - 1] == '/') + pathlen--; + + cl_assert_equal_strn(opts.pathspec.strings[i], delta->new_file.path, pathlen); + } + + git_diff_free(diff); +} + +void test_diff_submodules__can_be_identified_by_trailing_slash_in_pathspec(void) +{ + const char *one_path_without_slash[] = { "sm_changed_head" }; + const char *one_path_with_slash[] = { "sm_changed_head/" }; + const char *many_paths_without_slashes[] = { "sm_changed_head", "sm_changed_index" }; + const char *many_paths_with_slashes[] = { "sm_changed_head/", "sm_changed_index/" }; + + g_repo = setup_fixture_submod2(); + + ensure_submodules_found(g_repo, one_path_without_slash, ARRAY_SIZE(one_path_without_slash)); + ensure_submodules_found(g_repo, one_path_with_slash, ARRAY_SIZE(one_path_with_slash)); + ensure_submodules_found(g_repo, many_paths_without_slashes, ARRAY_SIZE(many_paths_without_slashes)); + ensure_submodules_found(g_repo, many_paths_with_slashes, ARRAY_SIZE(many_paths_with_slashes)); +} -- cgit v1.2.1 From d47f7e1c15560e327359e69caf1bc0c41538c676 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 2 Apr 2016 13:03:09 -0700 Subject: iterator: support trailing `/` in start for submod Allow callers to specify a start path with a trailing slash to match a submodule, instead of just a directory. This is for some legacy behavior that's sort of dumb, but there it is. --- src/iterator.c | 36 ++++++++++++++++++++++++++---------- tests/iterator/workdir.c | 18 ++++++++++++------ 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index 60b33cc70..ec44aac4c 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -169,7 +169,8 @@ static void iterator_clear(git_iterator *iter) iter->flags &= ~GIT_ITERATOR_FIRST_ACCESS; } -GIT_INLINE(bool) iterator_has_started(git_iterator *iter, const char *path) +GIT_INLINE(bool) iterator_has_started( + git_iterator *iter, const char *path, bool is_submodule) { size_t path_len; @@ -181,19 +182,32 @@ GIT_INLINE(bool) iterator_has_started(git_iterator *iter, const char *path) */ iter->started = (iter->prefixcomp(path, iter->start) >= 0); + if (iter->started) + return true; + + path_len = strlen(path); + + /* if, however, we are a submodule, then we support `start` being + * suffixed with a `/` for crazy legacy reasons. match `submod` + * with a start path of `submod/`. + */ + if (is_submodule && iter->start_len && path_len == iter->start_len - 1 && + iter->start[iter->start_len-1] == '/') + return true; + /* if, however, our current path is a directory, and our starting path * is _beneath_ that directory, then recurse into the directory (even * though we have not yet "started") */ - if (!iter->started && - (path_len = strlen(path)) > 0 && path[path_len-1] == '/' && + if (path_len > 0 && path[path_len-1] == '/' && iter->strncomp(path, iter->start, path_len) == 0) return true; - return iter->started; + return false; } -GIT_INLINE(bool) iterator_has_ended(git_iterator *iter, const char *path) +GIT_INLINE(bool) iterator_has_ended( + git_iterator *iter, const char *path, bool is_submodule) { if (iter->end == NULL) return false; @@ -779,11 +793,11 @@ static int tree_iterator_advance(const git_index_entry **out, git_iterator *i) break; /* if this path is before our start, advance over this entry */ - if (!iterator_has_started(&iter->base, iter->entry_path.ptr)) + if (!iterator_has_started(&iter->base, iter->entry_path.ptr, false)) continue; /* if this path is after our end, stop */ - if (iterator_has_ended(&iter->base, iter->entry_path.ptr)) { + if (iterator_has_ended(&iter->base, iter->entry_path.ptr, false)) { error = GIT_ITEROVER; break; } @@ -1400,7 +1414,7 @@ static int filesystem_iterator_frame_push( } /* Ensure that the pathlist entry lines up with what we expected */ - if (dir_expected && !S_ISDIR(statbuf.st_mode)) + else if (dir_expected) continue; entry = filesystem_iterator_entry_init(new_frame, @@ -1995,6 +2009,7 @@ static int index_iterator_advance( { index_iterator *iter = (index_iterator *)i; const git_index_entry *entry = NULL; + bool is_submodule; int error = 0; iter->base.flags |= GIT_ITERATOR_FIRST_ACCESS; @@ -2012,13 +2027,14 @@ static int index_iterator_advance( } entry = iter->entries.contents[iter->next_idx]; + is_submodule = S_ISGITLINK(entry->mode); - if (!iterator_has_started(&iter->base, entry->path)) { + if (!iterator_has_started(&iter->base, entry->path, is_submodule)) { iter->next_idx++; continue; } - if (iterator_has_ended(&iter->base, entry->path)) { + if (iterator_has_ended(&iter->base, entry->path, is_submodule)) { error = GIT_ITEROVER; break; } diff --git a/tests/iterator/workdir.c b/tests/iterator/workdir.c index fc7771c20..c8f795a0d 100644 --- a/tests/iterator/workdir.c +++ b/tests/iterator/workdir.c @@ -1197,8 +1197,11 @@ void test_iterator_workdir__bounded_submodules(void) git_iterator_free(i); } - /* Test that a submodule never matches when suffixed with a '/' */ + /* Test that a submodule still matches when suffixed with a '/' */ { + const char *expected[] = { "sm_changed_head" }; + size_t expected_len = 1; + git_vector_clear(&filelist); cl_git_pass(git_vector_insert(&filelist, "sm_changed_head/")); @@ -1207,7 +1210,7 @@ void test_iterator_workdir__bounded_submodules(void) i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE; cl_git_pass(git_iterator_for_workdir(&i, g_repo, index, head, &i_opts)); - cl_git_fail_with(GIT_ITEROVER, git_iterator_advance(NULL, i)); + expect_iterator_items(i, expected_len, expected, expected_len, expected); git_iterator_free(i); } @@ -1227,16 +1230,19 @@ void test_iterator_workdir__bounded_submodules(void) git_iterator_free(i); } - /* Test that start and end do not allow '/' suffixes of submodules */ + /* Test that start and end allow '/' suffixes of submodules */ { - i_opts.start = "sm_changed_head/"; - i_opts.end = "sm_changed_head/"; + const char *expected[] = { "sm_changed_head", "sm_changed_index" }; + size_t expected_len = 2; + + i_opts.start = "sm_changed_head"; + i_opts.end = "sm_changed_index"; i_opts.pathlist.strings = NULL; i_opts.pathlist.count = 0; i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE; cl_git_pass(git_iterator_for_workdir(&i, g_repo, index, head, &i_opts)); - cl_git_fail_with(GIT_ITEROVER, git_iterator_advance(NULL, i)); + expect_iterator_items(i, expected_len, expected, expected_len, expected); git_iterator_free(i); } -- cgit v1.2.1