git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Elijah Newren <newren@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Adam Dinwoodie <adam@dinwoodie.org>,
	David Turner <dturner@twitter.com>,
	David Turner <dturner@twopensource.com>
Subject: Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals
Date: Fri, 24 Nov 2017 15:04:05 -0500	[thread overview]
Message-ID: <CAPig+cTtT2xLHy6gE9fW9G_KdzGTNEfgwz7NUOUwHxGKOkiKtA@mail.gmail.com> (raw)
In-Reply-To: <20171124195901.2581-1-newren@gmail.com>

On Fri, Nov 24, 2017 at 2:59 PM, Elijah Newren <newren@gmail.com> wrote:
> In commit ae352c7f3 (merge-recursive.c: fix case-changing merge bug,
> 2014-05-01), it was observed that removing files could be problematic on
> case insensitive file systems, because we could end up removing files
> that differed in case only rather than deleting the intended file --
> something that happened when files were renamed on one branch in a way
> that differed only in case.  To avoid that problem, that commit added
> logic to avoid removing files other than the one intended, rejecting the
> removal if the files differed only in case.
>
> Unfortunately, the logic it used didn't fully implement that condition as
> stated above; instead it merely checked that a case-insensitive lookup of
> the file that was requested resulted in finding a file in the index at
> stage 0, not that the file found in the index actually differed in case.
> Alternatively, one could view the implementation as making an implicit
> assumption that the file we actually wanted to remove would never appear
> in the index with a stage of 0, and thus that if we found a file with our
> lookup, that it had to be a different file (but different in case only).
>
> The net result of this implementation is that it can ignore more requests
> than it should, leaving a file around in the working copy that should
> have been removed.  Make sure that the file found in the index actually
> differs in case before silently ignoring the request to remove the file.
>
> ---

Missing sign-off.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index b48b15a6f..100fb913f 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -646,7 +646,7 @@ static int remove_file(struct merge_options *o, int clean,
>                 if (ignore_case) {
>                         struct cache_entry *ce;
>                         ce = cache_file_exists(path, strlen(path), ignore_case);
> -                       if (ce && ce_stage(ce) == 0)
> +                       if (ce && ce_stage(ce) == 0 && strcmp(path, ce->name))
>                                 return 0;
>                 }
>                 if (remove_path(path))
> --
> 2.11.0

  reply	other threads:[~2017-11-24 20:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-24 19:59 Elijah Newren
2017-11-24 20:04 ` Eric Sunshine [this message]
2017-11-24 20:29   ` Elijah Newren
2017-11-25  3:29 ` Junio C Hamano
2017-11-25 22:35   ` Elijah Newren
2017-11-26  2:32     ` Junio C Hamano
2017-11-27  3:40 ` Junio C Hamano
2017-11-27 16:40   ` Elijah Newren
2017-11-27 23:39     ` Junio C Hamano
2017-11-28  1:02       ` Elijah Newren
2019-03-04 23:07 Woody Woodman
2019-03-06 14:23 ` Johannes Schindelin

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=CAPig+cTtT2xLHy6gE9fW9G_KdzGTNEfgwz7NUOUwHxGKOkiKtA@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=adam@dinwoodie.org \
    --cc=dturner@twitter.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --subject='Re: [PATCH] merge-recursive: ignore_case shouldn'\''t reject intentional removals' \
    /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

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).