git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] mingw_rename: do support directory renames
Date: Tue, 17 Dec 2024 12:52:04 +0000	[thread overview]
Message-ID: <pull.1839.git.1734439924842.gitgitgadget@gmail.com> (raw)

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In 391bceae435 (compat/mingw: support POSIX semantics for atomic
renames, 2024-10-27), we taught the `mingw_rename()` function to respect
POSIX semantics, but we did so only as a fallback after `_wrename()`
fails.

This hid a bug in the implementation that was not caught by Git's test
suite: The `CreateFileW()` function _can_ open handles to directories,
but not when asked to use the `FILE_ATTRIBUTE_NORMAL` flag, as that flag
only is allowed for files.

Let's fix this by using the common `FILE_FLAG_BACKUP_SEMANTICS` flag
that can be used for opening handles to directories, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    mingw_rename: do support directory renames
    
    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.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1839%2Fdscho%2Fps%2Fmingw-rename-followup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1839/dscho/ps/mingw-rename-followup-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1839

 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;

base-commit: 391bceae4350136a05d977573caeaa07059f2136
-- 
gitgitgadget

             reply	other threads:[~2024-12-17 12:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-17 12:52 Johannes Schindelin via GitGitGadget [this message]
2024-12-17 20:09 ` [PATCH] mingw_rename: do support directory renames Junio C Hamano
2024-12-18  6:00 ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1839.git.1734439924842.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).