Commit ff47537557f0ac1919e77c5cb21f36f2e98425de
2015-06-17T14:34:10
diff: check files with the same or newer timestamps When a file on the workdir has the same or a newer timestamp than the index, we need to perform a full check of the contents, as the update of the file may have happened just after we wrote the index. The iterator changes are such that we can reach inside the workdir iterator from the diff, though it may be better to have an accessor instead of moving these structs into the header.
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
diff --git a/src/diff.c b/src/diff.c
index d7365ef..cc93f57 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -816,11 +816,11 @@ static int maybe_modified(
} else if (git_oid_iszero(&nitem->id) && new_is_workdir) {
bool use_ctime = ((diff->diffcaps & GIT_DIFFCAPS_TRUST_CTIME) != 0);
bool use_nanos = ((diff->diffcaps & GIT_DIFFCAPS_TRUST_NANOSECS) != 0);
+ git_index *index;
+ git_iterator_index(&index, info->new_iter);
status = GIT_DELTA_UNMODIFIED;
- /* TODO: add check against index file st_mtime to avoid racy-git */
-
if (S_ISGITLINK(nmode)) {
if ((error = maybe_modified_submodule(&status, &noid, diff, info)) < 0)
return error;
@@ -839,7 +839,8 @@ static int maybe_modified(
!diff_time_eq(&oitem->ctime, &nitem->ctime, use_nanos)) ||
oitem->ino != nitem->ino ||
oitem->uid != nitem->uid ||
- oitem->gid != nitem->gid)
+ oitem->gid != nitem->gid ||
+ (index && nitem->mtime.seconds >= index->stamp.mtime))
{
status = GIT_DELTA_MODIFIED;
modified_uncertain = true;
diff --git a/src/iterator.c b/src/iterator.c
index 7807a16..d5f7eec 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -1762,6 +1762,18 @@ int git_iterator_current_workdir_path(git_buf **path, git_iterator *iter)
return 0;
}
+int git_iterator_index(git_index **out, git_iterator *iter)
+{
+ workdir_iterator *wi = (workdir_iterator *)iter;
+
+ if (iter->type != GIT_ITERATOR_TYPE_WORKDIR)
+ *out = NULL;
+
+ *out = wi->index;
+
+ return 0;
+}
+
int git_iterator_advance_over_with_status(
const git_index_entry **entryptr,
git_iterator_status_t *status,
diff --git a/src/iterator.h b/src/iterator.h
index db1f325..57f8241 100644
--- a/src/iterator.h
+++ b/src/iterator.h
@@ -11,6 +11,7 @@
#include "git2/index.h"
#include "vector.h"
#include "buffer.h"
+#include "ignore.h"
typedef struct git_iterator git_iterator;
@@ -286,4 +287,11 @@ typedef enum {
extern int git_iterator_advance_over_with_status(
const git_index_entry **entry, git_iterator_status_t *status, git_iterator *iter);
+/**
+ * Retrieve the index stored in the iterator.
+ *
+ * Only implemented for the workdir iterator
+ */
+extern int git_iterator_index(git_index **out, git_iterator *iter);
+
#endif
diff --git a/tests/diff/racy.c b/tests/diff/racy.c
index a109f8c..66630cc 100644
--- a/tests/diff/racy.c
+++ b/tests/diff/racy.c
@@ -1,4 +1,5 @@
#include "clar_libgit2.h"
+#include "../checkout/checkout_helpers.h"
#include "buffer.h"
@@ -37,3 +38,35 @@ void test_diff_racy__diff(void)
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
cl_assert_equal_i(1, git_diff_num_deltas(diff));
}
+
+void test_diff_racy__write_index_just_after_file(void)
+{
+ git_index *index;
+ git_diff *diff;
+ git_buf path = GIT_BUF_INIT;
+
+ /* Make sure we do have a timestamp */
+ cl_git_pass(git_repository_index(&index, g_repo));
+ cl_git_pass(git_index_write(index));
+ /* The timestamp will be one second before we change the file */
+ sleep(1);
+
+ cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "A"));
+ cl_git_mkfile(path.ptr, "A");
+
+ /*
+ * Put 'A' into the index, the size field will be filled,
+ * because the index' on-disk timestamp does not match the
+ * file's timestamp. Both timestamps will however match after
+ * writing out the index.
+ */
+ cl_git_pass(git_repository_index(&index, g_repo));
+ cl_git_pass(git_index_add_bypath(index, "A"));
+ cl_git_pass(git_index_write(index));
+
+ /* Change its contents quickly, so we get the same timestamp */
+ cl_git_mkfile(path.ptr, "B");
+
+ cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
+ cl_assert_equal_i(1, git_diff_num_deltas(diff));
+}
diff --git a/tests/diff/workdir.c b/tests/diff/workdir.c
index 6b72f32..3ca82e7 100644
--- a/tests/diff/workdir.c
+++ b/tests/diff/workdir.c
@@ -1623,6 +1623,8 @@ void test_diff_workdir__can_update_index(void)
/* now if we do it again, we should see fewer OID calculations */
+ /* tick again as the index updating from the previous diff might have reset the timestamp */
+ tick_index(index);
basic_diff_status(&diff, &opts);
cl_git_pass(git_diff_get_perfdata(&perf, diff));
diff --git a/tests/status/worktree.c b/tests/status/worktree.c
index 56f98a8..75c7b71 100644
--- a/tests/status/worktree.c
+++ b/tests/status/worktree.c
@@ -985,6 +985,8 @@ void test_status_worktree__update_stat_cache_0(void)
opts.flags &= ~GIT_STATUS_OPT_UPDATE_INDEX;
+ /* tick again as the index updating from the previous diff might have reset the timestamp */
+ tick_index(index);
cl_git_pass(git_status_list_new(&status, repo, &opts));
check_status0(status);
cl_git_pass(git_status_list_get_perfdata(&perf, status));