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

(+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:

quoted

quoted

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().

quoted

quoted

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).

quoted

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 <redacted>
---
 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.