Re: [RFC/PATCH 2/2] Teach commit to handle CHERRY_HEAD automatically

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 15, 2011, 23:47 UTC
Message-ID: <20110215234735.GA18151@elie>
In-Reply-To: <AANLkTinZ0ewJy01rV66xMMCKLon=7qz=hoJ3DbtXmtXL@mail.gmail.com>

Hi,

Thanks for a quick response.  Some small clarifications.

Jay Soffian wrote:

> On Tue, Feb 15, 2011 at 6:00 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 11 quoted lines

>>  - "git commit --amend" to say "I'm done fixing the broken thing".
>>
>>  - "git commit --fixup=HEAD/--squash=HEAD" to say "done fixing;
>>   let's look at this again later and squash it when I am less
>>   confused".
>>
>> Both are mistakes if HEAD is the previous and good commit rather than
>> the broken one.  Maybe there is some simple safety that could protect
>> against them?
>
> As you see below, this is already protected against.

The first mistake is protected against[*], the second left alone. (The second seems mostly harmless; just mentioning it for reference.)

[*] which I think is my favorite part of this patch

Show 8 quoted lines

>>>  --reset-author::
>>> +     When used with -C/-c/--amend options, or when committing after a
>>> +     a conflicting cherry-pick,
>>
>> or when committing after a conflicted merge, no?
>
> No. The person committing a merge is already the author of the merge,
> why would they use --reset-author?

You're right.  I find myself occasionally doing the following

	git merge --no-commit $branch
	... update version number; walk away for a while  ...
	git commit;	# will this use the old timestamp?
	git commit --amend --reset-author

out of a vague fear, but I think I was just confused.

Show 7 quoted lines

>>>                                  declare that the authorship of the
>>> +     resulting commit now belongs of the committer. This also renews
>>> +     the author timestamp.
>>
>> How does it interact with --author?
>
> No change from before, --author forces the author of the new commit.

Patch below.

[...]

> 		/* Let message-specifying options override CHERRY_HEAD */
>
> I'll make this logic clearer though as I need to address Junio's
> earlier message.

Ah, good to hear.

Show 12 quoted lines

>>> +             if (cherry_pick)
>>> +                     fprintf(fp,
>>> +                             "#\n"
>>> +                             "# It looks like you may be committing a cherry-pick.\n"
>>
>> Aside: shouldn't we suggest some porcelain-ish command (git merge
>> --clear?  git commit --no-merge?) to remove .git/MERGE_HEAD instead of
>> asking the committer to do it?
>
> We have git merge --reset as an alias for reset --merge. Since reset
> --merge takes care of this case too (I think, I'll check) we can
> suggest that for both.

No, I think "git merge --abort" does too much. If we were ready to commit, we almost surely have precious staged changes that it would remove.

The cure to a lingering MERGE_HEAD is still "rm -f .git/MERGE_HEAD", I fear. "git commit --no-merge" (meaning "ignore MERGE_HEAD") seems tempting.

>> How can I get out of this state if I really do want to amend?
>
> git reset, same as it ever was?

Not obvious at all. Maybe the manpage to cherry-pick could mention that CHERRY_HEAD is cleared away by git reset?

Show 5 quoted lines

>> Hmm, what if I have commits F and F' and after trying to cherry-pick F
>> I decide I want the message from F'?
>
> I don't think I understand. commit -c F', or just commit (w/o options)
> and you get MERGE_MSG generated by cherry-pick.

I meant the following sequence of operations:

	# by the way, does this set CHERRY_PICK_HEAD?
	git cherry-pick --no-commit F
	git commit -C F-prime

But that was a silly example --- -C takes care of authorship on its own. A better example might be

	git cherry-pick --no-commit F
	git commit -F the-message

or

	git cherry-pick --no-commit F
	git commit --amend -C F-prime

> Thanks for the feedback!

Thanks for making it happen. :) Jonathan