From: Junio C Hamano <gitster@pobox.com>
To: Erik Cervin Edin <erik@cervined.in>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: Bug: git stash store can create stash entries that can't be dropped
Date: Wed, 11 Oct 2023 09:29:50 -0700 [thread overview]
Message-ID: <xmqqzg0pnmv5.fsf@gitster.g> (raw)
In-Reply-To: <CA+JQ7M_effxh9BSOhF67N+rsvBVTULe0dWZzp-kq1yOiDq3+hQ@mail.gmail.com> (Erik Cervin Edin's message of "Wed, 11 Oct 2023 10:42:31 +0200")
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.
> 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 &&
next prev parent reply other threads:[~2023-10-11 16:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-11 8:42 Bug: git stash store can create stash entries that can't be dropped Erik Cervin Edin
2023-10-11 16:29 ` Junio C Hamano [this message]
2023-10-11 19:44 ` Erik Cervin Edin
2023-10-11 20:44 ` [PATCH] stash: be careful what we store Junio C Hamano
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=xmqqzg0pnmv5.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=erik@cervined.in \
--cc=git@vger.kernel.org \
/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