Re: [PATCH v2 11/11] rebase: dereference tags

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

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Aborting a rebase stated with 'git rebase <upstream> <tag-object>'
> should checkout the commit pointed to by <tag-object>. Instead it gives

I am not sure if "should checkout the commit pointed to by." is a
good description.  It does not seem to be sufficiently justified.

Did we auto-peel in scripted version of "git rebase" and is this a
regression when the command was rewritten in C?

If that is not the case, this topic is perhaps slightly below
borderline "meh" to me.  The optional "first switch to this <branch>
before doing anything" command-line argument in

    git rebase [--onto <there>] <upstream> [<branch>]

was meant to give a branch, and because we treat detached HEAD as
almost first-class citizen when dealing with branch-ish things, we
allowed

	git rebase master my-topic^0

to try rebasing my-topic on detached HEAD without losing the
original.  In other words, you had to be explicit that you meant the
commit object, not a ref that points at it, to trigger this "rebase
detached" feature.  The same thing for tags.

	git rebase master v12.3^0

would be a proper request to rebase the history leading to that
commit.  Without the peeling, it appears the user is asking to
update the ref that can be uniquely identified with "v12.3", but we
do not want to rebase a tag.

It would have been a different story if we had a problem when a tag
is given to "--onto <there>", but I do not think this topic is about
that case.

Having said that, even if we decide that we shouldn't accept the tag
object and require peeled form to avoid mistakes (instead of
silently peeling the tag ourselves), I do agree that

>     error: update_ref failed for ref 'HEAD': cannot update ref 'HEAD': trying to write non-commit object       710d743b2b9892457fdcc3970f397e6ec07447e0 to branch 'HEAD'
>

is a bad error message for this.  It should be something like

	error: cannot rebase a tag

perhaps.

But if we auto-peeled in an old version, I do not mind this series
(but let's drop pointless "clean-up" that is not, like what was
pointed out by Réne).  In such a case, the first paragraph should
say, instead of "should checkout", that "we used to do X, but commit
Y broke us and now we die with an error message".

Thanks.