Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 16, 2011, 21:07 UTC
- Message-ID
- <7vpqqrit8r.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1297850903-65038-3-git-send-email-jaysoffian@gmail.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/.
Show 9 quoted lines
> @@ -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.