From: Stefan Sperling <stsp@stsp.name>
Subject: Re: new gotd regression test: test_repo_write_empty
To: Mikhail <mp39590@gmail.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 8 Nov 2022 16:36:13 +0100

On Tue, Nov 08, 2022 at 05:19:23PM +0300, Mikhail wrote:
> On Mon, Nov 07, 2022 at 09:40:11PM +0100, Stefan Sperling wrote:
> > This tests gotd by sending to empty repositories. Testing this use
> > case this uncovered a couple of bugs in several places. I fixed
> > some bugs directly on the main branch, and some bugs have outstanding
> > fixes on this list I sent earlier today.
> > 
> > With all those changes, this new test is passing.
> > 
> > ok? (will commit on top of all the other changes once they are in)
>  
> Sometimes the test fails, tgzs attached.

Thanks, I could reproduce the problem as well by running the test in a loop.
This patch fixes the problem for me:

 use sub-second precision when checking for objects/pack/ modification
 
 Convert from st.m_time (second-precision time_t) to st.m_tim (struct timespec).
 To compensate for the potential case where a filesystem provides resolution
 in seconds only, always read the directory if no pack files are known to exist.
 
 Otherwise, there is a race condition when gotd repo_write creates a new pack
 and a request arrives for repo_read shortly after. Caught by a regression test
 for gotd on empty repositories. Test failure pointed out by Mikhail.
 
diff 59b6369a4820d240b419228dc1aa614add1ecee1 cb48353b4fece9d276dba2c64d22cbd25af8735c
commit - 59b6369a4820d240b419228dc1aa614add1ecee1
commit + cb48353b4fece9d276dba2c64d22cbd25af8735c
blob - cd9aa4370a65e5d58169777bb0760f75b984b0b1
blob + e78dd4fb4cada73349067798a377d8d17194da2e
--- gotd/privsep_stub.c
+++ gotd/privsep_stub.c
@@ -15,6 +15,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/time.h>
 #include <sys/queue.h>
 #include <sys/tree.h>
 #include <sys/uio.h>
blob - ced490e96ad476ac78d59ed49d235863982116c3
blob + b8032ffd3ffe9dab29beaf3cc57c39a6ef957859
--- lib/got_lib_repository.h
+++ lib/got_lib_repository.h
@@ -65,7 +65,7 @@ struct got_repository {
 	int gitdir_fd;
 
 	struct got_pathlist_head packidx_paths;
-	time_t pack_path_mtime;
+	struct timespec pack_path_mtime;
 
 	/* The pack index cache speeds up search for packed objects. */
 	struct got_packidx *packidx_cache[GOT_PACK_CACHE_SIZE];
blob - 0f29cc09331dd7d207bd1ec47e0b2310708660ee
blob + e76312afb71dc950015ded8344d412b185adb00c
--- lib/read_gitconfig_privsep.c
+++ lib/read_gitconfig_privsep.c
@@ -14,6 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <sys/time.h>
 #include <sys/types.h>
 #include <sys/tree.h>
 #include <sys/socket.h>
blob - 0cab79b8b8f5f06becd7bc5a485ab1f33f7e022e
blob + 5ede5726ac196d5b3ba820a830a44852e453b174
--- lib/repository.c
+++ lib/repository.c
@@ -1150,7 +1150,9 @@ refresh_packidx_paths(struct got_repository *repo)
 			err = got_error_from_errno2("stat", objects_pack_dir);
 			goto done;
 		}
-	} else if (sb.st_mtime != repo->pack_path_mtime) {
+	} else if (TAILQ_EMPTY(&repo->packidx_paths) ||
+	    sb.st_mtim.tv_sec != repo->pack_path_mtime.tv_sec ||
+	    sb.st_mtim.tv_nsec != repo->pack_path_mtime.tv_nsec) {
 		purge_packidx_paths(&repo->packidx_paths);
 		err = got_repo_list_packidx(&repo->packidx_paths, repo);
 		if (err)
@@ -1277,7 +1279,8 @@ got_repo_list_packidx(struct got_pathlist_head *packid
 		err = got_error_from_errno("fstat");
 		goto done;
 	}
-	repo->pack_path_mtime = sb.st_mtime;
+	repo->pack_path_mtime.tv_sec = sb.st_mtim.tv_sec;
+	repo->pack_path_mtime.tv_nsec = sb.st_mtim.tv_nsec;
 
 	while ((dent = readdir(packdir)) != NULL) {
 		if (!got_repo_is_packidx_filename(dent->d_name, dent->d_namlen))