Subject: [PATCH v2 0/2] Reduce explicit sleep calls in t7063 untracked cache tests

From: "Tao Klerks via GitGitGadget" <[email-masked]>
Date: Tue, 01 Mar 2022 09:45:08 +0000

As noted in a recent proposed patch to t/t7519-status-fsmonitor.sh, a number
of test cases in t\t7063-status-untracked-cache.sh explicitly sleep a
second, in order to avoid the untracked cache content being invalidated by
an mtime race condition.

Even though it's only 9 seconds of sleeping that can be straightforwardly
replaced, it seems worth fixing if possible.

Replace sleep calls with backdating of filesystem changes, but first fix the
test-tool chmtime functionality to work for directories in Windows.

I do have a question to the list here: Do mingw.c changes need to be
upstreamed somewhere? I don't understand the exact relationship between this
file and the MinGW project.

Tao Klerks (2):
  t/helper/test-chmtime: update mingw to support chmtime on directories
  t7063: mtime-mangling instead of delays in untracked cache testing

 compat/mingw.c                    | 21 +++++++++++++++++----
 t/t7063-status-untracked-cache.sh | 27 +++++++++++++++------------
 2 files changed, 32 insertions(+), 16 deletions(-)

base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1166%2FTaoK%2Ftaok-untracked-cache-testing-remote-waits-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1166/TaoK/taok-untracked-cache-testing-remote-waits-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1166

Range-diff vs v1:

 1:  76b6216281e ! 1:  7cdef0ad5fb t/helper/test-chmtime: update mingw to support chmtime on directories
     @@ Commit message
          The mingw_utime implementation in mingw.c does not support
          directories. This means that "test-tool chmtime" fails on Windows when
          targeting directories. This has previously been noted and sidestepped
     -    by Jeff Hostetler, in "t/helper/test-chmtime: skip directories
     -    on Windows" in the "Builtin FSMonitor Part 2" work.
     +    temporarily by Jeff Hostetler, in "t/helper/test-chmtime: skip
     +    directories on Windows" in the "Builtin FSMonitor Part 2" work, but
     +    not yet fixed.
      
          It would make sense to backdate file and folder changes in untracked
          cache tests, to avoid needing to insert explicit delays/pauses in the
          tests.
      
     -    Add support for directory date manipulation in mingw_utime by calling
     -    CreateFileW() explicitly to create the directory handle, and
     -    CloseHandle() to close it.
     +    Add support for directory date manipulation in mingw_utime by
     +    replacing the file-oriented _wopen() call with the
     +    directory-supporting CreateFileW() windows API explicitly.
      
          Signed-off-by: Tao Klerks <[email-masked]>
      
       ## compat/mingw.c ##
     -@@ compat/mingw.c: int mingw_utime (const char *file_name, const struct utimbuf *times)
     - 	int fh, rc;
     +@@ compat/mingw.c: static inline void time_t_to_filetime(time_t t, FILETIME *ft)
     + int mingw_utime (const char *file_name, const struct utimbuf *times)
     + {
     + 	FILETIME mft, aft;
     +-	int fh, rc;
     ++	int rc;
       	DWORD attrs;
       	wchar_t wfilename[MAX_PATH];
      +	HANDLE osfilehandle;
     @@ compat/mingw.c: int mingw_utime (const char *file_name, const struct utimbuf *ti
       	}
       
      -	if ((fh = _wopen(wfilename, O_RDWR | O_BINARY)) < 0) {
     --		rc = -1;
     --		goto revert_attrs;
     -+	if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
     -+		fh = 0;
     -+		osfilehandle = CreateFileW(wfilename,
     -+					   0x100 /*FILE_WRITE_ATTRIBUTES*/,
     -+					   0 /*FileShare.None*/,
     -+					   NULL,
     -+					   OPEN_EXISTING,
     -+					   FILE_FLAG_BACKUP_SEMANTICS,
     -+					   NULL);
     -+		if (osfilehandle == INVALID_HANDLE_VALUE) {
     -+			errno = err_win_to_posix(GetLastError());
     -+			rc = -1;
     -+			goto revert_attrs;
     -+		}
     -+	} else {
     -+		if ((fh = _wopen(wfilename, O_RDWR | O_BINARY)) < 0) {
     -+			rc = -1;
     -+			goto revert_attrs;
     -+		}
     -+		osfilehandle = (HANDLE)_get_osfhandle(fh);
     ++	osfilehandle = CreateFileW(wfilename,
     ++				   0x100 /*FILE_WRITE_ATTRIBUTES*/,
     ++				   0 /*FileShare.None*/,
     ++				   NULL,
     ++				   OPEN_EXISTING,
     ++				   attrs & FILE_ATTRIBUTE_DIRECTORY ?
     ++					FILE_FLAG_BACKUP_SEMANTICS : 0,
     ++				   NULL);
     ++	if (osfilehandle == INVALID_HANDLE_VALUE) {
     ++		errno = err_win_to_posix(GetLastError());
     + 		rc = -1;
     + 		goto revert_attrs;
       	}
     - 
     - 	if (times) {
      @@ compat/mingw.c: int mingw_utime (const char *file_name, const struct utimbuf *times)
       		GetSystemTimeAsFileTime(&mft);
       		aft = mft;
     @@ compat/mingw.c: int mingw_utime (const char *file_name, const struct utimbuf *ti
       		rc = 0;
      -	close(fh);
      +
     -+	if (fh)
     -+		close(fh);
     -+	else if (osfilehandle)
     ++	if (osfilehandle != INVALID_HANDLE_VALUE)
      +		CloseHandle(osfilehandle);
       
       revert_attrs:
 2:  a1806c56333 ! 2:  3e3c9c7faac t7063: mtime-mangling instead of delays in untracked cache testing
     @@ t/t7063-status-untracked-cache.sh: sync_mtime () {
       	find . -type d -exec ls -ld {} + >/dev/null
       }
       
     -+chmmtime_worktree_root () {
     ++chmtime_worktree_root () {
      +	# chmtime doesnt handle relative paths on windows, so need
      +	# to "hardcode" a reference to the worktree folder name.
     -+	cd .. &&
     -+	test-tool chmtime $1 worktree &&
     -+	cd worktree
     ++	test-tool -C .. chmtime $1 worktree
      +}
      +
       avoid_racy() {
     @@ t/t7063-status-untracked-cache.sh: test_expect_success 'setup' '
       	touch one two three done/one dtwo/two dthree/three &&
      +	test-tool chmtime =-300 one two three done/one dtwo/two dthree/three &&
      +	test-tool chmtime =-300 done dtwo dthree &&
     -+	chmmtime_worktree_root =-300 &&
     ++	chmtime_worktree_root =-300 &&
       	git add one two done/one &&
       	: >.git/info/exclude &&
       	git update-index --untracked-cache &&
     @@ t/t7063-status-untracked-cache.sh: test_expect_success 'create/modify files, som
      -	rm base &&
      +	test-tool chmtime =-180 done/two done/three done/four done/five done &&
      +	# we need to ensure that the root dir is touched (in the past);
     -+	chmmtime_worktree_root =-180 &&
     ++	chmtime_worktree_root =-180 &&
       	sync_mtime
       '

-- 
gitgitgadget