Re: git-cherry-pick and git-commit --amend in version 1.7.6.4

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2011, 07:06 UTC
Message-ID: <7v39f638fr.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAG+J_Dysix9fOCuvm5+aU7-AC4wmsxH4-MOX+yhaHEqzeN1cPg@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 32 quoted lines

> Something like this?
>
> diff --git i/builtin/revert.c w/builtin/revert.c
> index 3117776c2c..f7fcc88871 100644
> --- i/builtin/revert.c
> +++ w/builtin/revert.c
> @@ -384,6 +384,7 @@ static int do_pick_commit(void)
>  	char *defmsg = NULL;
>  	struct strbuf msgbuf = STRBUF_INIT;
>  	int res;
> +	int record_cherry_pick_head = 0;
>
>  	if (no_commit) {
>  		/*
> @@ -477,7 +478,7 @@ static int do_pick_commit(void)
>  			strbuf_addstr(&msgbuf, ")\n");
>  		}
>  		if (!no_commit)
> -			write_cherry_pick_head();
> +			record_cherry_pick_head = 1;
>  	}
>
>  	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
> @@ -514,6 +515,9 @@ static int do_pick_commit(void)
>  	free_message(&msg);
>  	free(defmsg);
>
> +	if (record_cherry_pick_head)
> +		write_cherry_pick_head();
> +
>  	return res;
>  }

I switched to "maint" to look at this patch in context without the sequencer complication.

The basic idea to delay writing the file feels sound, but when a conflict happens, print_advice() runs and tries to clear CHERRY_PICK_HEAD, but you are then writing the file out much later than that at the end of the function.

This patch seems to break a few tests. t3404, t3506 and t3507 are among them.

Also, if you are using the recursive strategy, a cherry-pick that did not start would die() in do_recursive_merge(), and your hunk at -477,7 to remove call to write_cherry_head() would be sufficient, but if you are using another strategy, then try_merge_command() would return with 2 and I think you would want to skip it for the same reason in that case.