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

Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

From
Johannes Sixt <j.sixt@viscovery.net>
Date
May 8, 2014, 06:37 UTC
Message-ID
<536B2637.2060808@viscovery.net>
In-Reply-To
<xmqqha51bhxg.fsf@gitster.dls.corp.google.com>
Am 5/7/2014 19:46, schrieb Junio C Hamano:
Show 24 quoted lines
> David Turner <dturner@twopensource.com> writes:
> 
>> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
>>>>  		} else if (cache_name_pos(src, length) < 0)
>>>>  			bad = _("not under version control");
>>>> -		else if (lstat(dst, &st) == 0) {
>>>> +		else if (lstat(dst, &dst_st) == 0 &&
>>>> +			 (src_st.st_ino != dst_st.st_ino ||
>>>> +			  (src_st.st_ino == 0 && strcasecmp(src, dst)))) {
>>>
>>> Don't do that. st_ino is zero on Windows only because we do not spend time
>>> to fill in the field. Don't use it as an indicator for a case-insensitive
>>> file system; zero may be a valid inode number on other systems.
>>
>> I don't think it is a problem if zero is a valid inode.  The only thing
>> that happens when there is a zero inode, is that we have to compare
>> filenames.  The inode check is just an optimization to avoid doing a
>> bunch of strcasecmp on systems that don't have to.
> 
> Am I correct to rephrase you that the code assumes that any
> filesystem that cannot give unique inum to different files will use
> 0 as the placeholder inum, so if src/dst share the same non-zero
> inum, it is guaranteed that is not a placeholder and we know they
> are different files without comparing the two paths?

"If src and dst share the same non-zero inum, it is guaranteed that it is not a placeholder and we know they are the same file" is more correct.

What if both inums are zero? Can this happen on any sane POSIX system? I don't know, but my gut feeling is that inode zero is too special to be allocated for files or directories.

In that case, it is safe to assume that the st_ino field is just a placeholder when it is zero, and we have to compare the file name. Then we can either assume that this happens only for our emulation layer on MinGW (and the comparison can be case-insensitive) or choose the comparison mode based on core.ignorecase. This patch does the former, but I think we should do the latter.

-- Hannes
Previous: David TurnerNext: Torsten Bögershausen
Message 12 of 24 in “Bug: Case-insensitive filesystems can cause merge and checkout problems”
  1. David TurnerApr 29, 2014
  2. merge-recursive.c: Fix case-changing merge bugDavid Turner, May 2, 2014
  3. Junio C HamanoMay 6, 2014
  4. David TurnerMay 6, 2014
  5. Junio C HamanoMay 6, 2014
  6. 1/2 merge-recursive.c: Fix case-changing merge.dturner@twopensource.com, May 6, 2014
  7. 2/2 ignorecase: Fix git mv on insensitive filesystemsdturner@twopensource.com, May 6, 2014
  8. Johannes SixtMay 7, 2014
  9. David TurnerMay 7, 2014
  10. Junio C HamanoMay 7, 2014
  11. David TurnerMay 7, 2014
  12. Johannes SixtMay 8, 2014
  13. Torsten BögershausenMay 8, 2014
  14. 0/2 Re: ignorecase: Fix git mv on insensitive filesystemsdturner@twopensource.com, May 8, 2014
  15. 1/2 merge-recursive.c: Fix case-changing merge.dturner@twopensource.com, May 8, 2014
  16. Junio C HamanoMay 8, 2014
  17. 2/2 ignorecase: Fix git mv on insensitive filesystemsdturner@twopensource.com, May 8, 2014
  18. Junio C HamanoMay 8, 2014
  19. David TurnerMay 8, 2014
  20. Junio C HamanoMay 8, 2014
  21. brian m. carlsonMay 8, 2014
  22. Junio C HamanoMay 7, 2014
  23. Jonathan NiederMay 7, 2014
  24. Junio C HamanoMay 7, 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.