git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>,
	David Turner <dturner@twopensource.com>
Cc: git@vger.kernel.org, David Turner <dturner@twitter.com>
Subject: Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems
Date: Thu, 08 May 2014 08:37:43 +0200	[thread overview]
Message-ID: <536B2637.2060808@viscovery.net> (raw)
In-Reply-To: <xmqqha51bhxg.fsf@gitster.dls.corp.google.com>

Am 5/7/2014 19:46, schrieb Junio C Hamano:
> 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

  parent reply	other threads:[~2014-05-08  6:38 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-29 19:02 Bug: Case-insensitive filesystems can cause merge and checkout problems David Turner
2014-05-02  0:21 ` [PATCH] merge-recursive.c: Fix case-changing merge bug David Turner
2014-05-06 17:07   ` Junio C Hamano
2014-05-06 17:36     ` David Turner
2014-05-06 19:46       ` Junio C Hamano
2014-05-06 22:59         ` [PATCH 1/2] merge-recursive.c: Fix case-changing merge dturner
2014-05-06 22:59           ` [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems dturner
2014-05-07  6:17             ` Johannes Sixt
2014-05-07 16:42               ` David Turner
2014-05-07 17:46                 ` Junio C Hamano
2014-05-07 18:01                   ` David Turner
2014-05-08  6:37                   ` Johannes Sixt [this message]
2014-05-08  8:55                     ` Torsten Bögershausen
2014-05-08 17:23                       ` [PATCH 0/2] " dturner
2014-05-08 17:23                         ` [PATCH 1/2] merge-recursive.c: Fix case-changing merge dturner
2014-05-08 19:45                           ` Junio C Hamano
2014-05-08 17:23                         ` [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems dturner
2014-05-08 19:54                           ` Junio C Hamano
2014-05-08 20:40                             ` David Turner
2014-05-08 20:55                               ` Junio C Hamano
2014-05-08  1:22             ` brian m. carlson
2014-05-07 18:01           ` [PATCH 1/2] merge-recursive.c: Fix case-changing merge Junio C Hamano
2014-05-07 18:13             ` Jonathan Nieder
2014-05-07 20:53               ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2014-05-08 20:48 [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems Thomas Braun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=536B2637.2060808@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=dturner@twitter.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).