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

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

From
Johannes Sixt <j6t@kdbg.org>
Date
Nov 14, 2014, 19:11 UTC
Message-ID
<546653D6.7040505@kdbg.org>
In-Reply-To
<20141113090832.GA8329@peff.net>
(+cc msysgit)
Am 13.11.2014 um 10:08 schrieb Jeff King:
Show 6 quoted lines
> 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().

Show 31 quoted lines
>>> 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.
Previous: Junio C HamanoNext: Jeff King
Message 20 of 26 in “Test failure”
  1. Michael BlumeNov 8, 2014
  2. Jeff KingNov 9, 2014
  3. t1410: fix breakage on case-insensitive filesystemsJeff King, Nov 9, 2014
  4. Michael BlumeNov 9, 2014
  5. Junio C HamanoNov 9, 2014
  6. Jeff KingNov 10, 2014
  7. Junio C HamanoNov 10, 2014
  8. Jeff KingNov 10, 2014
  9. Torsten BögershausenNov 9, 2014
  10. Michael BlumeNov 9, 2014
  11. Torsten BögershausenNov 9, 2014
  12. Michael BlumeNov 10, 2014
  13. Junio C HamanoNov 10, 2014
  14. Jeff KingNov 10, 2014
  15. Johannes SixtNov 12, 2014
  16. Jeff KingNov 12, 2014
  17. Johannes SixtNov 13, 2014
  18. Jeff KingNov 13, 2014
  19. Junio C HamanoNov 13, 2014
  20. Johannes SixtNov 14, 2014
  21. Jeff KingNov 14, 2014
  22. Junio C HamanoNov 14, 2014
  23. Andreas SchwabNov 14, 2014
  24. Johannes SixtNov 15, 2014
  25. Windows: correct detection of EISDIR in mingw_open()Johannes Sixt, Nov 16, 2014
  26. Michael BlumeNov 9, 2014

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.