Merge pull request #3219 from libgit2/cmn/racy-diff

Zero out racily-clean entries' file_size

src/checkout.c

@@ -150,6 +150,15 @@ static int checkout_notify(
 	}
 }
 
+GIT_INLINE(bool) is_workdir_base_or_new(
+	const git_oid *workdir_id,
+	const git_diff_file *baseitem,
+	const git_diff_file *newitem)
+{
+	return (git_oid__cmp(&baseitem->id, workdir_id) == 0 ||
+		git_oid__cmp(&newitem->id, workdir_id) == 0);
+}
+
 static bool checkout_is_workdir_modified(
 	checkout_data *data,
 	const git_diff_file *baseitem,
@@ -193,8 +202,7 @@ static bool checkout_is_workdir_modified(
 		if (wditem->mtime.seconds == ie->mtime.seconds &&
 			wditem->mtime.nanoseconds == ie->mtime.nanoseconds &&
 			wditem->file_size == ie->file_size)
-			return (git_oid__cmp(&baseitem->id, &ie->id) != 0 &&
-				git_oid_cmp(&newitem->id, &ie->id) != 0);
+			return !is_workdir_base_or_new(&ie->id, baseitem, newitem);
 	}
 
 	/* depending on where base is coming from, we may or may not know
@@ -206,7 +214,10 @@ static bool checkout_is_workdir_modified(
 	if (git_diff__oid_for_entry(&oid, data->diff, wditem, NULL) < 0)
 		return false;
 
-	return (git_oid__cmp(&baseitem->id, &oid) != 0);
+	/* Allow the checkout if the workdir is not modified *or* if the checkout
+	 * target's contents are already in the working directory.
+	 */
+	return !is_workdir_base_or_new(&oid, baseitem, newitem);
 }
 
 #define CHECKOUT_ACTION_IF(FLAG,YES,NO) \

src/index.c

@@ -658,11 +658,29 @@ int git_index__changed_relative_to(
 			index->stamp.ino != fs->ino);
 }
 
+/*
+ * Force the next diff to take a look at those entries which have the
+ * same timestamp as the current index.
+ */
+static void truncate_racily_clean(git_index *index)
+{
+	size_t i;
+	git_index_entry *entry;
+	git_time_t ts = index->stamp.mtime;
+
+	git_vector_foreach(&index->entries, i, entry) {
+		if (entry->mtime.seconds == ts || ts == 0)
+			entry->file_size = 0;
+	}
+}
+
 int git_index_write(git_index *index)
 {
 	git_indexwriter writer = GIT_INDEXWRITER_INIT;
 	int error;
 
+	truncate_racily_clean(index);
+
 	if ((error = git_indexwriter_init(&writer, index)) == 0)
 		error = git_indexwriter_commit(&writer);
 

src/unix/posix.h

@@ -19,6 +19,9 @@ typedef int GIT_SOCKET;
 #define p_lstat(p,b) lstat(p,b)
 #define p_stat(p,b) stat(p, b)
 
+#define p_utimes(f, t) utimes(f, t)
+#define p_futimes(f, t) futimes(f, t)
+
 #define p_readlink(a, b, c) readlink(a, b, c)
 #define p_symlink(o,n) symlink(o, n)
 #define p_link(o,n) link(o, n)

src/win32/posix.h

@@ -20,6 +20,9 @@ typedef SOCKET GIT_SOCKET;
 extern int p_lstat(const char *file_name, struct stat *buf);
 extern int p_stat(const char* path, struct stat* buf);
 
+extern int p_utimes(const char *filename, const struct timeval times[2]);
+extern int p_futimes(int fd, const struct timeval times[2]);
+
 extern int p_readlink(const char *path, char *buf, size_t bufsiz);
 extern int p_symlink(const char *old, const char *new);
 extern int p_link(const char *old, const char *new);

src/win32/posix_w32.c

@@ -201,6 +201,44 @@ int p_lstat_posixly(const char *filename, struct stat *buf)
 	return do_lstat(filename, buf, true);
 }
 
+int p_utimes(const char *filename, const struct timeval times[2])
+{
+	int fd, error;
+
+	if ((fd = p_open(filename, O_RDWR)) < 0)
+		return fd;
+
+	error = p_futimes(fd, times);
+
+	close(fd);
+	return error;
+}
+
+int p_futimes(int fd, const struct timeval times[2])
+{
+	HANDLE handle;
+	FILETIME atime = {0}, mtime = {0};
+
+	if (times == NULL) {
+		SYSTEMTIME st;
+
+		GetSystemTime(&st);
+		SystemTimeToFileTime(&st, &atime);
+		SystemTimeToFileTime(&st, &mtime);
+	} else {
+		git_win32__timeval_to_filetime(&atime, times[0]);
+		git_win32__timeval_to_filetime(&mtime, times[1]);
+	}
+
+	if ((handle = (HANDLE)_get_osfhandle(fd)) == INVALID_HANDLE_VALUE)
+		return -1;
+
+	if (SetFileTime(handle, NULL, &atime, &mtime) == 0)
+		return -1;
+
+	return 0;
+}
+
 int p_readlink(const char *path, char *buf, size_t bufsiz)
 {
 	git_win32_path path_w, target_w;

src/win32/w32_util.h

@@ -79,6 +79,16 @@ GIT_INLINE(time_t) git_win32__filetime_to_time_t(const FILETIME *ft)
 	return (time_t)winTime;
 }
 
