Please work on providing a Minimal, Complete & Verifiable Example. It will dramatically increase your chances to receive helpful assistance.

What you provided is neither minimal, nor complete, as you have not made your repository publicly accessible.

Having said that...

c:\git_CAP\repo_MFW2>git mv mfw2/application/phaseCalc_IQ_Xiong.hpp PC_phaseTest/phaseCalc_IQ_Xiong.hpp
Assertion failed: pos >= 0, file builtin/mv.c, line 295

[...]
c:\git_CAP\repo_MFW2>git status
[...]
deleted: MFW2/application/phaseCalc_IQ_Xiong.hpp

Please note that the Git index is not case-insensitive. So while you may access the directory on disk via mfw2, you cannot access the same directory in the index via mfw2: it is clearly recorded as MFW2. I bet it would work if you called this command instead:

git mv MFW2/application/phaseCalc_IQ_Xiong.hpp PC_phaseTest/

I will assume that the suggested git mv invocation fixed the problem.

In case it's helpful, here's a trivial repro as I hit this crash also:

C:\git\demo>git --version --build-options
git version 2.29.2.windows.3
cpu: x86_64
built from commit: d054eb1fc46ff23e7c95756a7c747e2f2864b478
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh

C:\git\demo>git init
Initialized empty Git repository in C:/git/demo/.git/

C:\git\demo>echo foo > foo

C:\git\demo>git add foo

C:\git\demo>git commit -am "add foo"
[master (root-commit) 3dbfa54] add foo
 1 file changed, 1 insertion(+)
 create mode 100644 foo

C:\git\demo>git mv foo FOO

C:\git\demo>git mv foo BAR
Assertion failed: pos >= 0, file builtin/mv.c, line 295

At this point git crashes and leaves .git/index.lock behind.

This is on Windows 10 20H2.

here's a trivial repro

This is on Windows 10 20H2.

Quick clarification: Have you been able to test the repo on WSL/WSL2 or Linux (i.e. did you check if it was an upstream Git problem or special to Git for Windows)?

No need to test WSL or whatever, as far as I can see.
Same problem exist under MacOS.
Here is a possible fix, with mangled whitespace

git diff
diff --git a/builtin/mv.c b/builtin/mv.c
index 7dac714af9..48d0c930a2 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -292,8 +292,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
continue;

pos = cache_name_pos(src, strlen(src));
- assert(pos >= 0);
- rename_cache_entry_at(pos, dst);
+ if (pos >= 0)
+ rename_cache_entry_at(pos, dst);
+ else if (!ignore_errors)
+ die(_("bad source: source=%s, destination=%s"),
+ src, dst);
}

if (gitmodules_modified)

@tboegi do you plan to contribute that fix?

@danmosemsft :
I have no plans to send it to git.git, at least not for the next 4 weeks or so.
If someone wants to pick it up: Please go ahead.

Trying 2.30.0.rc2 on Ubuntu 20.04 I get a nice error instead

dan@LAPTOP-P6UJDVTA:/tmp/tmp$ git mv foo FOO
dan@LAPTOP-P6UJDVTA:/tmp/tmp$ git mv foo BAR
fatal: bad source, source=foo, destination=BAR

I assume it repros on Linux if it is possible to have a case insensitive file system.

I'm happy to submit a fix, but I have a question about how to test it. I was expecting the exit code when the assert happens to be 128+SIGABRT = 134, however it is 3:

danmose@LAPTOP-P6UJDVTA MINGW64 ~/tmp (master)
$ git mv foo BAR
Assertion failed: pos >= 0, file builtin/mv.c, line 295

danmose@LAPTOP-P6UJDVTA MINGW64 ~/tmp (master)
$ echo $?
3

In my test, I have test_must_fail git mv foo BAR and test_must_fail considers 3 an acceptable failure, whereas I want test_must_fail to reject it with the message test_must_fail: died by signal 6: .... as it would do for 134.

My knowledge of signals/shell is limited. What am I missing?

