Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD

[Junio C Hamano <gitster@pobox.com>]

Jay Soffian <jaysoffian@gmail.com> writes:

> If the user wishes to reset authorship, that must now be done explicitly
> via --reset-author.

This is not a new requirement, is it?  Even "commit -c $that_commit"
before the previous "commit -c CHERRY_PICK_HEAD" does use the original,
no?


I think the changed code is _MUCH_ easier to follow compared to the
previous round; the only thing the reader needs to keep in mind is that
the most of the change essentially is "s/in_merge/whence != FROM_COMMIT/"
and making that work.

> * We remove a unused import from revert.c

Micronit: s/import/include/.


> @@ -766,7 +802,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  
>  	fclose(fp);
>  
> -	if (!commitable && !in_merge && !allow_empty &&
> +	if (!commitable && whence != FROM_MERGE && !allow_empty &&
>  	    !(amend && is_a_merge(head_sha1))) {
>  		run_status(stdout, index_file, prefix, 0, s);
>  		if (amend)

The original says that we reject an attempt to record a no-change commit
when the user is making his own commit without an explicit --allow-empty
request.  Should cherry-pick change this?

"git cherry-pick $a_no_change_commit" does not cause conflicts but it does
fail with "nothing added to commit".  If you said "whence == FROM_COMMIT"
here, you would end up allowing "git commit" following such a failure to
commit without --allow-empty, and that inconsistency is to be avoided
(note that I didn't check if you leave CHERRY_PICK_HEAD behind to trigger
this codepath when this happens).

On the other hand, when the earlier cherry-pick did fail with conflict,
but the resolution ended up to be a no-change commit, you would not
blindly want to record the result as a no-change commit, either.

So I think the above is the right thing to do, but we probably need a bit
of in-code comment to describe why we say "whence != FROM_MERGE" here.

Thanks.