+GIT_INLINE(void) git_win32__timeval_to_filetime(
+	FILETIME *ft, const struct timeval tv)
+{
+	long long ticks = (tv.tv_sec * 10000000LL) +
+		(tv.tv_usec * 10LL) + 116444736000000000LL;
+
+	ft->dwHighDateTime = ((ticks >> 32) & 0xffffffffLL);
+	ft->dwLowDateTime = (ticks & 0xffffffffLL);
+}
+
 GIT_INLINE(int) git_win32__file_attribute_to_stat(
 	struct stat *st,
 	const WIN32_FILE_ATTRIBUTE_DATA *attrdata,

tests/checkout/checkout_helpers.c

@@ -2,6 +2,7 @@
 #include "checkout_helpers.h"
 #include "refs.h"
 #include "fileops.h"
+#include "index.h"
 
 void assert_on_branch(git_repository *repo, const char *branch)
 {
@@ -128,3 +129,24 @@ int checkout_count_callback(
 
 	return 0;
 }
+
+void tick_index(git_index *index)
+{
+	int index_fd;
+	git_time_t ts;
+	struct timeval times[2];
+
+	cl_assert(index->on_disk);
+	cl_assert(git_index_path(index));
+
+	cl_git_pass(git_index_read(index, true));
+	ts = index->stamp.mtime;
+
+	times[0].tv_sec  = ts;
+	times[0].tv_usec = 0;
+	times[1].tv_sec  = ts + 1;
+	times[1].tv_usec = 0;
+
+	cl_git_pass(p_utimes(git_index_path(index), times));
+	cl_git_pass(git_index_read(index, true));
+}

tests/checkout/checkout_helpers.h

@@ -27,3 +27,5 @@ extern int checkout_count_callback(
 	const git_diff_file *target,
 	const git_diff_file *workdir,
 	void *payload);
+
+extern void tick_index(git_index *index);

tests/checkout/crlf.c

@@ -4,6 +4,7 @@
 
 #include "git2/checkout.h"
 #include "repository.h"
+#include "index.h"
 #include "posix.h"
 
 static git_repository *g_repo;
@@ -40,9 +41,10 @@ void test_checkout_crlf__autocrlf_false_index_size_is_unfiltered_size(void)
 
 	cl_repo_set_bool(g_repo, "core.autocrlf", false);
 
-	git_checkout_head(g_repo, &opts);
-
 	git_repository_index(&index, g_repo);
+	tick_index(index);
+
+	git_checkout_head(g_repo, &opts);
 
 	cl_assert((entry = git_index_get_bypath(index, "all-lf", 0)) != NULL);
 	cl_assert(entry->file_size == strlen(ALL_LF_TEXT_RAW));
@@ -140,9 +142,10 @@ void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void)
 
 	cl_repo_set_bool(g_repo, "core.autocrlf", true);
 
-	git_checkout_head(g_repo, &opts);
-
 	git_repository_index(&index, g_repo);
+	tick_index(index);
+
+	git_checkout_head(g_repo, &opts);
 
 	cl_assert((entry = git_index_get_bypath(index, "all-lf", 0)) != NULL);
 

tests/core/posix.c

@@ -97,3 +97,52 @@ void test_core_posix__inet_pton(void)
 	cl_git_fail(p_inet_pton(5, "315.124", NULL)); /* AF_CHAOS */
 	cl_assert_equal_i(EAFNOSUPPORT, errno);
 }
+
+void test_core_posix__utimes(void)
+{
+	struct timeval times[2];
+	struct stat st;
+	time_t curtime;
+	int fd;
+
+	/* test p_utimes */
+	times[0].tv_sec = 1234567890;
+	times[0].tv_usec = 0;
+	times[1].tv_sec = 1234567890;
+	times[1].tv_usec = 0;
+
+	cl_git_mkfile("foo", "Dummy file.");
+	cl_must_pass(p_utimes("foo", times));
+
+	p_stat("foo", &st);
+	cl_assert_equal_i(1234567890, st.st_atime);
+	cl_assert_equal_i(1234567890, st.st_mtime);
+
+
+	/* test p_futimes */
+	times[0].tv_sec = 1414141414;
+	times[0].tv_usec = 0;
+	times[1].tv_sec = 1414141414;
+	times[1].tv_usec = 0;
+
+	cl_must_pass(fd = p_open("foo", O_RDWR));
+	cl_must_pass(p_futimes(fd, times));
+	p_close(fd);
+
+	p_stat("foo", &st);
+	cl_assert_equal_i(1414141414, st.st_atime);
+	cl_assert_equal_i(1414141414, st.st_mtime);
+
+
+	/* test p_utimes with current time, assume that
+	 * it takes < 5 seconds to get the time...!
+	 */
+	cl_must_pass(p_utimes("foo", NULL));
+
+	curtime = time(NULL);
+	p_stat("foo", &st);
+	cl_assert((st.st_atime - curtime) < 5);
+	cl_assert((st.st_mtime - curtime) < 5);
+
+	p_unlink("foo");
+}

