[PATCH] compat/mingw.c: Teach mingw_rename() to replace read-only files

[Johannes Sixt <j.sixt@viscovery.net>]

From: Johannes Sixt <j6t@kdbg.org>

On POSIX, rename() can replace files that are not writable. On Windows,
however, read-only files cannot be replaced without additional efforts:
We have to make the destination writable first.

Since the situations where the destination is read-only are rare, we do not
make the destination writable on every invocation, but only if the first
try to rename a file failed with an "access denied" error.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Johannes Sixt schrieb:
> Nicolas Pitre schrieb:
>> On Wed, 19 Nov 2008, Johannes Sixt wrote:
>>> The unusual case is where you do this:
>>>
>>>  $ git rev-list -10 HEAD | git pack-objects foobar
>>>
>>> twice in a row: In this case the second invocation fails on Windows
>>> because the destination pack file already exists *and* is open. But not
>>> even git-repack does this even if it is called twice. OTOH, the test case
>>> *does* exactly this.
>> OK.... Well, despite my earlier assertion, I think the above should be a 
>> valid operation.

Would you please clarify which operation you exactly mean? As is written
above, or with .git/objects/pack/foobar instead like in the test case?

>> I'm looking at it now.  I'm therefore revoking my earlier ACK as well 
>> (better keep that test case alive).
> 
> Hold on a moment: When I tested the above sequence, I was fooled by a flaw
> in mingw_rename() (it doesn't replace read-only files). With that fixed...

Here is this fix.

-- Hannes

 compat/mingw.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index b534a8a..3dbe6a7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -819,6 +819,8 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz)
 #undef rename
 int mingw_rename(const char *pold, const char *pnew)
 {
+	DWORD attrs;
+
 	/*
 	 * Try native rename() first to get errno right.
 	 * It is based on MoveFile(), which cannot overwrite existing files.
@@ -830,12 +832,19 @@ int mingw_rename(const char *pold, const char *pnew)
 	if (MoveFileEx(pold, pnew, MOVEFILE_REPLACE_EXISTING))
 		return 0;
 	/* TODO: translate more errors */
-	if (GetLastError() == ERROR_ACCESS_DENIED) {
-		DWORD attrs = GetFileAttributes(pnew);
-		if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) {
+	if (GetLastError() == ERROR_ACCESS_DENIED &&
+	    (attrs = GetFileAttributes(pnew)) != INVALID_FILE_ATTRIBUTES) {
+		if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
 			errno = EISDIR;
 			return -1;
 		}
+		if ((attrs & FILE_ATTRIBUTE_READONLY) &&
+		    SetFileAttributes(pnew, attrs & ~FILE_ATTRIBUTE_READONLY)) {
+			if (MoveFileEx(pold, pnew, MOVEFILE_REPLACE_EXISTING))
+				return 0;
+			/* revert file attributes on failure */
+			SetFileAttributes(pnew, attrs);
+		}
 	}
 	errno = EACCES;
 	return -1;
-- 
1.6.0.4.1694.g07ac