why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Andy Zhang]

why "git rebase" searching the duplicate patches in <upstream branch>
rather than in <new base branch>?

hi, all:

 I am reading the help of "git rebase", it says:
    "If the upstream branch already contains a change you have made
(e.g., because you mailed a patch which was applied upstream), then
that commit will be skipped. "

 But, because we are applying commits to <new base branch> rather than
to <upstream branch>, I really don't understand why we are searching
the duplicate patches in <upstream branch> rather than in <new base
branch>?

 In the following example, the git command is as:
   git rebase --onto master next topic

 I think it should be reasonable to search the duplicate patches in
<new base branch>(i.e, master) instead of <next branch>.

 If possible, an improvement may be an option for enabling searching
in <new base branch> as well?

  Thank you in advance for any enlightenment you can provide!


Jintao Zhu

//----
example:

Old tree is:

o---o---o---o---o  master
    \
     o---o---o---o---o  next
                      \
                       o---o---o  topic


We want our tree to look like this:
o---o---o---o---o  master
   |            \
   |             o'--o'--o'  topic
    \
     o---o---o---o---o  next

We can get this using the following command:

  git rebase --onto master next topic

//----

RE: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Felipe Contreras]

Andy Zhang wrote:
> why "git rebase" searching the duplicate patches in <upstream branch>
> rather than in <new base branch>?
> 
> hi, all:
> 
>  I am reading the help of "git rebase", it says:
>     "If the upstream branch already contains a change you have made
> (e.g., because you mailed a patch which was applied upstream), then
> that commit will be skipped. "
> 
>  But, because we are applying commits to <new base branch> rather than
> to <upstream branch>, I really don't understand why we are searching
> the duplicate patches in <upstream branch> rather than in <new base
> branch>?
> 
>  In the following example, the git command is as:
>    git rebase --onto master next topic
> 
>  I think it should be reasonable to search the duplicate patches in
> <new base branch>(i.e, master) instead of <next branch>.

My understanding is that only applies when --onto is not specified:

  git rebase master topic

In this case "master" is both the new base, and upstream, and it's in
this case where duplicates are skipped.

If you specify --onto master, then this isn't done.

-- 
Felipe Contreras

Re: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Junio C Hamano]

Andy Zhang <zhgdrx@gmail.com> writes:

> why "git rebase" searching the duplicate patches in <upstream
> branch> rather than in <new base branch>?
>
> hi, all:
>
>  I am reading the help of "git rebase", it says:
>     "If the upstream branch already contains a change you have made
> (e.g., because you mailed a patch which was applied upstream), then
> that commit will be skipped. "
>
>  But, because we are applying commits to <new base branch> rather than
> to <upstream branch>, I really don't understand why we are searching
> the duplicate patches in <upstream branch> rather than in <new base
> branch>?

It is either a design bug or a documentation bug, or both ;-)

I do think it makes sense to skip commits from the branch we are
rebasing that have equivalent commits in the upstream, as it is
expected that upstream might have already applied/cherry-picked some
of the changes you are rebasing, and you do not want to use the same
change twice.

When we are transplanting a series of commits from an old base to
totally unrelated base using the --onto option, e.g. when replaying
the contents of 'topic' relative to 'next' down to 'master' in your
topology, however,

> Old tree is:
>
> o---o---o---o---o  master
>     \
>      o---o---o---o---o  next
>                       \
>                        o---o---o  topic

it is not necessarily obvious where to stop digging back at.  In the
above picture where 'master' and 'next' have ancestry relationship,
we could try to see if the three commits on 'topic' branch being
replayed match any of the commits in next..master range, but when
using the --onto option, there does not have to be any relationship
between the <upstream> and <new base> (they do not have to share a
root commit).  So from that point of view, it probably makes sense
to default to --no-reapply-cherry-picks when --onto is used, while
defaulting --reapply-cherry-picks when --onto is not used.

Re: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Andy Zhang <zhgdrx@gmail.com> writes:
>
>> why "git rebase" searching the duplicate patches in <upstream
>> branch> rather than in <new base branch>?
>>
>> hi, all:
>>
>>  I am reading the help of "git rebase", it says:
>>     "If the upstream branch already contains a change you have made
>> (e.g., because you mailed a patch which was applied upstream), then
>> that commit will be skipped. "
>>
>>  But, because we are applying commits to <new base branch> rather than
>> to <upstream branch>, I really don't understand why we are searching
>> the duplicate patches in <upstream branch> rather than in <new base
>> branch>?
>
> It is either a design bug or a documentation bug, or both ;-)

It's definitely /at least/ a documentation bug, as description of the
feature is not precise enough. For example, it's unclear if such a
commit will appear in the todo list of --interactive. Will it?

It looks like documentation of "git rebase" should be revised to make
clearer distinction between <branch>, <upstream>, and <newbase>.

>
> I do think it makes sense to skip commits from the branch we are
> rebasing that have equivalent commits in the upstream, as it is
> expected that upstream might have already applied/cherry-picked some
> of the changes you are rebasing, and you do not want to use the same
> change twice.

To me this only makes sense for the branch we rebase /onto/, and thus it
actually makes sense for <newbase>, and for <upstream> it only happens
to make sense by default as <newbase>=<upstream> in this case.

If Git currently indeed searches for duplicates in <upstream>, then it
looks like implementation bug, or misfeature. I think the <newbase>
should rather be used.

