Merge pull request #3109 from libgit2/cmn/index-use-diff Use a diff for iteration in index_update_all and index_add_all
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 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 69f02bb..810538c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,9 @@ support for HTTPS connections insead of OpenSSL.
remote functions now take these options or the callbacks instead of
setting them beforehand.
+* The index now uses diffs for `add_all()` and `update_all()` which
+ gives it a speed boost and closer semantics to git.
+
### API additions
diff --git a/src/index.c b/src/index.c
index 7c0c310..8f0976d 100644
--- a/src/index.c
+++ b/src/index.c
@@ -24,6 +24,10 @@
#include "git2/config.h"
#include "git2/sys/index.h"
+static int index_apply_to_wd_diff(git_index *index, int action, const git_strarray *paths,
+ unsigned int flags,
+ git_index_matched_path_cb cb, void *payload);
+
#define entry_size(type,len) ((offsetof(type, path) + (len) + 8) & ~7)
#define short_entry_size(len) entry_size(struct entry_short, len)
#define long_entry_size(len) entry_size(struct entry_long, len)
@@ -2554,6 +2558,13 @@ git_repository *git_index_owner(const git_index *index)
return INDEX_OWNER(index);
}
+enum {
+ INDEX_ACTION_NONE = 0,
+ INDEX_ACTION_UPDATE = 1,
+ INDEX_ACTION_REMOVE = 2,
+ INDEX_ACTION_ADDALL = 3,
+};
+
int git_index_add_all(
git_index *index,
const git_strarray *paths,
@@ -2564,29 +2575,15 @@ int git_index_add_all(
int error;
git_repository *repo;
git_iterator *wditer = NULL;
- const git_index_entry *wd = NULL;
- git_index_entry *entry;
git_pathspec ps;
- const char *match;
- size_t existing;
bool no_fnmatch = (flags & GIT_INDEX_ADD_DISABLE_PATHSPEC_MATCH) != 0;
- int ignorecase;
- git_oid blobid;
assert(index);
- if (INDEX_OWNER(index) == NULL)
- return create_index_error(-1,
- "Could not add paths to index. "
- "Index is not backed up by an existing repository.");
-
repo = INDEX_OWNER(index);
if ((error = git_repository__ensure_not_bare(repo, "index add all")) < 0)
return error;
- if (git_repository__cvar(&ignorecase, repo, GIT_CVAR_IGNORECASE) < 0)
- return -1;
-
if ((error = git_pathspec__init(&ps, paths)) < 0)
return error;
@@ -2597,77 +2594,115 @@ int git_index_add_all(
repo, &ps.pathspec, no_fnmatch)) < 0)
goto cleanup;
- if ((error = git_iterator_for_workdir(
- &wditer, repo, NULL, NULL, 0, ps.prefix, ps.prefix)) < 0)
- goto cleanup;
+ error = index_apply_to_wd_diff(index, INDEX_ACTION_ADDALL, paths, flags, cb, payload);
- while (!(error = git_iterator_advance(&wd, wditer))) {
+ if (error)
+ giterr_set_after_callback(error);
- /* check if path actually matches */
- if (!git_pathspec__match(
- &ps.pathspec, wd->path, no_fnmatch, (bool)ignorecase, &match, NULL))
- continue;
+cleanup:
+ git_iterator_free(wditer);
+ git_pathspec__clear(&ps);
- /* skip ignored items that are not already in the index */
- if ((flags & GIT_INDEX_ADD_FORCE) == 0 &&
- git_iterator_current_is_ignored(wditer) &&
- index_find(&existing, index, wd->path, 0, 0, true) < 0)
- continue;
+ return error;
+}
- /* issue notification callback if requested */
- if (cb && (error = cb(wd->path, match, payload)) != 0) {
- if (error > 0) /* return > 0 means skip this one */
- continue;
- if (error < 0) { /* return < 0 means abort */
- giterr_set_after_callback(error);
- break;
- }
- }
+struct foreach_diff_data {
+ git_index *index;
+ const git_pathspec *pathspec;
+ unsigned int flags;
+ git_index_matched_path_cb cb;
+ void *payload;
+};
- /* TODO: Should we check if the file on disk is already an exact
- * match to the file in the index and skip this work if it is?
- */
+static int apply_each_file(const git_diff_delta *delta, float progress, void *payload)
+{
+ struct foreach_diff_data *data = payload;
+ const char *match, *path;
+ int error = 0;
- /* write the blob to disk and get the oid */
- if ((error = git_blob_create_fromworkdir(&blobid, repo, wd->path)) < 0)
- break;
+ GIT_UNUSED(progress);
- /* make the new entry to insert */
- if ((error = index_entry_dup(&entry, INDEX_OWNER(index), wd)) < 0)
- break;
+ path = delta->old_file.path;
- entry->id = blobid;
+ /* We only want those which match the pathspecs */
+ if (!git_pathspec__match(
+ &data->pathspec->pathspec, path, false, (bool)data->index->ignore_case,
+ &match, NULL))
+ return 0;
- /* add working directory item to index */
- if ((error = index_insert(index, &entry, 1, false)) < 0)
- break;
+ if (data->cb)
+ error = data->cb(path, match, data->payload);
- git_tree_cache_invalidate_path(index->tree, wd->path);
+ if (error > 0) /* skip this entry */
+ return 0;
+ if (error < 0) /* actual error */
+ return error;
- /* add implies conflict resolved, move conflict entries to REUC */
- if ((error = index_conflict_to_reuc(index, wd->path)) < 0) {
- if (error != GIT_ENOTFOUND)
- break;
- giterr_clear();
- }
+ if (delta->status == GIT_DELTA_DELETED)
+ error = git_index_remove_bypath(data->index, path);
+ else
+ error = git_index_add_bypath(data->index, delta->new_file.path);
+
+ return error;
+}
+
+static int index_apply_to_wd_diff(git_index *index, int action, const git_strarray *paths,
+ unsigned int flags,
+ git_index_matched_path_cb cb, void *payload)
+{
+ int error;
+ git_diff *diff;
+ git_pathspec ps;
+ git_repository *repo;
+ git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+ struct foreach_diff_data data = {
+ index,
+ NULL,
+ flags,
+ cb,
+ payload,
+ };
+
+ assert(index);
+ assert(action == INDEX_ACTION_UPDATE || action == INDEX_ACTION_ADDALL);
+
+ repo = INDEX_OWNER(index);
+
+ if (!repo) {
+ return create_index_error(-1,
+ "cannot run update; the index is not backed up by a repository.");
+ }
+
+ /*
+ * We do the matching ourselves intead of passing the list to
+ * diff because we want to tell the callback which one
+ * matched, which we do not know if we ask diff to filter for us.
+ */
+ if ((error = git_pathspec__init(&ps, paths)) < 0)
+ return error;
+
+ opts.flags = GIT_DIFF_INCLUDE_TYPECHANGE;
+ if (action == INDEX_ACTION_ADDALL) {
+ opts.flags |= GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_RECURSE_IGNORED_DIRS;
+ if (flags == GIT_INDEX_ADD_FORCE)
+ opts.flags |= GIT_DIFF_INCLUDE_IGNORED;
}
- if (error == GIT_ITEROVER)
- error = 0;
+ if ((error = git_diff_index_to_workdir(&diff, repo, index, &opts)) < 0)
+ goto cleanup;
+
+ data.pathspec = &ps;
+ error = git_diff_foreach(diff, apply_each_file, NULL, NULL, &data);
+ git_diff_free(diff);
+
+ if (error) /* make sure error is set if callback stopped iteration */
+ giterr_set_after_callback(error);
cleanup:
- git_iterator_free(wditer);
git_pathspec__clear(&ps);
-
return error;
}
-enum {
- INDEX_ACTION_NONE = 0,
- INDEX_ACTION_UPDATE = 1,
- INDEX_ACTION_REMOVE = 2,
-};
-
static int index_apply_to_all(
git_index *index,
int action,
@@ -2764,9 +2799,7 @@ int git_index_update_all(
git_index_matched_path_cb cb,
void *payload)
{
- int error = index_apply_to_all(
- index, INDEX_ACTION_UPDATE, pathspec, cb, payload);
-
+ int error = index_apply_to_wd_diff(index, INDEX_ACTION_UPDATE, pathspec, 0, cb, payload);
if (error) /* make sure error is set if callback stopped iteration */
giterr_set_after_callback(error);
diff --git a/tests/index/addall.c b/tests/index/addall.c
index a7e2583..f13b768 100644
--- a/tests/index/addall.c
+++ b/tests/index/addall.c
@@ -193,6 +193,19 @@ void test_index_addall__repo_lifecycle(void)
cl_repo_commit_from_index(NULL, g_repo, NULL, 0, "first commit");
check_status(g_repo, 0, 0, 0, 3, 0, 0, 1);
+ if (cl_repo_get_bool(g_repo, "core.filemode")) {
+ cl_git_pass(git_index_update_all(index, NULL, NULL, NULL));
+ cl_must_pass(p_chmod(TEST_DIR "/file.zzz", 0777));
+ cl_git_pass(git_index_update_all(index, NULL, NULL, NULL));
+ check_status(g_repo, 0, 0, 1, 3, 0, 0, 1);
+
+ /* go back to what we had before */
+ cl_must_pass(p_chmod(TEST_DIR "/file.zzz", 0666));
+ cl_git_pass(git_index_update_all(index, NULL, NULL, NULL));
+ check_status(g_repo, 0, 0, 0, 3, 0, 0, 1);
+ }
+
+
/* attempt to add an ignored file - does nothing */
strs[0] = "file.foo";
cl_git_pass(git_index_add_all(index, &paths, 0, NULL, NULL));