Re: [PATCH 5/3] revert: introduce --abort to cancel a failed cherry-pick
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 30, 2011, 22:52 UTC
- Message-ID
- <7vty5lp6jc.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20111122112001.GF7399@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 5 quoted lines
> After running some ill-advised command like "git cherry-pick > HEAD..linux-next", the bewildered novice may want to return to more > familiar territory. Introduce a "git cherry-pick --abort" command > that rolls back the entire cherry-pick sequence and places the > repository back on solid ground.
This is confusing; if you have many commits in the range, and a handful of them replayed without conflicts and then you hit a conflict, where should (I am not asking "where does ... with your patch") abort take us? The state after the random commit that happened to have replayed successfully? The state before the entire cherry-pick sequence started? "back on solid ground" does not tell us which one you meant.
I am assuming that it is the latter.
Show 11 quoted lines
> diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt > index 75f8e869..5747f442 100644 > --- a/Documentation/sequencer.txt > +++ b/Documentation/sequencer.txt > @@ -7,3 +7,6 @@ > Forget about the current operation in progress. Can be used > to clear the sequencer state after a failed cherry-pick or > revert. > + > +--abort:: > + Cancel the operation and return to the pre-sequence state.
Ok, it is the latter.
Show 9 quoted lines
> +static int reset_for_rollback(const unsigned char *sha1)
> +{
> + const char *argv[4]; /* reset --merge <arg> + NULL */
> + argv[0] = "reset";
> + argv[1] = "--merge";
> + argv[2] = sha1_to_hex(sha1);
> + argv[3] = NULL;
> + return run_command_v_opt(argv, RUN_GIT_CMD);
> +}So you give the value of the HEAD before the sequence started to this function and all should go well. Where do you read that value from?
Show 11 quoted lines
> +static int rollback_single_pick(void)
> +{
> + unsigned char head_sha1[20];
> +
> + if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
> + !file_exists(git_path("REVERT_HEAD")))
> + return error(_("no cherry-pick or revert in progress"));
> + if (!resolve_ref("HEAD", head_sha1, 0, NULL))
> + return error(_("cannot resolve HEAD"));
> + if (is_null_sha1(head_sha1))
> + return error(_("cannot abort from a branch yet to be born"));Ok, this is for single-pick so HEAD is where we came from. Good.
Show 28 quoted lines
> + return reset_for_rollback(head_sha1);
> +}
> +
> +static int sequencer_rollback(struct replay_opts *opts)
> +{
> + const char *filename;
> + FILE *f;
> + unsigned char sha1[20];
> + struct strbuf buf = STRBUF_INIT;
> +
> + filename = git_path(SEQ_HEAD_FILE);
> + f = fopen(filename, "r");
> + if (!f && errno == ENOENT) {
> + /*
> + * There is no multiple-cherry-pick in progress.
> + * If CHERRY_PICK_HEAD or REVERT_HEAD indicates
> + * a single-cherry-pick in progress, abort that.
> + */
> + return rollback_single_pick();
> + }
> + if (!f)
> + return error(_("cannot open %s: %s"), filename,
> + strerror(errno));
> + if (strbuf_getline(&buf, f, '\n')) {
> + error(_("cannot read %s: %s"), filename, ferror(f) ?
> + strerror(errno) : _("unexpected end of file"));
> + goto fail;
> + }And when we are in multi-pick, SEQ_HEAD_FILE has it.
Looks good from a cursory review. Thanks.