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

tboegi@web.de writes:

From: Torsten Bögershausen <redacted>

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?

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

This fixes
https://github.com/git-for-windows/git/issues/2920

Signed-off-by: Torsten Bögershausen <redacted>

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.

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.

On Mon, Mar 1, 2021 at 11:26 AM Junio C Hamano [off-list ref] wrote:

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.

For what it's worth, that was what I did originally; the change
to look up the ce "up front" was because someone objected to the
double search implied by calling cache_name_pos once, then
cache_file_exists to determine the correct error message...

(I've never been 100% on how the ignore-case stuff works
internally.)

Chris