[PATCH] rebase: 'pull --rebase' rebase on new branching only

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue May 10 10:14:48 EDT 2016



On 05/07/2016 11:59 PM, Ryan McElroy wrote:
> On 5/5/2016 11:57 PM, Pierre-Yves David wrote:
>>
>>
>> On 05/05/2016 10:45 PM, Martin von Zweigbergk wrote:
>>> On Thu, May 5, 2016 at 9:04 AM, Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org> wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>>>> # Date 1462366525 -7200
>>>> #      Wed May 04 14:55:25 2016 +0200
>>>> # Node ID 7e72800c070d941f294c4e7ed91db29a08bac073
>>>> # Parent  906a1c8a75fd8a18e43e8545eedcbe5222f84647
>>>> # EXP-Topic pull.rebase
>>>> rebase: 'pull --rebase' rebase on new branching only
>
> "new branching" sounds awkward to me as a native english speaker. 
> After reviewing what this patch does, I think it would be more clear 
> to say "rebase on newly introduced branch only".

Noted, thanks

>
>>> Meant for stable?
>>
>> No, this is a new behavior making 'hg pull --rebase' a bit closer to 
>> its initial intend (and therefore hopefully more reliable to build 
>> work-flow on) but the 3.7 behavior was even more baroque so no 
>> regression testing here.
>
> Are you saying that `hg pull --rebase` was intended to have 
> non-idempotent semantics? Or am I missing something? Can you spell out 
> what you think the original intention was, and why getting closer to 
> it is a good thing?

So my personal interpretation is that the point of `hg pull --rebase` is 
to "rebase on what you pull" or another phrasing "re-linearize newly 
introduced branch".

Note that the fix for issue5214 (no bookmark involved version) already 
broke the idem-potency, this change is pushing the behavior change further.

Getting closer to it is useful because -in-the-current-state- of 
Mercurial, it is common to have multiple anonymous heads on branch, 
leading to ambiguity on destination and various "surprising" behavior.

Archeology does not raise much details on this.

Let's move to the next block for wider discussion

>
>>
>>>> Previously, if two heads existed on the current branch before the 
>>>> pull and the
>>>> pull add changeset on the one unrelated to the working copy 'hg 
>>>> pull --rebase'
>>>> would trigger a rebase on that branch. This might not what the user 
>>>> wants
>>>> as they could have resolved the head situation before pulling if 
>>>> they wanted to.
>
> The new behavior might also not be what the user wants, so I don't 
> think this is a good enough reason to make the change.

My goal here is to make the -current- state more predictable/useful for 
users. A medium terms solution to this is to vastly improve the 
experience around destination selection. My main lead to make progress 
here is the topic experiment and tracking related feature around this. 
(note that you are the one who convinced me how destination/tracking was 
such an important part of the user experience we need to fix).

That said, I'm proposing this change because I think it is a small 
improvement to the current situation, I'm open to delaying it and having 
the destination improvement first (this year?). Such improvement would 
makes the risk of surprising behavior much lower and this change less 
important.

> The changes I would like to see in pull --rebase are to make it so 
> that `hg pull && hg pull --rebase` (eg, "oh crap I forgot the flag") 
> has the same result as `hg pull --rebase` from the beginning. This 
> patch doesn't move us in that direction (indeed, I would argue that it 
> moves us further away), so I'm skeptical of the direction.

While I'm open to change to my approach/interpretation, I'm not super 
convinced by this argument:

A: if we keep `hg pull --rebase` as an (almost) strict equivalent for 
`hg pull && hg rebase`, there is not much extra effort to run `hg 
rebase` if one forgot to add `--rebase`. So the value of running a full 
pull again is unclear to me.

     (I says almost, because, hg pull --rebase can also perform an update)

B: We make the change proposed in this patch,
B.1: if we get (or by chance, the user already are) to a clear 
destination/tracking situation, `hg rebase` after a pull should behave 
properly,
B.2: if we had another pre-existing head the user do not want to rebase 
to, `hg pull --rebase` now behave better (do nothing) and `hg rebase` 
still "misbehave (rebase on the other, pre-existing, head),
B.3: if we had another pre-existing head and we pull a third one 
(branching from the working copy parent), `hg pull rebase` will pick the 
newly pulled one as destination (solving the ambguity using the fact one 
is newly pulled) and `hg rebase` will properly complain about the ambiguity.

Overall we improve behavior in case B2 and B3, without regressions from 
the previous situation.

Let's make a table to summarize

Situation 1: (wc is on A)
1) 1 head (A) before pull, pull bring another head (N)
2) 2 head (A+B) before pull, pull bring new changeset on another branch; 
A is tip of branch
3) 2 head (A+B) before pull, pull bring new changeset on another branch; 
B is tip of branch
4) 2 head (A+B) before pull, pull bring changeset on B, but no new heads 
(or not one branching from ".")
5) 2 head before pull (A+B), pull bring a new head (N) branching from A
6) 2 head before pull (A+B), pull bring a new head (N) branching from A 
and change on B
(we could have a longer list but I'm trying to keep things "simple"

Behavior:
A: 3.7 behavior "full rebase, tip of branch is destination)
B: 3.8 behavior "pick destination in pulled changeset only"
C: This patch behavior "pick destionation on branch from "::." newly pulled"

For `hg pull --rebase`:
     1    | 2       | 3       | 4       | 5    | 6
A | on N | nothing | on B    | on B    | on N | max(B,N)
B | on N | on B    | on B    | on B    | on N | abort
C | on N | nothing | nothing | nothing | on N | N

for `hg pull && hg rebase`
     1    | 2       | 3       | 4       | 5     | 6
A | on N | nothing | on B    | on B    | on N  | max(B,N)
B | on N | on B    | on B    | on B    | abort | abort
C | on N | on B    | on B    | on B    | abort | abort
(note, 2+3+4 would pick B even without the pull)


> That being said, after having written all this, I see that you have 
> had a nice long discussion about it already over at 
> https://bz.mercurial-scm.org/show_bug.cgi?id=5214, so I'll bow out 
> now, haha.

It is more a nice long monologue, but I'm not sure this issue is the 
right place for a discussion, there is already too many topic 
interleaved there.

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list