Re: [PATCH] apply: support case-only renames in case-insensitive filesystems

From: Tao Klerks <tao@klerks.biz>
Date: Jun 14, 2022, 06:22 UTC
Message-ID: <CAPMMpogcm36pd7fjvG64G7Vg29arukF-wzOKYbNYG9NOpVCXvQ@mail.gmail.com>
In-Reply-To: <xmqqedzt8nfq.fsf@gitster.g>

On Mon, Jun 13, 2022 at 1:35 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > ...  I
> > do not think in_fn_table() pays attention to "ignore_case" option,
> > so there may be an existing bug there already, regardless of the
> > problem you are trying to address with your patch.
> >
> > And I wonder if doing case-insensitive match in in_fn_table() lets
> > us cover this new case as well as "fixing" the existing issue.
>
> While I still think in_fn_table() should be looked into for an
> existing case sensitivity bug, I think this one is different enough
> that in_fn_table() logic wouild not trigger for it, and a patch to
> add an extra piece of logic for renames is probably needed.
>

Having spent some time with this yesterday and today, I'm quite confident not only that you were right about the general case-sensitivity fix required here, but also that it fixes the case-only rename issue. It turns it (on case-insensitive filesystems) into a similar issue to the mode change, which is treated as a remove and add of the same file.

As you suggested, it is possible to construct "case-differing file swaps" which are not file swaps on a case-sensitive FS but are on a case-insensitive one, and without a fix these fail. The same (very small) change fixes both issues.

Show 9 quoted lines

> It might be sufficient to tighten the condition so that it triggers
> only to the case you wanted to handle, i.e. a rename between the
> same name.
>
>         else if (ignore_case && old_name && new_name &&
>                  !strcasecmp(old_name, new_name))
>
> (the "both names must be non-NULL" check is new).
>

This was my original tack, but I think it makes more sense to make the general fix and explain how it also handles this case.

New patch coming.

Thanks!