Re: [PATCH] t1410: fix breakage on case-insensitive filesystems

[Johannes Sixt <j6t@kdbg.org>]

(+cc msysgit)

Am 13.11.2014 um 10:08 schrieb Jeff King:
> On Thu, Nov 13, 2014 at 09:50:24AM +0100, Johannes Sixt wrote:
> 
>>> That looks more like it is failing the actual test (i.e., the creation
>>> of branch "one" when there is cruft in the reflog). My guess is that
>>> calling open() on a directory is giving us EACCES instead of EISDIR. Can
>>> you verify that?

That is indeed the case. It's an ancient bug in our wrapper
mingw_open().

>>> If that is the case, then this isn't a new breakage, I think, but just
>>> code we weren't previously exercising. It would be interesting to know
>>> whether:
>>>
>>>   git config core.logallrefupdates true
>>>   git branch one/two
>>>   git branch -d one/two
>>>   git branch one
>>>
>>> works (even without my patch). If so, then there's probably something
>>> else going on.
>>
>> Don't know what you mean with "my patch" (the one I was responding to
>> touches only t1410).
> 
> The patch you are responding to is a fix-up for 9233887, which tweaked
> the code and added those tests in the first place (I doubt it would work
> for you, though, as it has a problem on case-insensitive filesystems).
> 
>> But the sequence works as expected with a version built
>> in September:
> 
> Hmph. So that would mean my theory is not right. Or maybe I am not
> accounting for something else in my analysis.
> 
> I guess it is odd that the test right before the failing one passes (it
> is basically that same sequence, with reflogs turned on for both
> operations), which implies that we are properly getting EISDIR. The only
> difference in the failing test is that reflogs are turned off for the
> "git branch one" operation. But I cannot see why that would be broken if
> the other one passes.

Not a comment, on this paragraph of yours, but while I was walking
through the code with gdb, I was wondering why the reflog directory is
being touched at all when core.logallrefupdates is off (in
log_ref_setup via log_ref_write). With the patch below I now get the
same unlink warning as on Linux.

--- 8< ---
Subject: [PATCH] Windows: correct detection of EISDIR in mingw_open()

According to the Linux open(2) man page, open() returns EISDIR if a
directory was attempted to be opened for writing. Our emulation in
mingw_open() does not get this right: it checks only for O_CREAT. Fix
it to check for one of the write flags.

This fixes a failure in reflog handling, which opens files with
O_APPEND|O_WRONLY, but without O_CREAT, and expects EISDIR when the
named file happens to be a directory.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 2ee3fe3..fc64b73 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -312,7 +312,7 @@ int mingw_open (const char *filename, int oflags, ...)
 		return -1;
 	fd = _wopen(wfilename, oflags, mode);
 
-	if (fd < 0 && (oflags & O_CREAT) && errno == EACCES) {
+	if (fd < 0 && (oflags & (O_WRONLY|O_RDWR)) && errno == EACCES) {
 		DWORD attrs = GetFileAttributesW(wfilename);
 		if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY))
 			errno = EISDIR;
-- 
2.0.0.12.gbcf935e

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.