Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

[Igor Djordjevic <igor.d.djordjevic@gmail.com>]

Hi Sergey,

On 19/03/2018 06:44, Sergey Organov wrote:
> 
> > > > > > Second side note: if we can fast-forward, currently we prefer
> > > > > > that, and I think we should keep that behavior with -R, too.
> > > > >
> > > > > I agree.
> > > >
> > > > I'm admittedly somewhat lost in the discussion, but are you
> > > > talking fast-forward on _rebasing_ existing merge? Where would it
> > > > go in any of the suggested algorithms of rebasing and why?
> > > >
> > > > I readily see how it can break merges. E.g., any "git merge
> > > > --ff-only --no-ff" merge will magically disappear. So, even if
> > > > somehow supported, fast-forward should not be performed by default
> > > > during _rebasing_ of a merge.
> > >
> > > Hmm, now that you brought this up, I can only agree, of course.
> > >
> > > What I had in my mind was more similar to "no-rebase-cousins", like 
> > > if we can get away without actually rebasing the merge but still 
> > > using the original one, do it. But I guess that`s not what Johannes 
> > > originally asked about.
> > >
> > > This is another definitive difference between rebasing (`pick`?) and 
> > > recreating (`merge`) a merge commit - in the case where we`re rebasing, 
> > > of course it doesn`t make sense to drop commit this time (due to 
> > > fast-forward). This does make sense in recreating the merge (only).
> >
> > Eh, I might take this back. I think my original interpretation (and 
> > agreement) to fast-forwarding is correct.
> >
> > But the confusion here comes from `--no-ff` as used for merging, as 
> > opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant 
> > the latter one.
> >
> > In rebasing, `--no-ff` means that even if a commit inside todo list 
> > isn`t to be changed, do not reuse it but create a new one. Here`s 
> > excerpt from the docs[1]:
> >
> >   --no-ff
> >     With --interactive, cherry-pick all rebased commits instead of 
> >     fast-forwarding over the unchanged ones. This ensures that the 
> >     entire history of the rebased branch is composed of new commits.
> >
> >     Without --interactive, this is a synonym for --force-rebase.
> >
> >
> > So fast-forwarding in case of rebasing (merge commits as well) is 
> > something you would want by default, as it wouldn`t drop/lose 
> > anything, but merely reuse existing commit (if unchanged), instead of 
> > cherry-picking (rebasing) it into a new (merge) commit anyway.
> 
> This sounds like breakage. E.g., it seems to be breaking every "-x ours"
> merge out there.

Either you are not understanding how rebase fast-forward works, or 
I`m missing what you are pointing to... Mind explaining how can 
something that`s left unchanged suddenly become a breakage?

> Fast-forwarding existing merge, one way or another, still seems to be
> wrong idea to me, as merge commit is not only about content change, but
> also about joint point at particular place in the DAG.

Not sure what this has to do with rebase fast-forwarding, either - 
nothing changes for fast-forwarded (merge or non-merge) commit in 
question, both content, joint point and everything else stays exactly 
the same. If anything changed, then it can`t/won`t be fast-forwarded, 
being unchanged is a prerequisite.

Let me elaborate a bit. Here`s a starting diagram:

(1) ---X1---X2---X3 (master)
                 |\
                 | A1---A2---A3
                 |            \
                 |             M---C1---C2 (topic)
                 |            /
                 \-B1---B2---B3


With "topic" being active branch, we start interactive rebase with 
`git rebase -i master`. Generated todo list will hold commits A1 to 
A3, B1 to B3, M and C1 to C2.

Now, if we decide to `edit` commit C1, leaving everything else the 
same, fast-forward logic will make the new situation look like this:

(2) ---X1---X2---X3 (master)
                 |\
                 | A1---A2---A3
                 |            \
                 |             M---C1'--C2' (topic)
                 |            /
                 \-B1---B2---B3


Notice how only C1 and C2 changed to C1' and C2'? That`s rebase 
fast-forwarding, noticing earlier commits left unchanged, thus 
reusing original ones.

No matter what, no breakage can happen to M in this case, as it`s 
left (reused) exactly as it was - it`s fast-forward rebased.

If we `edit`-ed commit A2, we would have ended in a situation like this:

(3) ---X1---X2---X3 (master)
                 |\
                 | A1---A2'--A3'
                 |            \
                 |             M'--C1'--C2' (topic)
                 |            /
                 \-B1---B2---B3


This time we have new commits A2', A3', M', C1' and C2' - so 
everything influenced by the change that happened will be changed 
(merge commit as well), where all the rest can still be reused 
(fast-forwarded).

If we had started rebasing with `git rebase -i --no-ff master`, no 
matter which commits we `edit` (or none, even), we would end up with 
this instead:

(4) ---X1---X2---X3 (master)
                 |\
                 | A1'--A2'--A3'
                 |            \
                 |             M'--C1'--C2' (topic)
                 |            /
                 \-B1'--B2'--B3'


So even in case where nothing is changed, no rebase fast-forwarding 
is performed (as requested), and each and every commit is changed (old 
commit replaced with its rebased version, commit hash changed).

Now, if our starting position looked like this instead:

(5) ---X1---X2---X3---X4---X5 (master)
                 |\
                 | A1---A2---A3
                 |            \
                 |             M---C1---C2 (topic)
                 |            /
                 \-B1---B2---B3

... and we simply do `git rebase -i master` again, causing all side 
commits to be rebased onto X5, then no fast forwarding can be done, as 
all commits _are_ changed (by having their parent changed, no matter if 
we additionally edit them or not), so even without `--no-ff` option we 
end up with this:

(6) ---X1---X2---X3---X4---X5 (master)
                           |\
                           | A1'--A2'--A3'
                           |            \
                           |             M'--C1'--C2' (topic)
                           |            /
                           \-B1'--B2'--B3'


Does this settle your concerns, or I`m missing something?

> As for fast-forwarding re-merge, explicitly requested, I'm not sure. On
> one hand, it's inline with the default "git merge" behavior, on the
> other hand, it still feels wrong, somehow.

Regarding fast-forwarding in context of merging, in case where we are 
recreating merges (not rebasing them), following existing `git merge` 
logic might make sense, where I would expect rebasing todo list `merge` 
command to pick-up tricks from `git merge` as needed, like learning 
to accept `--no-ff` option, for example, thus not fast-forwarding 
merges (on request) even when possible.

Though, I do agree that in case you want to recreate an existing merge 
(instead of just rebasing it), `merge` command fast-forwarding might 
probably not be what you want for the most of the time, but I`m afraid 
having rebase todo list `merge` command default behavior different than 
`git merge` default one (in regards to fast-forwarding) would be 
confusing... or not?

From what I could grasp so far, usually Git commands` default 
behavior is (explained to be) chosen per "most common use case", so 
might be non fast-forwarding would be fine as default for rebase todo 
list `merge` command, even though different than `git merge` itself...?

Regards, Buga