[PATCH v3] t7001: add failure test which triggers assertion

From: kristofferhaugsbakk@fastmail.com <kristofferhaugsbakk@fastmail.com>
Date: Oct 22, 2024, 21:14 UTC
Message-ID: <c4ada0b787736ecd5aee986b1b8a4f90ccb84e21.1729631436.git.code@khaugsbakk.name>
In-Reply-To: <29d71db280c972c91174bd0a501af66be72643af.1729462326.git.code@khaugsbakk.name>

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

`git mv a/a.txt a b/` is a nonsense instruction. Instead of failing gracefully the command trips over itself,[1] leaving behind unfinished work:

1. first it moves `a/a.txt` to `b/a.txt`; then
2. tries to move `a/`, including `a/a.txt`; then
3. figures out that it’s in a bad state (assertion); and finally
4. aborts.

Now you’re left with a partially-updated index.

The command should instead fail gracefully and make no changes to the index until it knows that it can complete a sensible action.

For now just add a failing test since this has been known about for a while.[2]

† 1: Caused by a `pos >= 0` assertion [2]: https://lore.kernel.org/git/d1f739fe-b28e-451f-9e01-3d2e24a0fe0d@app.fastmail.com/

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    v3:
    • Rewrite commit message based on Junio’s reply
    • Tweak test description: less volatile.  Also mention index state.
    v1/v2:
    • It’s been a good while.  Let’s just add this as a known breakage?

Notes (meta-trailers):
    Helped-by: Junio C Hamano <gitster@pobox.com>
        Comment: Commit message is based on his description
        Link: https://lore.kernel.org/git/xmqqil47obnw.fsf@gitster.g/

 t/t7001-mv.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 86258f9f43..69c9190772 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -551,4 +551,16 @@ test_expect_success 'moving nested submodules' '
 	git status
 '
 
+test_expect_failure 'nonsense mv triggers assertion failure and partially updated index' '
+	test_when_finished git reset --hard HEAD &&
+	git reset --hard HEAD &&
+	mkdir -p a &&
+	mkdir -p b &&
+	>a/a.txt &&
+	git add a/a.txt &&
+	test_must_fail git mv a/a.txt a b &&
+	git status --porcelain >actual &&
+	grep "^A[ ]*a/a.txt$" actual
+'
+
 test_done

Interdiff against v2:
  diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
  index 739c25e255..69c9190772 100755
  --- a/t/t7001-mv.sh
  +++ b/t/t7001-mv.sh
  @@ -551,7 +551,7 @@ test_expect_success 'moving nested submodules' '
   	git status
   '
   
  -test_expect_failure 'nonsense mv triggers assertion failure at builtin/mv.c:502' '
  +test_expect_failure 'nonsense mv triggers assertion failure and partially updated index' '
   	test_when_finished git reset --hard HEAD &&
   	git reset --hard HEAD &&
   	mkdir -p a &&

base-commit: 34b6ce9b30747131b6e781ff718a45328aa887d0

-- 
2.46.1.641.g54e7913fcb6