>
> When we are transplanting a series of commits from an old base to
> totally unrelated base using the --onto option, e.g. when replaying
> the contents of 'topic' relative to 'next' down to 'master' in your
> topology, however,
>
>> Old tree is:
>>
>> o---o---o---o---o  master
>>     \
>>      o---o---o---o---o  next
>>                       \
>>                        o---o---o  topic
>
> it is not necessarily obvious where to stop digging back at.

Similar problem should exist for explicitly specified <upstream> that
might happen to have little in common with the current <branch>, right?
If so, then it's already somehow being solved, even if simply by
ignoring the issue, so adding <newbase> to the picture doesn't actually
bring anything significantly new.

> In the
> above picture where 'master' and 'next' have ancestry relationship,
> we could try to see if the three commits on 'topic' branch being
> replayed match any of the commits in next..master range, but when
> using the --onto option, there does not have to be any relationship
> between the <upstream> and <new base> (they do not have to share a
> root commit).  So from that point of view, it probably makes sense
> to default to --no-reapply-cherry-picks when --onto is used, while
> defaulting --reapply-cherry-picks when --onto is not used.

I don't actually like this.

First, in general, changing default of another option is not to be taken
lightly. For example, defaulting to --fork-point when no <upstream> is
specified is already a point of confusion.

Second, changing the default is not backward compatible, so there should
be very sound reason to change it.

Finally, if user does specify /both/ --onto and
--no-reapply-cherry-picks, where would Git supposedly stop digging for
matching cherry-picks? Provided this is to be solved anyway, the
rationale to change the default does not sound strong enough.

Overall, it seems that we should take the <newbase> rather than
<upstream> (that is still <upstream> when --onto is not specified), and
apply the skipping logic from there, to whatever depth the merge-base
will give us. If it's already implemented this way, then only the manual
page needs to be fixed.

Thanks,
-- 
Sergey Organov

Re: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> Similar problem should exist for explicitly specified <upstream> that
> might happen to have little in common with the current <branch>, right?

I do not think so.  Plain-vanilla rebase is to carry forward our
changes on top of updated upstream, which means that there is

            x--x--x (side)
           /
   ---o---o---o---o---o---o (upstream)
          ^
       (old upstream)

inherently ancestry relationship between the old upstream and the
current upstream when rebasing 'side' to 'upstream'.

> I don't actually like this.

You do not have to ;-) because I was not suggesting to change any
existing behaviour.  It was merely me thinking aloud, how I might
do the feature if I were designing it from scratch now.

> Overall, it seems that we should take the <newbase> rather than
> <upstream> (that is still <upstream> when --onto is not specified), and
> apply the skipping logic from there, to whatever depth the merge-base
> will give us. If it's already implemented this way, then only the manual
> page needs to be fixed.

Sounds sensible.  I didn't check what the actual code does ;-)

Re: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>

[...]

>
>> I don't actually like this.
>
> You do not have to ;-) because I was not suggesting to change any
> existing behaviour.  It was merely me thinking aloud, how I might
> do the feature if I were designing it from scratch now.

Yeah, I've noticed the absence of [PATCH] ;-) and was just thinking
aloud myself as well.

Thanks,
-- 
Sergey Organov

Re: why "git rebase" searching the duplicate patches in <upstream branch> rather than in <new base branch>?

[Andy Zhang]

On Tue, Jul 20, 2021 at 6:23 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Andy Zhang <zhgdrx@gmail.com> writes:
>
> > why "git rebase" searching the duplicate patches in <upstream
> > branch> rather than in <new base branch>?
> >
> > hi, all:
> >
> >  I am reading the help of "git rebase", it says:
> >     "If the upstream branch already contains a change you have made
> > (e.g., because you mailed a patch which was applied upstream), then
> > that commit will be skipped. "
> >
> >  But, because we are applying commits to <new base branch> rather than
> > to <upstream branch>, I really don't understand why we are searching
> > the duplicate patches in <upstream branch> rather than in <new base
> > branch>?
>
> It is either a design bug or a documentation bug, or both ;-)
>
It should NOT be a documentation bug, because, in my experiment, I
observe that, "git"'s behavior is exactly the same as documentation.
So, it should be "work as design".

> I do think it makes sense to skip commits from the branch we are
> rebasing that have equivalent commits in the upstream, as it is
> expected that upstream might have already applied/cherry-picked some
> of the changes you are rebasing, and you do not want to use the same
> change twice.
>
> When we are transplanting a series of commits from an old base to
> totally unrelated base using the --onto option, e.g. when replaying
> the contents of 'topic' relative to 'next' down to 'master' in your
> topology, however,
>
> > Old tree is:
> >
> > o---o---o---o---o  master
> >     \
> >      o---o---o---o---o  next
> >                       \
> >                        o---o---o  topic
>
> it is not necessarily obvious where to stop digging back at.  In the
> above picture where 'master' and 'next' have ancestry relationship,
> we could try to see if the three commits on 'topic' branch being
> replayed match any of the commits in next..master range, but when
> using the --onto option, there does not have to be any relationship
> between the <upstream> and <new base> (they do not have to share a
> root commit).  So from that point of view, it probably makes sense
> to default to --no-reapply-cherry-picks when --onto is used, while
> defaulting --reapply-cherry-picks when --onto is not used.
>
>