tests/diff/racy.c

@@ -0,0 +1,39 @@
+#include "clar_libgit2.h"
+
+#include "buffer.h"
+
+static git_repository *g_repo;
+
+void test_diff_racy__initialize(void)
+{
+	cl_git_pass(git_repository_init(&g_repo, "diff_racy", false));
+}
+
+void test_diff_racy__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_diff_racy__diff(void)
+{
+	git_index *index;
+	git_diff *diff;
+	git_buf path = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "A"));
+	cl_git_mkfile(path.ptr, "A");
+
+	/* Put 'A' into 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));
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
+	cl_assert_equal_i(0, git_diff_num_deltas(diff));
+
+	/* 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));
+}

tests/diff/workdir.c

@@ -2,6 +2,7 @@
 #include "diff_helpers.h"
 #include "repository.h"
 #include "git2/sys/diff.h"
+#include "../checkout/checkout_helpers.h"
 
 static git_repository *g_repo = NULL;
 
@@ -1583,6 +1584,7 @@ void test_diff_workdir__can_update_index(void)
 	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
 	git_diff *diff = NULL;
 	git_diff_perfdata perf = GIT_DIFF_PERFDATA_INIT;
+	git_index *index;
 
 	g_repo = cl_git_sandbox_init("status");
 
@@ -1607,6 +1609,10 @@ void test_diff_workdir__can_update_index(void)
 	/* now allow diff to update stat cache */
 	opts.flags |= GIT_DIFF_UPDATE_INDEX;
 
+	/* advance a tick for the index so we don't re-calculate racily-clean entries */
+	cl_git_pass(git_repository_index__weakptr(&index, g_repo));
+	tick_index(index);
+
 	basic_diff_status(&diff, &opts);
 
 	cl_git_pass(git_diff_get_perfdata(&perf, diff));

tests/merge/workdir/dirty.c

@@ -2,6 +2,7 @@
 #include "git2/merge.h"
 #include "buffer.h"
 #include "merge.h"
+#include "index.h"
 #include "../merge_helpers.h"
 #include "posix.h"
 
@@ -231,9 +232,20 @@ static int merge_differently_filtered_files(char *files[])
 	cl_git_pass(git_reference_peel(&head_object, head, GIT_OBJ_COMMIT));
 	cl_git_pass(git_reset(repo, head_object, GIT_RESET_HARD, NULL));
 
+	/* Emulate checkout with a broken or misconfigured filter:  modify some
+	 * files on-disk and then update the index with the updated file size
+	 * and time, as if some filter applied them.  These files should not be
+	 * treated as dirty since we created them.
+	 *
+	 * (Make sure to update the index stamp to defeat racy-git protections
+	 * trying to sanity check the files in the index; those would rehash the
+	 * files, showing them as dirty, the exact mechanism we're trying to avoid.)
+	 */
+
 	write_files(files);
 	hack_index(files);
 
+	repo_index->stamp.mtime = time(NULL) + 1;
 	cl_git_pass(git_index_write(repo_index));
 
 	error = merge_branch();

tests/status/worktree.c

@@ -6,6 +6,7 @@
 #include "util.h"
 #include "path.h"
 #include "../diff/diff_helpers.h"
+#include "../checkout/checkout_helpers.h"
 #include "git2/sys/diff.h"
 
 /**
@@ -956,6 +957,7 @@ void test_status_worktree__update_stat_cache_0(void)
 	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
 	git_status_list *status;
 	git_diff_perfdata perf = GIT_DIFF_PERFDATA_INIT;
+	git_index *index;
 
 	opts.flags = GIT_STATUS_OPT_DEFAULTS;
 
@@ -967,6 +969,10 @@ void test_status_worktree__update_stat_cache_0(void)
 
 	git_status_list_free(status);
 
+	/* tick the index so we avoid recalculating racily-clean entries */
+	cl_git_pass(git_repository_index__weakptr(&index, repo));
+	tick_index(index);
+
 	opts.flags |= GIT_STATUS_OPT_UPDATE_INDEX;
 
 	cl_git_pass(git_status_list_new(&status, repo, &opts));