mingw_rename: do support directory renames

[dscho]

This is not quite a critical bug fix for Git because (unlike Git for Windows) it attempts _wrename() first. If that succeeds, we'll not bother with the POSIX semantics.

However, Git for Windows knows how to deal with symbolic links, and _wrename() does not work for them. Therefore, that _wrename() call was patched out there and we rely on the native Win32 API call to SetFileInformationByHandle() to rename files and directories.

It is that latter part that is at heart of this here bug fix: To be able to call SetFileInformationByHandle(), we need a handle, and CreateFileW() is what we use, for files, and crucially, also for directories.

So while it is not critical for Git to take this patch, it still is important because that _wrename() call can fail, even when renaming directories, and then we want the fallback to fail not because we tried to obtain a handle using incorrect flags, but only because the actual rename operation failed.

This patch is based on ps/mingw-rename.

Cc: Patrick Steinhardt [email protected]

[dscho]

/submit

[gitgitgadget]

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1839/dscho/ps/mingw-rename-followup-v1

To fetch this version to local tag pr-1839/dscho/ps/mingw-rename-followup-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1839/dscho/ps/mingw-rename-followup-v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

>     This is not quite a critical bug fix for Git because (unlike Git for
>     Windows) it attempts _wrename() first. If that succeeds, we'll not
>     bother with the POSIX semantics.
>     
>     However, Git for Windows knows how to deal with symbolic links, and
>     _wrename() does not work for them. Therefore, that _wrename() call was
>     patched out there and we rely on the native Win32 API call to
>     SetFileInformationByHandle() to rename files and directories.
>     
>     It is that latter part that is at heart of this here bug fix: To be able
>     to call SetFileInformationByHandle(), we need a handle, and
>     CreateFileW() is what we use, for files, and crucially, also for
>     directories.
>     
>     So while it is not critical for Git to take this patch, it still is
>     important because that _wrename() call can fail, even when renaming
>     directories, and then we want the fallback to fail not because we tried
>     to obtain a handle using incorrect flags, but only because the actual
>     rename operation failed.
>     
>     This patch is based on ps/mingw-rename.

Thanks.

As this will be part of GfW 2.48 anyway (I presume), let me take it
and include it in my tree to keep the divergence between our trees
small.

[gitgitgadget]

This patch series was integrated into seen via git@48a7879.

[gitgitgadget]

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Tue, Dec 17, 2024 at 12:52:04PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/compat/mingw.c b/compat/mingw.c
> index c4320769db6..e8f491d03a7 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2273,7 +2273,7 @@ repeat:
>  
>  		old_handle = CreateFileW(wpold, DELETE,
>  					 FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
> -					 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> +					 NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
>  		if (old_handle == INVALID_HANDLE_VALUE) {
>  			errno = err_win_to_posix(GetLastError());
>  			return -1;

Okay. I saw that FILE_FLAG_BACKUP_SEMANTICS was used in several other
users of `CreateFileW()` in this context, but I couldn't find good
enough information around what it is actually doing anywhere. The
explanation makes sense to me, so thanks for the follow-up fix!

Patrick

[gitgitgadget]

This branch is now known as js/mingw-rename-fix.

[gitgitgadget]

This patch series was integrated into seen via git@ec5de6a.

[gitgitgadget]

This patch series was integrated into next via git@57dc9a0.