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
next prev 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).