From: Junio C Hamano <gitster@pobox.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Erik Cervin Edin <erik@cervined.in>
Subject: [PATCH] stash: be careful what we store
Date: Wed, 11 Oct 2023 13:44:03 -0700 [thread overview]
Message-ID: <xmqqbkd4lwj0.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqqzg0pnmv5.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 11 Oct 2023 09:29:50 -0700")
"git stash store" is meant to store what "git stash create"
produces, as these two are implementation details of the end-user
facing "git stash save" command. Even though it is clearly
documented as such, users would try silly things like "git stash
store HEAD" to render their stash unusable.
Worse yet, because "git stash drop" does not allow such a stash
entry to be removed, "git stash clear" would be the only way to
recover from such a mishap. Reuse the logic that allows "drop" to
refrain from working on such a stash entry to teach "store" to avoid
storing an object that is not a stash entry in the first place.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
> 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.
Now it is at least compile-tested.
builtin/stash.c | 6 ++++++
t/t3903-stash.sh | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/builtin/stash.c b/builtin/stash.c
index 3a4f9fd566..8073ef4019 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -977,6 +977,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 a/t/t3903-stash.sh b/t/t3903-stash.sh
index 376cc8f4ab..35c8569aea 100755
--- a/t/t3903-stash.sh
+++ b/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 &&
--
2.42.0-345-gaab89be2eb
prev parent reply other threads:[~2023-10-11 20:44 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
2023-10-11 19:44 ` Erik Cervin Edin
2023-10-11 20:44 ` Junio C Hamano [this message]
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=xmqqbkd4lwj0.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