Re: [PATCH] cherry-pick: allow to pick to unborn branches

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2016, 07:03 UTC
Message-ID: <xmqqoa7dscov.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <88c61a4d-abee-2c01-a521-8094b4baf3ac@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 5 quoted lines

> Here and below, I'm mimicking/copying the behavior that we have right
> now already. I asked myself the same question - rolling back to orphan
> state shouldn't be that hard after all. But that would be a change in
> behavior that - if considered a fix/improvement - would be orthogonal to
> the multi-pick fix.

Hmph, OK.

Show 23 quoted lines

>>> @@ -1086,11 +1090,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
>>>  	walk_revs_populate_todo(&todo_list, opts);
>>>  	if (create_seq_dir() < 0)
>>>  		return -1;
>>> -	if (get_sha1("HEAD", sha1)) {
>>> -		if (opts->action == REPLAY_REVERT)
>>> -			return error(_("Can't revert as initial commit"));
>>> -		return error(_("Can't cherry-pick into empty head"));
>>> -	}
>>> +	if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
>>> +		return error(_("Can't revert as initial commit"));
>> 
>> Not a new issue, but I cannot quite fathom what this error message
>> wants to say.  "Can't revert an initial commit"?
>
> Cannot create a "revert commit" as the initial commit on a yet unborn
> branch. Maybe "nothing to revert" would be clearer, but then another
> user might ask: If I say "git revert deabeef" and there is a commit
> "deadbeef" then why is there "nothing to revert"?
>
> Applying the reverse of a patch to an empty tree should result in an
> empty tree, and creating a commit with that empty tree as "This reverts
> commit deadbeef" is what we are refusing here, unless I'm confused.

OK, so the message wants to say "a revert cannot be recorded as initial commit" (because the original change has to be one that results in an empty tree to be able to sanely be reversed its effect on top of an empty tree)? That interpretation is understandable.

But that still sounds like a fishy logic.

A root commit should be cherry-pickable as it is a change to whatever state from an empty tree, and we do allow it. The same reasoning suggests that if you find a change that makes the tree empty, such a change should be revert-able on top of an empty tree.

In fact, we do allow cherry-picking a non-root commit onto an empty tree, as long as the changes are all about creating new files. So perhaps a change that only removes files should be revert-able on top of an empty tree?

In any case, this topic is not about fixing that semantics of the revert and it is a tangent. I'll just leave the usual "low hanging fruit" mark so that people can list-archive-search for that string to find it as a possible mini-project to allow reverting such a change on top of an empty tree.

Thanks.