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

[PATCH] mingw_rename: do support directory renames

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 17, 2024, 12:52 UTC
Message-ID
<pull.1839.git.1734439924842.gitgitgadget@gmail.com>
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
Next: Junio C Hamano
Message 1 of 3 in “mingw_rename: do support directory renames”
  1. mingw_rename: do support directory renamesJohannes Schindelin via GitGitGadget, Dec 17, 2024
  2. Junio C HamanoDec 17, 2024
  3. Patrick SteinhardtDec 18, 2024

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.