From: Jeff King <peff@peff.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Andrei Rybak <rybak.a.v@gmail.com>,
git@vger.kernel.org,
Christian Couder <christian.couder@gmail.com>,
Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH v2 2/2] sequencer: don't say BUG on bogus input
Date: Tue, 10 Jul 2018 00:32:08 -0400 [thread overview]
Message-ID: <20180710043207.GB1909@sigill.intra.peff.net> (raw)
In-Reply-To: <20180710043120.GA1330@sigill.intra.peff.net>
When cherry-picking a single commit, we go through a special
code path that avoids creating a sequencer todo list at all.
This path expects our revision parsing to turn up exactly
one commit, and dies with a BUG if it doesn't.
But it's actually quite easy to fool. For example:
$ git cherry-pick --author=no.such.person HEAD
error: BUG: expected exactly one commit from walk
fatal: cherry-pick failed
This isn't a bug; it's just bogus input.
The condition to trigger this message actually has two
parts:
1. We saw no commits. That's the case in the example
above. Let's drop the "BUG" here to make it clear that
the input is the problem. And let's also use the phrase
"empty commit set passed", which matches what we say
when we do a real revision walk and it turns up empty.
2. We saw more than one commit. That one _should_ be
impossible to trigger, since we fed at most one tip and
provided the no_walk option (and we'll have already
expanded options like "--branches" that can turn into
multiple tips). If this ever triggers, it's an
indication that the conditional added by 7acaaac275
(revert: allow single-pick in the middle of cherry-pick
sequence, 2011-12-10) needs to more carefully define
the single-pick case.
So this can remain a bug, but we'll upgrade it to use
the BUG() macro, which would make it easier to detect
and analyze if it does trigger.
Signed-off-by: Jeff King <peff@peff.net>
---
sequencer.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index f692b2ef44..8f0a015160 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3636,8 +3636,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
if (prepare_revision_walk(opts->revs))
return error(_("revision walk setup failed"));
cmit = get_revision(opts->revs);
- if (!cmit || get_revision(opts->revs))
- return error("BUG: expected exactly one commit from walk");
+ if (!cmit)
+ return error(_("empty commit set passed"));
+ if (get_revision(opts->revs))
+ BUG("unexpected extra commit from walk");
return single_pick(cmit, opts);
}
--
2.18.0.400.g702e398724
next prev parent reply other threads:[~2018-07-10 4:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-09 14:16 [BUG] git cherry-pick does not complain about unknown options Andrei Rybak
2018-07-09 19:16 ` Jeff King
2018-07-09 19:46 ` [PATCH 0/2] de-confuse git cherry-pick --author behavior Jeff King
2018-07-09 19:48 ` [PATCH 1/2] sequencer: handle empty-set cases consistently Jeff King
2018-07-09 20:20 ` Junio C Hamano
2018-07-09 20:21 ` Johannes Schindelin
2018-07-09 19:49 ` [PATCH 2/2] sequencer: don't say BUG on bogus input Jeff King
2018-07-09 20:24 ` Johannes Schindelin
2018-07-09 21:11 ` Junio C Hamano
2018-07-10 2:15 ` Jeff King
2018-07-10 4:31 ` [PATCH v2 0/2] de-confuse git cherry-pick --author Jeff King
2018-07-10 4:31 ` [PATCH v2 1/2] sequencer: handle empty-set cases consistently Jeff King
2018-07-10 4:32 ` Jeff King [this message]
2018-07-11 8:58 ` [PATCH v2 0/2] de-confuse git cherry-pick --author Johannes Schindelin
2018-07-11 15:37 ` 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180710043207.GB1909@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=rybak.a.v@gmail.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).