diff mbox series

[v3] t7001: add failure test which triggers assertion

Message ID c4ada0b787736ecd5aee986b1b8a4f90ccb84e21.1729631436.git.code@khaugsbakk.name (mailing list archive)
State Accepted
Commit 0fcd473fdd3f8d2c418a4fafcbee2a523bd610cb
Headers show
Series [v3] t7001: add failure test which triggers assertion | expand

Commit Message

Kristoffer Haugsbakk Oct. 22, 2024, 9:14 p.m. UTC 
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(+)


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

Comments

Taylor Blau Oct. 23, 2024, 8:36 p.m. UTC | #1 
On Tue, Oct 22, 2024 at 11:14:33PM +0200, kristofferhaugsbakk@fastmail.com wrote:
> 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?

Thanks, this the new version looks good to me. Let's start merging this
one down.

Thanks,
Taylor
diff mbox series

Patch

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