Ah, I see this is MSVCRT default behavior to exit(3) on a failed assertion. Does that mean when running the tests on Windows, test_must_fail will ignore failed assertions?

I will just do test_expect_code 128 git mv foo BAR

Ah, I see this is MSVCRT default behavior to exit(3) on a failed assertion. Does that mean when running the tests on Windows, test_must_fail will ignore failed assertions?

I will just do test_expect_code 128 git mv foo BAR

It means that we probably don't test assertions in the test suite. Typically, assert() is used for conditions that the author expects never to happen, so it is relatively easy to see why.

Besides, there is a trend to remove assert() calls in Git's source code, sometimes replacing them by BUG() calls.

so it is relatively easy to see why.

My observation was that an assert within test_must_fail will not fail the test so maybe the assert will be undetected. But, if they're being removed, no matter.

so it is relatively easy to see why.

My observation was that an assert within test_must_fail will not fail the test

I wonder how one would set up a test case that runs into a situation expected never to happen (and only defensively guarded via an assert() call).

so maybe the assert will be undetected. But, if they're being removed, no matter.

I wonder how one would set up a test case that runs into a situation expected never to happen (and only defensively guarded via an assert() call

Presumably any test could in theory hit an assert, just as it could hit a segfault. It might be configuration specific, non deterministic, etc.

I wonder how one would set up a test case that runs into a situation expected never to happen (and only defensively guarded via an assert() call

Presumably any test could in theory hit an assert, just as it could hit a segfault. It might be configuration specific, non deterministic, etc.

test_must_fail is reserved for known bugs in Git, though.

Seems like it's being used to test cases where git commands are supposed to fail for bad input? eg

test_expect_success \
    'do not move directory over existing directory' \
    'mkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0'

test_expect_success '"add non-existent" should fail' '
	test_must_fail git add non-existent &&

README says the purpose of test_must_fail is to verify a git command fails without missing segv's as ! would do.

Whoops, I confused test_expect_failure with test_must_fail (again! Grmbl).

Having said that, test_must_fail intends to verify that Git errors out when it should.

An assert() is very different from an expected error message: typically, those statements are skipped when compiling in release mode. Those calls are purely intended to help developing code, to verify invariants. I'm still puzzled, therefore, why anybody would want to test for assert() in the test suite. It strikes me as a case of "not holding the tools correctly".

I'm still puzzled, therefore, why anybody would want to test for assert() in the test suite.

Hi @dscho, isn't that a catch 22 problem.

An assert without a test only has a vain hope of triggering, so the 'developer' is left in a bit of no-man's land where production (release) code will wander off into some seg fault because the check is skipped, while the maintaining dev does want to know how to try and trigger the fault.

Not sure there's an easy solution. Maybe those tests could have a 'DEVELOPER' pre-req? Just a thought.

I'm still puzzled, therefore, why anybody would want to test for assert() in the test suite.

Hi @dscho, isn't that a catch 22 problem.

No.

I fear you're mistaking assert() for something it is not.

All you're supposed to do with that construct is verify invariants, things you (the developer) take for granted, e.g. that a given array is sorted (assert(i + 1 == nr || strcmp(array[i], array[i+1]) <= 0)). If such a basic contract is violated, you want the debugger to stop right there. That's what assertions are all about.

If you need to add a regression test to verify that your invariant testing is sound, you need to think about your life choices :-)

An assert without a test only has a vain hope of triggering, so the 'developer' is left in a bit of no-man's land where production (release) code will wander off into some seg fault because the check is skipped, while the maintaining dev does want to know how to try and trigger the fault.

If you're relying on assert() to prevent a segmentation fault, you're definitely not using that construct correctly.

Again, this is a function intended to verify invariants while developing the code in question (or while debugging it). That's all assert() ever was. Really.

that construct is verify invariants, things you (the developer) take for granted

So, by that argument, this assert never triggered .... which is why it's a catch 22 at this level.

Why are we even talking about this anymore? The fact that an assert() should not trigger is by design.

This very assert should not trigger at all - a patch has been send upstream.