Commit 381caf56 by Carlos Martín Nieto

Merge pull request #3724 from ethomson/submodule_start_supports_silly_slashes

iterator/diff: allow trailing `/` on start/end paths to match submodules
parents 7018e3b7 d47f7e1c
...@@ -169,7 +169,8 @@ static void iterator_clear(git_iterator *iter) ...@@ -169,7 +169,8 @@ static void iterator_clear(git_iterator *iter)
iter->flags &= ~GIT_ITERATOR_FIRST_ACCESS; 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; size_t path_len;
...@@ -181,19 +182,32 @@ GIT_INLINE(bool) iterator_has_started(git_iterator *iter, const char *path) ...@@ -181,19 +182,32 @@ GIT_INLINE(bool) iterator_has_started(git_iterator *iter, const char *path)
*/ */
iter->started = (iter->prefixcomp(path, iter->start) >= 0); 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 /* if, however, our current path is a directory, and our starting path
* is _beneath_ that directory, then recurse into the directory (even * is _beneath_ that directory, then recurse into the directory (even
* though we have not yet "started") * though we have not yet "started")
*/ */
if (!iter->started && if (path_len > 0 && path[path_len-1] == '/' &&
(path_len = strlen(path)) > 0 && path[path_len-1] == '/' &&
iter->strncomp(path, iter->start, path_len) == 0) iter->strncomp(path, iter->start, path_len) == 0)
return true; 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) if (iter->end == NULL)
return false; return false;
...@@ -779,11 +793,11 @@ static int tree_iterator_advance(const git_index_entry **out, git_iterator *i) ...@@ -779,11 +793,11 @@ static int tree_iterator_advance(const git_index_entry **out, git_iterator *i)
break; break;
/* if this path is before our start, advance over this entry */ /* 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; continue;
/* if this path is after our end, stop */ /* 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; error = GIT_ITEROVER;
break; break;
} }
...@@ -1400,7 +1414,7 @@ static int filesystem_iterator_frame_push( ...@@ -1400,7 +1414,7 @@ static int filesystem_iterator_frame_push(
} }
/* Ensure that the pathlist entry lines up with what we expected */ /* Ensure that the pathlist entry lines up with what we expected */
if (dir_expected && !S_ISDIR(statbuf.st_mode)) else if (dir_expected)
continue; continue;
entry = filesystem_iterator_entry_init(new_frame, entry = filesystem_iterator_entry_init(new_frame,
...@@ -1995,6 +2009,7 @@ static int index_iterator_advance( ...@@ -1995,6 +2009,7 @@ static int index_iterator_advance(
{ {
index_iterator *iter = (index_iterator *)i; index_iterator *iter = (index_iterator *)i;
const git_index_entry *entry = NULL; const git_index_entry *entry = NULL;
bool is_submodule;
int error = 0; int error = 0;
iter->base.flags |= GIT_ITERATOR_FIRST_ACCESS; iter->base.flags |= GIT_ITERATOR_FIRST_ACCESS;
...@@ -2012,13 +2027,14 @@ static int index_iterator_advance( ...@@ -2012,13 +2027,14 @@ static int index_iterator_advance(
} }
entry = iter->entries.contents[iter->next_idx]; 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++; iter->next_idx++;
continue; continue;
} }
if (iterator_has_ended(&iter->base, entry->path)) { if (iterator_has_ended(&iter->base, entry->path, is_submodule)) {
error = GIT_ITEROVER; error = GIT_ITEROVER;
break; break;
} }
......
...@@ -493,3 +493,50 @@ void test_diff_submodules__skips_empty_includes_used(void) ...@@ -493,3 +493,50 @@ void test_diff_submodules__skips_empty_includes_used(void)
cl_assert_equal_i(1, exp.file_status[GIT_DELTA_UNTRACKED]); cl_assert_equal_i(1, exp.file_status[GIT_DELTA_UNTRACKED]);
git_diff_free(diff); 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));
}
...@@ -1197,8 +1197,11 @@ void test_iterator_workdir__bounded_submodules(void) ...@@ -1197,8 +1197,11 @@ void test_iterator_workdir__bounded_submodules(void)
git_iterator_free(i); 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); git_vector_clear(&filelist);
cl_git_pass(git_vector_insert(&filelist, "sm_changed_head/")); cl_git_pass(git_vector_insert(&filelist, "sm_changed_head/"));
...@@ -1207,7 +1210,7 @@ void test_iterator_workdir__bounded_submodules(void) ...@@ -1207,7 +1210,7 @@ void test_iterator_workdir__bounded_submodules(void)
i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE; i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE;
cl_git_pass(git_iterator_for_workdir(&i, g_repo, index, head, &i_opts)); 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); git_iterator_free(i);
} }
...@@ -1227,16 +1230,19 @@ void test_iterator_workdir__bounded_submodules(void) ...@@ -1227,16 +1230,19 @@ void test_iterator_workdir__bounded_submodules(void)
git_iterator_free(i); 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/"; const char *expected[] = { "sm_changed_head", "sm_changed_index" };
i_opts.end = "sm_changed_head/"; 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.strings = NULL;
i_opts.pathlist.count = 0; i_opts.pathlist.count = 0;
i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE; i_opts.flags = GIT_ITERATOR_DONT_IGNORE_CASE;
cl_git_pass(git_iterator_for_workdir(&i, g_repo, index, head, &i_opts)); 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); git_iterator_free(i);
} }
......
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