Re: Bug: git stash store can create stash entries that can't be dropped

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2023, 16:29 UTC
Message-ID: <xmqqzg0pnmv5.fsf@gitster.g>
In-Reply-To: <CA+JQ7M_effxh9BSOhF67N+rsvBVTULe0dWZzp-kq1yOiDq3+hQ@mail.gmail.com>

Erik Cervin Edin <erik@cervined.in> writes:

> ... even if it wasn't created using git stash create

I am of two minds. As "stash store" and "stash create" were invented as, and have always been ever since, pretty much implementation details of scripted "stash save", the user deserves what they get when they abuse them: garbage-in, garbage-out.

Show 5 quoted lines

> A stash entry is created that cannot be dropped, because it's not
> stash-like commit.
>
>   git stash drop
>   fatal: 'refs/stash@{0}' is not a stash-like commit

Yes, this is exactly what the user deserves.

Having said that, I agree that this shows an uneven UI. The "drop" command cares about what it is dropping and refuses if it is not a stash-like thing, so it is understandable to wish "store" to also care to the same degree.

It may be just the matter of doing something silly like this. Not even compile tested, but hopefully it is sufficient to convey the idea.

 builtin/stash.c  | 6 ++++++
 t/t3903-stash.sh | 4 ++++
 2 files changed, 10 insertions(+)

diff --git c/builtin/stash.c w/builtin/stash.c
index 1ad496985a..4a6771c9f4 100644
--- c/builtin/stash.c
+++ w/builtin/stash.c
@@ -989,6 +989,12 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 static int do_store_stash(const struct object_id *w_commit, const char *stash_msg,
 			  int quiet)
 {
+	struct stash_info info;
+	char revision[GIT_MAX_HEXSZ];
+
+	oid_to_hex_r(revision, w_commit);
+	assert_stash_like(&info, revision);
+
 	if (!stash_msg)
 		stash_msg = "Created via \"git stash store\".";
 
diff --git c/t/t3903-stash.sh w/t/t3903-stash.sh
index 0b3dfeaea2..30b64260a8 100755
--- c/t/t3903-stash.sh
+++ w/t/t3903-stash.sh
@@ -931,6 +931,10 @@ test_expect_success 'store called with invalid commit' '
 	test_must_fail git stash store foo
 '
 
+test_expect_success 'store called with non-stash commit' '
+	test_must_fail git stash store HEAD
+'
+
 test_expect_success 'store updates stash ref and reflog' '
 	git stash clear &&
 	git reset --hard &&