[v3,1/1] git mv foo FOO ; git mv foo bar gave an assert

Commit Message

Torsten Bögershausen March 1, 2021, 9:41 p.m. UTC

From: Torsten Bögershausen <[e-mail]>

The following sequence, on a case-insensitive file system,
(strictly speeking with core.ignorecase=true)
leads to an assertion failure and leaves .git/index.lock behind.

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" later in the code and keeps setting up the
"ce" pointer for ce_stage(ce) done in the next else if.

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

Reported-By: Dan Moseley <[e-mail]>
Signed-off-by: Torsten Bögershausen <[e-mail]>
---
Changes since V2:
  avoid the trailer mess-up by putting "this fixes" before the trailers
  Make clearer why cache_name_pos() (leaving a dangling ce) followed
  by an additional cache_file_exists() to set up ce can be optimized
  into a single cache_file_exists()

builtin/mv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.30.0.155.g66e871b664

Comments

Junio C Hamano March 1, 2021, 11:59 p.m. UTC | #1

[e-mail] writes:

> From: Torsten Bögershausen <[e-mail]>
>
> The following sequence, on a case-insensitive file system,
> (strictly speeking with core.ignorecase=true)
> leads to an assertion failure and leaves .git/index.lock behind.
>
> 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" later in the code and keeps setting up the
> "ce" pointer for ce_stage(ce) done in the next else if.
>
> This fixes
> https://github.com/git-for-windows/git/issues/2920
>
> Reported-By: Dan Moseley <[e-mail]>
> Signed-off-by: Torsten Bögershausen <[e-mail]>

Thanks, that looks better.

> ---
> Changes since V2:
>   avoid the trailer mess-up by putting "this fixes" before the trailers
>   Make clearer why cache_name_pos() (leaving a dangling ce) followed
>   by an additional cache_file_exists() to set up ce can be optimized
>   into a single cache_file_exists()
>
> builtin/mv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 7dac714af9..3fccdcb645 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -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))) {
>  			bad = _("not under version control");
>  		} else if (ce_stage(ce)) {
>  			bad = _("conflicted");
> --
> 2.30.0.155.g66e871b664

Patch

diff --git a/builtin/mv.c b/builtin/mv.c
index 7dac714af9..3fccdcb645 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -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))) {
 			bad = _("not under version control");
 		} else if (ce_stage(ce)) {
 			bad = _("conflicted");