Re: [PATCH v2 1/1] git mv foo FOO ; git mv foo bar gave an assert

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2021, 19:21 UTC
Message-ID: <xmqqzgzmptmy.fsf@gitster.c.googlers.com>
In-Reply-To: <20210301170536.12265-1-tboegi@web.de>

tboegi@web.de writes:

Show 5 quoted lines

> From: Torsten Bögershausen <tboegi@web.de>
>
> The following sequence, on a case-insensitive file system,
> (strictly speeking with core.ignorecase=true)
> leads to an assertion, and leaves .git/index.lock behind.

"an assertion failure", I presume?

Show 20 quoted lines

>
> git init
> echo foo >foo
> git add foo
> git mv foo FOO
> git mv foo bar
>
> This regression was introduced in Commit 9b906af657,
> "git-mv: improve error message for conflicted file"
>
> The bugfix is to change the "file exist case-insensitive in the index"
> into the correct "file exist (case-sensitive) in the index".
> This avoids the "assert".
>
> Reported-By: Dan Moseley <Dan.Moseley@microsoft.com>
>
> This fixes
> https://github.com/git-for-windows/git/issues/2920
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>

By breaking the list of trailers with an unrelated comment in the middle, the "Reported-by" won't be recognized as a trailer. We'd want to credit non-authorship contribution in the release notes starting in the upcoming release, and this will start mattering. Please fix it.

Show 6 quoted lines

> @@ -221,7 +221,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  				}
>  				argc += last - first;
>  			}
> -		} else if (!(ce = cache_file_exists(src, length, ignore_case))) {
> +		} else if (!(ce = cache_file_exists(src, length, 0))) {

Good finding.

Before the problematic patch, this used to be

               } else if (cache_name_pos(src, length) < 0)

I wonder if we should revert the change to use cache_file_exists() in the first place (and rewrite the subsequent use of ce to match), though.

Thanks.