Merge pull request #3724 from ethomson/submodule_start_supports_silly_slashes iterator/diff: allow trailing `/` on start/end paths to match submodules
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204
diff --git a/src/iterator.c b/src/iterator.c
index 60b33cc..ec44aac 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/diff/submodules.c b/tests/diff/submodules.c
index 08682cd..eebfef3 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));
+}
diff --git a/tests/iterator/workdir.c b/tests/iterator/workdir.c
index fc7771c..c8f795a 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);
}