git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: kristofferhaugsbakk@fastmail.com
To: git@vger.kernel.org
Cc: Kristoffer Haugsbakk <code@khaugsbakk.name>,
	gitster@pobox.com, dsimic@manjaro.org, me@ttaylorr.com
Subject: [PATCH v3] t7001: add failure test which triggers assertion
Date: Tue, 22 Oct 2024 23:14:33 +0200	[thread overview]
Message-ID: <c4ada0b787736ecd5aee986b1b8a4f90ccb84e21.1729631436.git.code@khaugsbakk.name> (raw)
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


  parent reply	other threads:[~2024-10-22 21:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-05 17:41 [BUG] mv: can trigger assertion failure with three parameters (builtin/mv.c:481) Kristoffer Haugsbakk
2024-01-05 18:52 ` Junio C Hamano
2024-01-05 19:06   ` Dragan Simic
2024-02-18 12:42 ` Kristoffer Haugsbakk
2024-10-20 22:14   ` [PATCH v2] t7001: add failure test which triggers assertion kristofferhaugsbakk
2024-10-21 21:21     ` Taylor Blau
2024-10-21 21:25       ` Kristoffer Haugsbakk
2024-10-21 21:29         ` Taylor Blau
2024-10-22 21:14     ` kristofferhaugsbakk [this message]
2024-10-23 20:36       ` [PATCH v3] " Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c4ada0b787736ecd5aee986b1b8a4f90ccb84e21.1729631436.git.code@khaugsbakk.name \
    --to=kristofferhaugsbakk@fastmail.com \
    --cc=code@khaugsbakk.name \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).