Commit 4bf630b6 by Russell Belfer

Make diff and status perform soft index reload

This changes `git_index_read` to have two modes - a hard index
reload that always resets the index to match the on-disk data
(which was the old behavior) and a soft index reload that uses
the timestamp / file size information and only replaces the index
data if the file on disk has been modified.

This then updates the git_status code to do a soft reload unless
the new GIT_STATUS_OPT_NO_REFRESH flag is passed in.

This also changes the behavior of the git_diff functions that use
the index so that when an index is not explicitly passed in (i.e.
when the functions call git_repository_index for you), they will
also do a soft reload for you.

This intentionally breaks the file signature of git_index_read
because there has been some confusion about the behavior previously
and it seems like all existing uses of the API should probably be
examined to select the desired behavior.
parent 3940310e
......@@ -33,7 +33,7 @@ int main (int argc, char** argv)
git_repository_free(repo);
}
git_index_read(index);
git_index_read(index, 0);
ecount = git_index_entrycount(index);
if (!ecount)
......
......@@ -607,6 +607,10 @@ GIT_EXTERN(int) git_diff_tree_to_tree(
* The tree you pass will be used for the "old_file" side of the delta, and
* the index will be used for the "new_file" side of the delta.
*
* If you pass NULL for the index, then the existing index of the `repo`
* will be used. In this case, the index will be refreshed from disk
* (if it has changed) before the diff is generated.
*
* @param diff Output pointer to a git_diff pointer to be allocated.
* @param repo The repository containing the tree and index.
* @param old_tree A git_tree object to diff from, or NULL for empty tree.
......@@ -631,6 +635,10 @@ GIT_EXTERN(int) git_diff_tree_to_index(
* The index will be used for the "old_file" side of the delta, and the
* working directory will be used for the "new_file" side of the delta.
*
* If you pass NULL for the index, then the existing index of the `repo`
* will be used. In this case, the index will be refreshed from disk
* (if it has changed) before the diff is generated.
*
* @param diff Output pointer to a git_diff pointer to be allocated.
* @param repo The repository.
* @param index The index to diff from; repo index used if NULL.
......
......@@ -222,16 +222,23 @@ GIT_EXTERN(unsigned int) git_index_caps(const git_index *index);
GIT_EXTERN(int) git_index_set_caps(git_index *index, unsigned int caps);
/**
* Update the contents of an existing index object in memory
* by reading from the hard disk.
* Update the contents of an existing index object in memory by reading
* from the hard disk.
*
* If the file doesn't exist on the filesystem, the index
* will be cleared from its current content.
* Pass 0 for `only_if_changed` to perform a "hard" read that discards
* in-memory changes and always reloads the on-disk index data. If there
* is no on-disk version, the index will be cleared.
*
* Pass non-zero for `only_if_changed` to perform a "soft" read that only
* reloads the index data if it has changed since the last time it was
* loaded. In-memory index data will be untouched. Be aware: if there
* are changes on disk, unwritten in-memory changes will be discarded.
*
* @param index an existing index object
* @param only_if_changed only read if on-disk file is newer than last read
* @return 0 or an error code
*/
GIT_EXTERN(int) git_index_read(git_index *index);
GIT_EXTERN(int) git_index_read(git_index *index, int only_if_changed);
/**
* Write an existing index object from memory back to disk
......
......@@ -118,6 +118,9 @@ typedef enum {
* case-insensitive order
* - GIT_STATUS_OPT_RENAMES_FROM_REWRITES indicates that rename detection
* should include rewritten files
* - GIT_STATUS_OPT_NO_REFRESH bypasses the default status behavior of
* doing a "soft" index reload (i.e. reloading the index data if the
* file on disk has been modified outside libgit2).
*
* Calling `git_status_foreach()` is like calling the extended version
* with: GIT_STATUS_OPT_INCLUDE_IGNORED, GIT_STATUS_OPT_INCLUDE_UNTRACKED,
......@@ -137,6 +140,7 @@ typedef enum {
GIT_STATUS_OPT_SORT_CASE_SENSITIVELY = (1u << 9),
GIT_STATUS_OPT_SORT_CASE_INSENSITIVELY = (1u << 10),
GIT_STATUS_OPT_RENAMES_FROM_REWRITES = (1u << 11),
GIT_STATUS_OPT_NO_REFRESH = (1u << 12),
} git_status_opt_t;
#define GIT_STATUS_OPT_DEFAULTS \
......
......@@ -1837,7 +1837,7 @@ static int checkout_data_init(
} else {
/* otherwise, grab and reload the index */
if ((error = git_repository_index(&data->index, data->repo)) < 0 ||
(error = git_index_read(data->index)) < 0)
(error = git_index_read(data->index, false)) < 0)
goto cleanup;
/* cannot checkout if unresolved conflicts exist */
......
......@@ -1184,6 +1184,17 @@ int git_diff_tree_to_tree(
return error;
}
static int diff_load_index(git_index **index, git_repository *repo)
{
int error = git_repository_index__weakptr(index, repo);
/* reload the repository index when user did not pass one in */
if (!error && git_index_read(*index, true) < 0)
giterr_clear();
return error;
}
int git_diff_tree_to_index(
git_diff **diff,
git_repository *repo,
......@@ -1196,7 +1207,7 @@ int git_diff_tree_to_index(
assert(diff && repo);
if (!index && (error = git_repository_index__weakptr(&index, repo)) < 0)
if (!index && (error = diff_load_index(&index, repo)) < 0)
return error;
if (index->ignore_case) {
......@@ -1239,7 +1250,7 @@ int git_diff_index_to_workdir(
assert(diff && repo);
if (!index && (error = git_repository_index__weakptr(&index, repo)) < 0)
if (!index && (error = diff_load_index(&index, repo)) < 0)
return error;
DIFF_FROM_ITERATORS(
......@@ -1278,11 +1289,15 @@ int git_diff_tree_to_workdir_with_index(
{
int error = 0;
git_diff *d1 = NULL, *d2 = NULL;
git_index *index = NULL;
assert(diff && repo);
if (!(error = git_diff_tree_to_index(&d1, repo, old_tree, NULL, opts)) &&
!(error = git_diff_index_to_workdir(&d2, repo, NULL, opts)))
if ((error = diff_load_index(&index, repo)) < 0)
return error;
if (!(error = git_diff_tree_to_index(&d1, repo, old_tree, index, opts)) &&
!(error = git_diff_index_to_workdir(&d2, repo, index, opts)))
error = git_diff_merge(d1, d2);
git_diff_free(d2);
......
......@@ -349,7 +349,7 @@ int git_index_open(git_index **index_out, const char *index_path)
*index_out = index;
GIT_REFCOUNT_INC(index);
return (index_path != NULL) ? git_index_read(index) : 0;
return (index_path != NULL) ? git_index_read(index, false) : 0;
}
int git_index_new(git_index **out)
......@@ -451,11 +451,11 @@ unsigned int git_index_caps(const git_index *index)
(index->no_symlinks ? GIT_INDEXCAP_NO_SYMLINKS : 0));
}
int git_index_read(git_index *index)
int git_index_read(git_index *index, int only_if_changed)
{
int error = 0, updated;
git_buf buffer = GIT_BUF_INIT;
git_futils_filestamp stamp = {0};
git_futils_filestamp stamp = index->stamp;
if (!index->index_file_path)
return create_index_error(-1,
......@@ -464,12 +464,13 @@ int git_index_read(git_index *index)
index->on_disk = git_path_exists(index->index_file_path);
if (!index->on_disk) {
git_index_clear(index);
if (!only_if_changed)
git_index_clear(index);
return 0;
}
updated = git_futils_filestamp_check(&stamp, index->index_file_path);
if (updated <= 0)
if (updated < 0 || (only_if_changed && !updated))
return updated;
error = git_futils_readbuffer(&buffer, index->index_file_path);
......
......@@ -251,12 +251,17 @@ int git_status_list_new(
return error;
/* if there is no HEAD, that's okay - we'll make an empty iterator */
if (((error = git_repository_head_tree(&head, repo)) < 0) &&
error != GIT_ENOTFOUND && error != GIT_EUNBORNBRANCH) {
git_index_free(index); /* release index */
return error;
if ((error = git_repository_head_tree(&head, repo)) < 0) {
if (error != GIT_ENOTFOUND && error != GIT_EUNBORNBRANCH)
goto done;
giterr_clear();
}
/* refresh index from disk unless prevented */
if ((flags & GIT_STATUS_OPT_NO_REFRESH) == 0 &&
git_index_read(index, true) < 0)
giterr_clear();
status = git_status_list_alloc(index);
GITERR_CHECK_ALLOC(status);
......@@ -291,7 +296,7 @@ int git_status_list_new(
if (show != GIT_STATUS_SHOW_WORKDIR_ONLY) {
if ((error = git_diff_tree_to_index(
&status->head2idx, repo, head, NULL, &diffopt)) < 0)
&status->head2idx, repo, head, index, &diffopt)) < 0)
goto done;
if ((flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 &&
......@@ -301,7 +306,7 @@ int git_status_list_new(
if (show != GIT_STATUS_SHOW_INDEX_ONLY) {
if ((error = git_diff_index_to_workdir(
&status->idx2wd, repo, NULL, &diffopt)) < 0)
&status->idx2wd, repo, index, &diffopt)) < 0)
goto done;
if ((flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0 &&
......
......@@ -1527,6 +1527,7 @@ static void submodule_get_wd_status(
const git_oid *wd_oid =
(sm->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID) ? &sm->wd_oid : NULL;
git_tree *sm_head = NULL;
git_index *index = NULL;
git_diff_options opt = GIT_DIFF_OPTIONS_INIT;
git_diff *diff;
......@@ -1558,12 +1559,14 @@ static void submodule_get_wd_status(
if (ign == GIT_SUBMODULE_IGNORE_NONE)
opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED;
(void)git_repository_index__weakptr(&index, sm_repo);
/* if we don't have an unborn head, check diff with index */
if (git_repository_head_tree(&sm_head, sm_repo) < 0)
giterr_clear();
else {
/* perform head to index diff on submodule */
if (git_diff_tree_to_index(&diff, sm_repo, sm_head, NULL, &opt) < 0)
if (git_diff_tree_to_index(&diff, sm_repo, sm_head, index, &opt) < 0)
giterr_clear();
else {
if (git_diff_num_deltas(diff) > 0)
......@@ -1576,7 +1579,7 @@ static void submodule_get_wd_status(
}
/* perform index-to-workdir diff on submodule */
if (git_diff_index_to_workdir(&diff, sm_repo, NULL, &opt) < 0)
if (git_diff_index_to_workdir(&diff, sm_repo, index, &opt) < 0)
giterr_clear();
else {
size_t untracked =
......
......@@ -54,7 +54,6 @@ void test_checkout_head__with_index_only_tree(void)
cl_git_pass(git_checkout_head(g_repo, &opts));
cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_read(index)); /* reload if needed */
cl_assert(!git_path_isfile("testrepo/newdir/newfile.txt"));
cl_assert(git_index_get_bypath(index, "newdir/newfile.txt", 0) == NULL);
......
......@@ -221,11 +221,15 @@ static int diff_print_cb(
const git_diff_line *line,
void *payload)
{
GIT_UNUSED(payload);
GIT_UNUSED(delta);
GIT_UNUSED(hunk);
fprintf((FILE *)payload, "%c%.*s",
line->origin, (int)line->content_len, line->content);
FILE *fp = payload;
GIT_UNUSED(delta); GIT_UNUSED(hunk);
if (line->origin == GIT_DIFF_LINE_CONTEXT ||
line->origin == GIT_DIFF_LINE_ADDITION ||
line->origin == GIT_DIFF_LINE_DELETION)
fputc(line->origin, fp);
fwrite(line->content, 1, line->content_len, fp);
return 0;
}
......
......@@ -1329,3 +1329,73 @@ void test_diff_workdir__patience_diff(void)
git_patch_free(patch);
git_diff_free(diff);
}
void test_diff_workdir__with_stale_index(void)
{
git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
git_diff *diff = NULL;
git_index *idx = NULL;
diff_expects exp;
g_repo = cl_git_sandbox_init("status");
cl_git_pass(git_repository_index(&idx, g_repo));
/* make the in-memory index invalid */
{
git_repository *r2;
git_index *idx2;
cl_git_pass(git_repository_open(&r2, "status"));
cl_git_pass(git_repository_index(&idx2, r2));
cl_git_pass(git_index_add_bypath(idx2, "new_file"));
cl_git_pass(git_index_add_bypath(idx2, "subdir/new_file"));
cl_git_pass(git_index_remove_bypath(idx2, "staged_new_file"));
cl_git_pass(git_index_remove_bypath(idx2, "staged_changes_file_deleted"));
cl_git_pass(git_index_write(idx2));
git_index_free(idx2);
git_repository_free(r2);
}
opts.context_lines = 3;
opts.interhunk_lines = 1;
opts.flags |= GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_UNMODIFIED;
/* first try with index pointer which should prevent reload */
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, idx, &opts));
memset(&exp, 0, sizeof(exp));
cl_git_pass(git_diff_foreach(
diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
cl_assert_equal_i(17, exp.files);
cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
cl_assert_equal_i(4, exp.file_status[GIT_DELTA_DELETED]);
cl_assert_equal_i(4, exp.file_status[GIT_DELTA_MODIFIED]);
cl_assert_equal_i(4, exp.file_status[GIT_DELTA_UNTRACKED]);
cl_assert_equal_i(5, exp.file_status[GIT_DELTA_UNMODIFIED]);
git_diff_free(diff);
/* now let's try without the index pointer which should trigger reload */
/* two files that were UNTRACKED should have become UNMODIFIED */
/* one file that was UNMODIFIED should now have become UNTRACKED */
/* one file that was DELETED should now be gone completely */
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
memset(&exp, 0, sizeof(exp));
cl_git_pass(git_diff_foreach(
diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
cl_assert_equal_i(16, exp.files);
cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
cl_assert_equal_i(3, exp.file_status[GIT_DELTA_DELETED]);
cl_assert_equal_i(4, exp.file_status[GIT_DELTA_MODIFIED]);
cl_assert_equal_i(3, exp.file_status[GIT_DELTA_UNTRACKED]);
cl_assert_equal_i(6, exp.file_status[GIT_DELTA_UNMODIFIED]);
git_index_free(idx);
}
......@@ -68,10 +68,11 @@ static int index_status_cb(
return 0;
}
static void check_status(
static void check_status_at_line(
git_repository *repo,
size_t index_adds, size_t index_dels, size_t index_mods,
size_t wt_adds, size_t wt_dels, size_t wt_mods, size_t ignores)
size_t wt_adds, size_t wt_dels, size_t wt_mods, size_t ignores,
const char *file, int line)
{
index_status_counts vals;
......@@ -79,15 +80,25 @@ static void check_status(
cl_git_pass(git_status_foreach(repo, index_status_cb, &vals));
cl_assert_equal_sz(index_adds, vals.index_adds);
cl_assert_equal_sz(index_dels, vals.index_dels);
cl_assert_equal_sz(index_mods, vals.index_mods);
cl_assert_equal_sz(wt_adds, vals.wt_adds);
cl_assert_equal_sz(wt_dels, vals.wt_dels);
cl_assert_equal_sz(wt_mods, vals.wt_mods);
cl_assert_equal_sz(ignores, vals.ignores);
clar__assert_equal(
file,line,"wrong index adds", 1, "%"PRIuZ, index_adds, vals.index_adds);
clar__assert_equal(
file,line,"wrong index dels", 1, "%"PRIuZ, index_dels, vals.index_dels);
clar__assert_equal(
file,line,"wrong index mods", 1, "%"PRIuZ, index_mods, vals.index_mods);
clar__assert_equal(
file,line,"wrong workdir adds", 1, "%"PRIuZ, wt_adds, vals.wt_adds);
clar__assert_equal(
file,line,"wrong workdir dels", 1, "%"PRIuZ, wt_dels, vals.wt_dels);
clar__assert_equal(
file,line,"wrong workdir mods", 1, "%"PRIuZ, wt_mods, vals.wt_mods);
clar__assert_equal(
file,line,"wrong ignores", 1, "%"PRIuZ, ignores, vals.ignores);
}
#define check_status(R,IA,ID,IM,WA,WD,WM,IG) \
check_status_at_line(R,IA,ID,IM,WA,WD,WM,IG,__FILE__,__LINE__)
static void check_stat_data(git_index *index, const char *path, bool match)
{
const git_index_entry *entry;
......
......@@ -63,7 +63,7 @@ void test_index_names__roundtrip(void)
git_index_clear(repo_index);
cl_assert(git_index_name_entrycount(repo_index) == 0);
cl_git_pass(git_index_read(repo_index));
cl_git_pass(git_index_read(repo_index, 0));
cl_assert(git_index_name_entrycount(repo_index) == 3);
conflict_name = git_index_name_get_byindex(repo_index, 0);
......@@ -120,7 +120,7 @@ void test_index_names__cleaned_on_checkout_tree(void)
git_reference_name_to_id(&oid, repo, "refs/heads/master");
git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY);
git_checkout_tree(repo, obj, &opts);
cl_assert(git_index_name_entrycount(repo_index) == 0);
cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
git_object_free(obj);
}
......@@ -133,7 +133,7 @@ void test_index_names__cleaned_on_checkout_head(void)
test_index_names__add();
git_checkout_head(repo, &opts);
cl_assert(git_index_name_entrycount(repo_index) == 0);
cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
}
void test_index_names__retained_on_checkout_index(void)
......
......@@ -276,8 +276,6 @@ void test_index_reuc__write(void)
0100644, &their_oid));
cl_git_pass(git_index_write(repo_index));
cl_git_pass(git_index_read(repo_index));
cl_assert_equal_i(2, git_index_reuc_entrycount(repo_index));
/* ensure sort order was round-tripped correct */
......
......@@ -349,14 +349,14 @@ void test_index_tests__remove_entry(void)
cl_git_pass(git_index_add_bypath(index, "hello"));
cl_git_pass(git_index_write(index));
cl_git_pass(git_index_read(index)); /* reload */
cl_git_pass(git_index_read(index, false)); /* reload */
cl_assert(git_index_entrycount(index) == 1);
cl_assert(git_index_get_bypath(index, "hello", 0) != NULL);
cl_git_pass(git_index_remove(index, "hello", 0));
cl_git_pass(git_index_write(index));
cl_git_pass(git_index_read(index)); /* reload */
cl_git_pass(git_index_read(index, false)); /* reload */
cl_assert(git_index_entrycount(index) == 0);
cl_assert(git_index_get_bypath(index, "hello", 0) == NULL);
......@@ -388,7 +388,7 @@ void test_index_tests__remove_directory(void)
cl_git_pass(git_index_add_bypath(index, "b.txt"));
cl_git_pass(git_index_write(index));
cl_git_pass(git_index_read(index)); /* reload */
cl_git_pass(git_index_read(index, false)); /* reload */
cl_assert_equal_i(4, (int)git_index_entrycount(index));
cl_assert(git_index_get_bypath(index, "a/1.txt", 0) != NULL);
cl_assert(git_index_get_bypath(index, "a/2.txt", 0) != NULL);
......@@ -397,7 +397,7 @@ void test_index_tests__remove_directory(void)
cl_git_pass(git_index_remove(index, "a/1.txt", 0));
cl_git_pass(git_index_write(index));
cl_git_pass(git_index_read(index)); /* reload */
cl_git_pass(git_index_read(index, false)); /* reload */
cl_assert_equal_i(3, (int)git_index_entrycount(index));
cl_assert(git_index_get_bypath(index, "a/1.txt", 0) == NULL);
cl_assert(git_index_get_bypath(index, "a/2.txt", 0) != NULL);
......@@ -406,7 +406,7 @@ void test_index_tests__remove_directory(void)
cl_git_pass(git_index_remove_directory(index, "a", 0));
cl_git_pass(git_index_write(index));
cl_git_pass(git_index_read(index)); /* reload */
cl_git_pass(git_index_read(index, false)); /* reload */
cl_assert_equal_i(1, (int)git_index_entrycount(index));
cl_assert(git_index_get_bypath(index, "a/1.txt", 0) == NULL);
cl_assert(git_index_get_bypath(index, "a/2.txt", 0) == NULL);
......@@ -517,7 +517,7 @@ void test_index_tests__reload_from_disk(void)
/* Sync the changes back into the read_index */
cl_assert_equal_sz(0, git_index_entrycount(read_index));
cl_git_pass(git_index_read(read_index));
cl_git_pass(git_index_read(read_index, false));
cl_assert_equal_i(true, read_index->on_disk);
cl_assert_equal_sz(2, git_index_entrycount(read_index));
......@@ -526,7 +526,7 @@ void test_index_tests__reload_from_disk(void)
cl_git_pass(p_unlink(write_index->index_file_path));
/* Sync the changes back into the read_index */
cl_git_pass(git_index_read(read_index));
cl_git_pass(git_index_read(read_index, false));
cl_assert_equal_i(false, read_index->on_disk);
cl_assert_equal_sz(0, git_index_entrycount(read_index));
......
......@@ -470,16 +470,15 @@ void test_status_worktree__conflict_with_diff3(void)
cl_assert_equal_i(GIT_STATUS_WT_MODIFIED, status);
cl_git_pass(git_repository_index(&index, repo));
cl_git_pass(git_index_remove(index, "modified_file", 0));
cl_git_pass(git_index_conflict_add(index, &ancestor_entry,
&our_entry, &their_entry));
cl_git_pass(git_index_conflict_add(
index, &ancestor_entry, &our_entry, &their_entry));
cl_git_pass(git_index_write(index));
git_index_free(index);
cl_git_pass(git_status_file(&status, repo, "modified_file"));
cl_assert_equal_i(GIT_STATUS_INDEX_DELETED | GIT_STATUS_WT_NEW, status);
git_index_free(index);
}
static const char *filemode_paths[] = {
......
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