Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Tao Klerks <tao@klerks.biz>
To: Junio C Hamano <gitster@pobox.com>
Cc: Tao Klerks via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] apply: support case-only renames in case-insensitive filesystems
Date: Tue, 14 Jun 2022 08:22:37 +0200	[thread overview]
Message-ID: <CAPMMpogcm36pd7fjvG64G7Vg29arukF-wzOKYbNYG9NOpVCXvQ@mail.gmail.com> (raw)
In-Reply-To: <xmqqedzt8nfq.fsf@gitster.g>

On Mon, Jun 13, 2022 at 1:35 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.

> 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!

  reply	other threads:[~2022-06-14  6:22 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-11 17:03 [PATCH] apply: support case-only renames in case-insensitive filesystems Tao Klerks via GitGitGadget
2022-06-11 19:17 ` Junio C Hamano
2022-06-12 23:35   ` Junio C Hamano
2022-06-14  6:22     ` Tao Klerks [this message]
2022-06-15 11:24       ` Tao Klerks
2022-06-14  5:13   ` Tao Klerks
2022-06-18  0:45     ` Junio C Hamano
2022-06-18 15:34       ` Tao Klerks
2022-06-12 23:30 ` Junio C Hamano
2022-06-13 18:12   ` Junio C Hamano
2022-06-14  6:26     ` Tao Klerks
2022-06-14  6:16   ` Tao Klerks
2022-06-19 16:10 ` [PATCH v2 0/3] RFC: " Tao Klerks via GitGitGadget
2022-06-19 16:10   ` [PATCH v2 1/3] t4141: test "git apply" with core.ignorecase Junio C Hamano via GitGitGadget
2022-06-19 16:10   ` [PATCH v2 2/3] reset: new failing test for reset of case-insensitive duplicate in index Tao Klerks via GitGitGadget
2022-06-19 16:10   ` [PATCH v2 3/3] apply: support case-only renames in case-insensitive filesystems Tao Klerks via GitGitGadget
2022-10-10  4:09   ` [PATCH v2 0/3] RFC: " Tao Klerks
2023-05-28  9:59   ` [PATCH v3 0/3] " Tao Klerks via GitGitGadget
2023-05-28  9:59     ` [PATCH v3 1/3] t4142: test "git apply" with core.ignorecase Junio C Hamano via GitGitGadget
2023-05-28  9:59     ` [PATCH v3 2/3] reset: new failing test for reset of case-insensitive duplicate in index Tao Klerks via GitGitGadget
2023-05-28  9:59     ` [PATCH v3 3/3] apply: support case-only renames in case-insensitive filesystems Tao Klerks via GitGitGadget

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=CAPMMpogcm36pd7fjvG64G7Vg29arukF-wzOKYbNYG9NOpVCXvQ@mail.gmail.com \
    --to=tao@klerks.biz \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).