Re: [RFC PATCH] add t3420-rebase-topology

[Martin von Zweigbergk <martinvonz@gmail.com>]

On Tue, Sep 18, 2012 at 12:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> do you agree
>> that 'rebase --onto does not re-apply patches in onto' is desirable?
>
> This depends on how you look at --onto.  Recall the most typical and
> the original use case of rebase:
>
>                                A'--C' your rebased work
>                               /
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> You could view "git rebase master" as a short-hand for
>
>         $ git rebase --onto $(git merge-base master HEAD) master

Exactly. I frequently consider it a short-hand for that. It might
be worth pointing out that 'git pull --rebase', which might be
one of the most frequent uses of rebase, internally often does

  git rebase --onto upstream upstream@{...} branch

where upstream@{...} is the most recent upstream that is an
ancestor of "branch". For example, if your work is based on
origin/pu and you send the bottom-most patch ("B" in the figure
below) to the maintainer and and it gets applied to
pu. Running "git pull --rebase" would then lead to
"git rebase --onto T A". You would want this to drop B.

                               C' your rebased work
                              /
  ---o---o---o---F---B'--o---T origin/pu
                  \
                   A origin/pu@{1}
                    \
                     B---C your work


> The intended use case for "--onto", however, is primarily to replay
> a history to older base, i.e. [...] a moral equivalent of
>
>         $ git checkout v1.7.12
>         $ git cherry-pick A B C ;# or git cherry-pick master..HEAD

Yes, this is the alternative way of looking it at and exactly why
I, too, was not sure how it should behave.

> You could argue that you can compute the patch equivalence between
> the commits in "onto..master" and commits in "master..HEAD" and
> filter out the equivalent commits

I'm not sure if you meant "master..onto" rather
than "onto..master". Rebase (well, all flavors of rebase
but "-m") currently drops patches from "master..HEAD" that are
also in "HEAD..master". This is what the "rebase --onto does not
lose patches in upstream" test is about. It is also one of the
main problems that I try to fix in my long-stalled rebase-range
series. I think we should drop patches in "master..HEAD" that are
also in "HEAD..onto" (which is almost the same
as "master..onto").

> The "replay to an updated base" case (i.e. without "--onto")

Or _with_ --onto as in the above example from "git pull --rebase".

> On the other hand, when the user replays to an older base, she has
> some idea what constitutes "a series" that she is replaying (i.e.
> "$(git merge-base master HEAD)..HEAD").  It smells to go against the
> user's world model if the command silently filtered commits by patch
> equivalence.

If it's truly about rebasing onto an older base, there can't
possibly be any patches in "HEAD..onto", so assuming you agree
with my reasoning above that those are the patches we should
drop, rebasing onto older history would be safe.

> Besides, the whole point of a separate "onto" is to allow the user
> to specify a commit that does not have a straightforward ancestry
> relationship with the bottom of the series (i.e. either "master" or
> "F"), and computation of patch equivalence is expected to be much
> higher.  Given that it is unlikely to find any match, it feels
> doubly wrong to always run "git cherry" equivalent in that case.

Yes, this was my only concern (apart from it possibly being
conceptually wrong to do, depending on what the user meant by
issuing the command).

>> How about 'rebase --root is not a no-op'?
>
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> If "git rebase F" when you are at C in the above illustration
> (reproduced only the relevant parts) is a no-op (and I think it
> should be), "git rebase --root" in the illustration below ought to
> be as well, no?
>
>                  F---B'--o---T master
>                   \
>                    A---B---C your work

Yeah, that's what I thought as well at first. I think my test
case even started out as "rebase --root _is_ a no-op".

When thinking about how to handle roots in general, I often
imagine a single virtual root commit (parent of all "initial"
commits), and that reasoning also implies that "git rebase
--root" should be a no-op. Then I saw that the test case
failed (or perhaps I remembered how it is implemented with the
clever fake root/initial commit) and started thinking about why
anyone would use "git rebase --root" if it was a no-op. I could
only think of using it to linearize history, but that doesn't
seem like a very likely use case. So it seems like weighing
purity/correctness against usefulness to me. I'm not sure which
way to go.

Thanks for quick and detailed feedback on an RFC patch.

Martin