Thread (1 message) 1 message, 1 author, 2016-06-16

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

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:46

Michael J Gruber [off-list ref] writes:
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.
quoted
quoted
@@ -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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help