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.

diff --git a/src/diff.c b/src/diff.c
index d7365ef772ea0f27c34ff1981ceec2c2de5f35a2..cc93f57cd649917d43f00b21d988a56178960347 100644 (file)
--- 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 7807a16364bbe49500276791a0e8ac59dc5f636b..d5f7eec341ad5895b0ee200fbb5c4cf99e03db63 100644 (file)
--- 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 db1f325a73795670f0fc42d77e56ea07edced774..57f82416a3b2d7d265bacfc032f48c8ee873708e 100644 (file)
--- 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 a109f8c3b7f3e777aaec7a12170d1589f16d4825..66630cc4d8f9252b1cfb63e31688c1ea6c1d9715 100644 (file)
--- 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 6b72f3286b458e582c1b63a45ef47e7ebbe8f80b..3ca82e73306668be66bfd5270e1835246604362e 100644 (file)
--- 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 56f98a8824b0088701d34a27d47fb97d240ec5a6..75c7b71b079f56c6477e8879fa18772251ce36d4 100644 (file)
--- 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));