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
David Turner <dturner@twopensource.com>
Date
May 7, 2014, 18:01 UTC
Message-ID
<1399485666.11843.68.camel@stross>
In-Reply-To
<xmqqha51bhxg.fsf@gitster.dls.corp.google.com>
On Wed, 2014-05-07 at 10:46 -0700, Junio C Hamano wrote:
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?

Yes, this is indeed a fair rephrasing. In fact, the entire zero-check should not be necessary, as POSIX requires that the st_ino field has a "meaningful" value. So in the case that this ever runs into a problem, we ought to wrap the lstat call with a compatibility layer anyway.

But maybe there is an OS I'm not thinking of which fills in st_ino with something else?

Previous: Junio C HamanoNext: Johannes Sixt
Message 11 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.