git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] t/helper/test-chmtime: update mingw to support chmtime on directories

From
JHJeff Hostetler <git@jeffhostetler.com>
Date
Feb 28, 2022, 15:27 UTC
Message-ID
<b5fb21f5-4aa6-fa36-a8f6-abf82b543c1d@jeffhostetler.com>
In-Reply-To
<76b6216281e3463821e650495f3090c677905f73.1646041236.git.gitgitgadget@gmail.com>
On 2/28/22 4:40 AM, Tao Klerks via GitGitGadget wrote:
Show 30 quoted lines
> From: Tao Klerks <tao@klerks.biz>
> 
> 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.
> 
> 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.
> 
> Signed-off-by: Tao Klerks <tao@klerks.biz>
> ---
>   compat/mingw.c | 33 ++++++++++++++++++++++++++++-----
>   1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 03af369b2b9..2284ea90511 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -964,6 +964,8 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
>   	int fh, rc;
>   	DWORD attrs;
>   	wchar_t wfilename[MAX_PATH];
> +	HANDLE osfilehandle;

I'd be more comfortable initializing this variable to INVALID_HANDLE_VALUE.

Show 13 quoted lines
> +
>   	if (xutftowcs_path(wfilename, file_name) < 0)
>   		return -1;
>   
> @@ -975,9 +977,26 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
>   		SetFileAttributesW(wfilename, attrs & ~FILE_ATTRIBUTE_READONLY);
>   	}
>   
> -	if ((fh = _wopen(wfilename, O_RDWR | O_BINARY)) < 0) {
> -		rc = -1;
> -		goto revert_attrs;
> +	if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
> +		fh = 0;
and here initializing fh = -1.
> +		osfilehandle = CreateFileW(wfilename,
> +					   0x100 /*FILE_WRITE_ATTRIBUTES*/,
> +					   0 /*FileShare.None*/,

Is there a reason that you're not using the #define's here? I assume you ran into a header file problem or something, but there are other symbols used nearby, so I'm not sure.

Show 34 quoted lines
> +					   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);
>   	}
>   
>   	if (times) {
> @@ -987,12 +1006,16 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
>   		GetSystemTimeAsFileTime(&mft);
>   		aft = mft;
>   	}
> -	if (!SetFileTime((HANDLE)_get_osfhandle(fh), NULL, &aft, &mft)) {
> +	if (!SetFileTime(osfilehandle, NULL, &aft, &mft)) {
>   		errno = EINVAL;
>   		rc = -1;
>   	} else
>   		rc = 0;
> -	close(fh);
> +
> +	if (fh)
> +		close(fh);
> +	else if (osfilehandle)
> +		CloseHandle(osfilehandle);
And then this becomes:
	if (fh != -1)
		close(fh)
	else if (osfilehandle != INVALID_HANDLE_VALUE)
		Closehandle(osfilehandle);
Which I think makes it more clear that we'll properly close the handle.
>   
>   revert_attrs:
>   	if (attrs != INVALID_FILE_ATTRIBUTES &&
> 

An alternative solution would be to replace the _wopen() with a call to CreateFileW() without the backup flag for non-directories and just convert the whole routine to use HANDLE's rather fd's.

Thanks Jeff

Previous: Tao Klerks via GitGitGadgetNext: Junio C Hamano
Message 3 of 30 in “Reduce explicit sleep calls in t7063 untracked cache tests”
  1. 0/2 Reduce explicit sleep calls in t7063 untracked cache testsTao Klerks via GitGitGadget, Feb 28, 2022
  2. 1/2 t/helper/test-chmtime: update mingw to support chmtime on directoriesTao Klerks via GitGitGadget, Feb 28, 2022
  3. Jeff HostetlerFeb 28, 2022
  4. Junio C HamanoFeb 28, 2022
  5. Tao KlerksMar 1, 2022
  6. Junio C HamanoFeb 28, 2022
  7. Tao KlerksMar 1, 2022
  8. 2/2 t7063: mtime-mangling instead of delays in untracked cache testingTao Klerks via GitGitGadget, Feb 28, 2022
  9. Junio C HamanoFeb 28, 2022
  10. Tao KlerksMar 1, 2022
  11. Ævar Arnfjörð BjarmasonFeb 28, 2022
  12. Jeff HostetlerFeb 28, 2022
  13. 0/2 Reduce explicit sleep calls in t7063 untracked cache testsTao Klerks via GitGitGadget, Mar 1, 2022
  14. 1/2 t/helper/test-chmtime: update mingw to support chmtime on directoriesTao Klerks via GitGitGadget, Mar 1, 2022
  15. Jeff HostetlerMar 1, 2022
  16. Tao KlerksMar 1, 2022
  17. 2/2 t7063: mtime-mangling instead of delays in untracked cache testingTao Klerks via GitGitGadget, Mar 1, 2022
  18. Junio C HamanoMar 1, 2022
  19. Tao KlerksMar 1, 2022
  20. Tao KlerksMar 1, 2022
  21. Junio C HamanoMar 1, 2022
  22. 0/2 Reduce explicit sleep calls in t7063 untracked cache testsTao Klerks via GitGitGadget, Mar 2, 2022
  23. 1/2 t/helper/test-chmtime: update mingw to support chmtime on directoriesTao Klerks via GitGitGadget, Mar 2, 2022
  24. Johannes SchindelinMar 9, 2022
  25. 2/2 t7063: mtime-mangling instead of delays in untracked cache testingTao Klerks via GitGitGadget, Mar 2, 2022
  26. Johannes SchindelinMar 9, 2022
  27. Tao KlerksMar 5, 2022
  28. Junio C HamanoMar 6, 2022
  29. Tao KlerksMar 7, 2022
  30. Junio C HamanoMar 7, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.