fsync parent directories when fsyncing

When fsync'ing files, fsync the parent directory in the case where we
rename a file into place, or create a new file, to ensure that the
directory entry is flushed correctly.

6 files changed, 63 insertions, 5 deletions

diff --git a/src/filebuf.c b/src/filebuf.c
index c4a13ffab..f37cf02b4 100644
--- a/src/filebuf.c
+++ b/src/filebuf.c
@@ -445,6 +445,9 @@ int git_filebuf_commit(git_filebuf *file)
 		goto on_error;
 	}
 
+	if (file->do_fsync && git_futils_fsync_parent(file->path_original) < 0)
+		goto on_error;
+
 	file->did_rename = true;
 
 	git_filebuf_cleanup(file);
diff --git a/src/fileops.c b/src/fileops.c
index 13b1fc2a1..881d1bed4 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -265,8 +265,13 @@ int git_futils_writebuffer(
 		return error;
 	}
 
-	if ((error = p_close(fd)) < 0)
+	if ((error = p_close(fd)) < 0) {
 		giterr_set(GITERR_OS, "error while closing '%s'", path);
+		return error;
+	}
+
+	if (do_fsync && (flags & O_CREAT))
+		error = git_futils_fsync_parent(path);
 
 	return error;
 }
@@ -1119,3 +1124,28 @@ void git_futils_filestamp_set_from_stat(
 		memset(stamp, 0, sizeof(*stamp));
 	}
 }
+
+int git_futils_fsync_dir(const char *path)
+{
+	int fd, error = -1;
+
+	if ((fd = p_open(path, O_RDONLY)) < 0) {
+		giterr_set(GITERR_OS, "failed to open directory '%s' for fsync", path);
+		return -1;
+	}
+
+	if ((error = p_fsync(fd)) < 0)
+		giterr_set(GITERR_OS, "failed to fsync directory '%s'", path);
+
+	p_close(fd);
+	return error;
+}
+
+int git_futils_fsync_parent(const char *path)
+{
+	char *parent = git_path_dirname(path);
+	int error = git_futils_fsync_dir(parent);
+
+	git__free(parent);
+	return error;
+}
diff --git a/src/fileops.h b/src/fileops.h
index 9a6fc3f6b..46886b0d7 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -363,4 +363,22 @@ extern void git_futils_filestamp_set(
 extern void git_futils_filestamp_set_from_stat(
 	git_futils_filestamp *stamp, struct stat *st);
 
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the directory to sync.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_dir(const char *path);
+
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the file whose parent directory should be synced.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_parent(const char *path);
+
 #endif /* INCLUDE_fileops_h__ */
diff --git a/src/indexer.c b/src/indexer.c
index 30c365565..9510253b5 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -1086,7 +1086,14 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 		goto on_error;
 
 	/* And don't forget to rename the packfile to its new place. */
-	p_rename(idx->pack->pack_name, git_buf_cstr(&filename));
+	if (p_rename(idx->pack->pack_name, git_buf_cstr(&filename)) < 0)
+		goto on_error;
+
+	/* And fsync the parent directory if we're asked to. */
+	if (git_object__synchronized_writing &&
+		git_futils_fsync_parent(git_buf_cstr(&filename)) < 0)
+		goto on_error;
+
 	idx->pack_committed = 1;
 
 	git_buf_free(&filename);
diff --git a/tests/pack/packbuilder.c b/tests/pack/packbuilder.c
index 9afc178b3..77f61f79b 100644
--- a/tests/pack/packbuilder.c
+++ b/tests/pack/packbuilder.c
@@ -204,7 +204,7 @@ void test_pack_packbuilder__fsync_when_asked(void)
 	p_fsync__cnt = 0;
 	seed_packbuilder();
 	git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL);
-	cl_assert_equal_sz(2, p_fsync__cnt);
+	cl_assert_equal_sz(4, p_fsync__cnt);
 }
 
 static int foreach_cb(void *buf, size_t len, void *payload)
diff --git a/tests/refs/create.c b/tests/refs/create.c
index 5c46fb3c2..e9630e853 100644
--- a/tests/refs/create.c
+++ b/tests/refs/create.c
@@ -329,7 +329,7 @@ void test_refs_create__fsyncs_when_requested(void)
 	git_oid_fromstr(&id, current_master_tip);
 	cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/fsync_test", &id, 0, "log message"));
 	git_reference_free(ref);
-	cl_assert_equal_i(2, p_fsync__cnt);
+	cl_assert_equal_i(4, p_fsync__cnt);
 
 	p_fsync__cnt = 0;
 
@@ -337,5 +337,5 @@ void test_refs_create__fsyncs_when_requested(void)
 	cl_git_pass(git_refdb_compress(refdb));
 	git_refdb_free(refdb);
 
-	cl_assert_equal_i(1, p_fsync__cnt);
+	cl_assert_equal_i(2, p_fsync__cnt);
 }