From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: "Tilman Vogel" <tilman.vogel@web.de>, "Jan Krüger" <jk@jk.gs>
Subject: [PATCH] push: do not turn --delete '' into a matching push
Date: Tue, 23 Feb 2021 15:13:32 -0800 [thread overview]
Message-ID: <xmqq4ki21ioz.fsf@gitster.g> (raw)
When we added a syntax sugar "git push remote --delete <ref>" to
"git push" as a synonym to the canonical "git push remote :<ref>"
syntax at f517f1f2 (builtin-push: add --delete as syntactic sugar
for :foo, 2009-12-30), we weren't careful enough to make sure that
<ref> is not empty.
Blindly rewriting "--delete <ref>" to ":<ref>" means that an empty
string <ref> results in refspec ":", which is the syntax to ask for
"matching" push that does not delete anything.
Worse yet, if there were matching refs that can be fast-forwarded,
they would have been published prematurely, even if the user feels
that they are not ready yet to be pushed out, which would be a real
disaster.
Noticed-by: Tilman Vogel <tilman.vogel@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* So this time with an obvious test. It is somewhat surprising
that this has been left unnoticed for the past 10 years.
builtin/push.c | 2 +-
t/t5516-fetch-push.sh | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/push.c b/builtin/push.c
index 03adb58602..194967ed79 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -115,7 +115,7 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
else
refspec_appendf(&rs, "refs/tags/%s", ref);
} else if (deleterefs) {
- if (strchr(ref, ':'))
+ if (strchr(ref, ':') || !*ref)
die(_("--delete only accepts plain target ref names"));
refspec_appendf(&rs, ":%s", ref);
} else if (!strchr(ref, ':')) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 3ed121d0ce..7eee4e782f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -824,6 +824,11 @@ test_expect_success 'push --delete refuses src:dest refspecs' '
test_must_fail git push testrepo --delete master:foo
'
+test_expect_success 'push --delete refuses empty string' '
+ mk_test testrepo heads/master &&
+ test_must_fail git push testrepo --delete ""
+'
+
test_expect_success 'warn on push to HEAD of non-bare repository' '
mk_test testrepo heads/master &&
(
--
2.30.1-824-gddfeb442a8
next reply other threads:[~2021-02-23 23:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-23 23:13 Junio C Hamano [this message]
2021-02-24 4:58 ` [PATCH] push: do not turn --delete '' into a matching push Jeff King
2021-02-24 7:07 ` Elijah Newren
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=xmqq4ki21ioz.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jk@jk.gs \
--cc=tilman.vogel@web.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.