git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Igor Djordjevic <igor.d.djordjevic@gmail.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	Jacob Keller <jacob.keller@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Johannes Sixt <j6t@kdbg.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Date: Mon, 19 Mar 2018 22:35:09 +0100
Message-ID: <34e8d563-a035-b09e-e959-748f2b4f4b99@gmail.com> (raw)
In-Reply-To: <87h8pc1uxr.fsf@javad.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

  reply index

Thread overview: 173+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-16 13:08 [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear) Sergey Organov
2018-02-18  4:16 ` Jacob Keller
2018-02-19  5:28   ` Sergey Organov
2018-02-19 23:44 ` Igor Djordjevic
2018-02-20 12:42   ` Sergey Organov
2018-02-27  0:07   ` Johannes Schindelin
2018-02-27  5:01     ` Sergey Organov
2018-02-27  5:30     ` Jacob Keller
2018-02-27 16:21       ` Johannes Schindelin
2018-02-27 18:55         ` Igor Djordjevic
2018-02-27 19:59           ` Igor Djordjevic
2018-02-27 23:27             ` Johannes Schindelin
2018-02-28  2:12               ` Igor Djordjevic
2018-02-28  4:35                 ` Igor Djordjevic
2018-02-28  6:14                   ` Sergey Organov
2018-02-28 20:53                     ` Igor Djordjevic
2018-02-28  5:44               ` Sergey Organov
2018-02-28 19:42                 ` Igor Djordjevic
2018-02-27 23:40             ` Igor Djordjevic
2018-02-28  0:10               ` Junio C Hamano
2018-02-28  2:35                 ` Igor Djordjevic
2018-02-28  5:27                 ` Sergey Organov
2018-02-28  0:36               ` Jacob Keller
2018-02-28  1:33                 ` Igor Djordjevic
2018-02-28  1:43                   ` Igor Djordjevic
2018-02-28  5:21                   ` Sergey Organov
2018-02-28 19:09                     ` Igor Djordjevic
2018-03-01  5:27                       ` Sergey Organov
2018-02-28  5:19               ` Sergey Organov
2018-02-28 20:25                 ` Igor Djordjevic
2018-02-28 22:17                   ` Igor Djordjevic
2018-03-01  5:19                     ` Sergey Organov
2018-03-01  5:39                   ` Sergey Organov
2018-03-02  1:16                     ` Igor Djordjevic
2018-03-02  5:40                       ` Sergey Organov
2018-03-02 17:45                         ` Igor Djordjevic
2018-03-02 11:17                       ` [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear) Phillip Wood
2018-03-02 12:36                         ` Phillip Wood
2018-03-02 16:02                           ` Jacob Keller
2018-03-02 23:33                             ` Igor Djordjevic
2018-03-06 10:36                               ` Phillip Wood
2018-03-06 18:12                                 ` Johannes Schindelin
2018-03-06 19:43                                   ` Igor Djordjevic
2018-03-07  7:26                                     ` Johannes Schindelin
2018-03-08 11:20                                       ` Phillip Wood
2018-03-08 12:16                                         ` Phillip Wood
2018-03-08 16:05                                           ` Igor Djordjevic
2018-03-11 12:00                                             ` Johannes Schindelin
2018-03-11 16:33                                               ` Igor Djordjevic
2018-03-12 10:37                                                 ` Johannes Schindelin
2018-03-12 12:56                                                   ` Sergey Organov
2018-03-13  0:01                                                     ` Igor Djordjevic
2018-03-26 12:03                                                       ` Johannes Schindelin
2018-03-27  5:08                                                         ` Sergey Organov
2018-03-27 13:35                                                           ` Johannes Schindelin
2018-04-02  6:07                                                             ` Sergey Organov
2018-03-12 23:54                                                   ` Igor Djordjevic
2018-03-13  6:25                                                     ` Sergey Organov
2018-03-08 15:56                                         ` Igor Djordjevic
2018-03-11 12:08                                           ` Johannes Schindelin
2018-03-11 17:34                                             ` Igor Djordjevic
2018-03-12 10:46                                               ` Johannes Schindelin
2018-03-13  0:16                                                 ` Igor Djordjevic
2018-03-26 13:07                                                   ` Johannes Schindelin
2018-03-27  5:51                                                     ` Sergey Organov
2018-03-27 13:49                                                       ` Johannes Schindelin
2018-03-28  5:57                                                         ` Sergey Organov
2018-03-30 13:41                                                           ` Johannes Schindelin
2018-03-30 16:36                                                             ` Sergey Organov
2018-03-28  5:57                                                         ` Sergey Organov
     [not found]                                                           ` <CA+P7+xoDQ2mzhxeZPFhaY+TaSoKkQm=5AtoduHH06-VggOJ2jg@mail.gmail.com>
2018-03-28 11:29                                                             ` Sergey Organov
     [not found]                                                               ` <CA+P7+xo19mHrWz9Fy-ifgCcVJM2xwzcLj7F2NvFe2LwGbaJiDQ@mail.gmail.com>
2018-03-29  5:53                                                                 ` Sergey Organov
2018-03-30 10:38                                                                   ` Johannes Schindelin
2018-03-30 12:36                                                                     ` Sergey Organov
2018-03-30 13:33                                                                       ` Johannes Schindelin
2018-03-30 15:13                                                                         ` Sergey Organov
2018-03-28 12:10                                                             ` Sergey Organov
2018-03-08 16:07                                         ` Jacob Keller
2018-03-11 12:11                                           ` Johannes Schindelin
2018-03-11 17:46                                             ` Igor Djordjevic
2018-03-08 15:16                                       ` Igor Djordjevic
2018-03-08 16:21                                         ` Igor Djordjevic
2018-03-11 12:22                                           ` Johannes Schindelin
2018-03-14 14:24                                         ` Sergey Organov
2018-03-14 23:11                                           ` Igor Djordjevic
2018-03-15  6:00                                             ` Sergey Organov
2018-03-15 21:51                                               ` Igor Djordjevic
2018-03-17  2:08                                             ` Igor Djordjevic
2018-03-19  5:44                                               ` Sergey Organov
2018-03-19 21:35                                                 ` Igor Djordjevic [this message]
2018-03-20 14:43                                                   ` Sergey Organov
2018-03-26 13:47                                           ` Johannes Schindelin
2018-03-06 23:24                                   ` Junio C Hamano
2018-03-07  7:09                                     ` Johannes Schindelin
2018-03-07 18:20                                       ` Junio C Hamano
2018-03-08  7:03                                         ` Johannes Schindelin
2018-03-08  8:11                                           ` Junio C Hamano
2018-03-09 17:09                                             ` Johannes Schindelin
2018-03-02 16:00                         ` Jacob Keller
2018-03-02 18:14                           ` Igor Djordjevic
2018-03-03 17:29                             ` Igor Djordjevic
2018-03-05  5:35                               ` Sergey Organov
2018-03-02 11:31                     ` [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear) Phillip Wood
2018-03-03  0:29                       ` Igor Djordjevic
2018-03-05  5:00                         ` Sergey Organov
2018-03-06 10:52                           ` Phillip Wood
2018-03-06 16:56                             ` Junio C Hamano
2018-03-08  7:05                               ` Johannes Schindelin
2018-03-08  8:18                                 ` Junio C Hamano
2018-03-11 11:56                                   ` Johannes Schindelin
2018-03-13 18:24                                     ` Junio C Hamano
2018-03-26 13:17                                       ` Johannes Schindelin
2018-03-05 17:29                         ` Johannes Schindelin
2018-03-06 23:21                           ` Igor Djordjevic
2018-03-07  7:04                             ` Johannes Schindelin
2018-03-06 10:45                         ` Phillip Wood
2018-03-06 11:45                           ` Sergey Organov
2018-03-08 16:30                             ` Igor Djordjevic
2018-03-06 18:12                           ` Johannes Schindelin
2018-03-07  5:08                             ` Sergey Organov
2018-03-07  6:58                               ` Johannes Schindelin
2018-03-07 14:34                                 ` Sergey Organov
2018-03-08  6:45                                   ` Johannes Schindelin
2018-03-12 12:31                                     ` Sergey Organov
2018-03-26 11:37                                       ` Johannes Schindelin
2018-03-27  5:11                                         ` Sergey Organov
2018-03-27 12:55                                           ` Johannes Schindelin
2018-03-28  4:32                                             ` Sergey Organov
2018-03-08 11:08                             ` Phillip Wood
2018-03-05 17:52                       ` Johannes Schindelin
2018-03-13 16:10                       ` Sergey Organov
2018-03-14  1:12                         ` Igor Djordjevic
2018-03-14  7:21                           ` Sergey Organov
2018-03-15  0:09                             ` Igor Djordjevic
2018-03-15  7:52                               ` Sergey Organov
2018-03-15 23:08                                 ` Igor Djordjevic
2018-03-16  7:31                                   ` Sergey Organov
2018-03-17  3:04                                     ` Igor Djordjevic
2018-03-19  6:01                                       ` Sergey Organov
2018-03-26 14:11                                   ` Johannes Schindelin
2018-02-28  0:29         ` Jacob Keller
2018-02-27 11:57     ` Sergey Organov
2018-02-27 18:14       ` Junio C Hamano
2018-02-28  0:30         ` Jacob Keller
2018-02-28  5:54           ` Sergey Organov
2018-02-28  4:53         ` Sergey Organov
2018-03-06 13:26 ` [RFC v2] " Sergey Organov
2018-03-07  6:46   ` Johannes Schindelin
2018-03-07 13:27     ` Sergey Organov
2018-03-07 14:08       ` Johannes Schindelin
2018-03-07 15:16         ` Sergey Organov
2018-03-08  7:01           ` Johannes Schindelin
2018-03-12 12:42             ` Sergey Organov
2018-03-26 11:50               ` Johannes Schindelin
2018-03-08 19:58         ` Igor Djordjevic
2018-03-08 20:27           ` Igor Djordjevic
2018-03-08 22:05             ` Igor Djordjevic
2018-03-08 23:31               ` Igor Djordjevic
2018-03-11 15:47                 ` Johannes Schindelin
2018-03-11 20:53                   ` Igor Djordjevic
2018-03-12 10:20                     ` Johannes Schindelin
2018-03-12 13:49                       ` Sergey Organov
2018-03-26 12:44                         ` Johannes Schindelin
2018-03-27  5:32                           ` Sergey Organov
2018-03-13  0:29                       ` Igor Djordjevic
2018-03-26 13:58                         ` Johannes Schindelin
2018-03-12 13:07                     ` Sergey Organov
2018-03-11 15:40           ` Johannes Schindelin
2018-03-11 22:04             ` Igor Djordjevic
2018-03-12 12:05               ` Sergey Organov
2018-03-26 11:33                 ` Johannes Schindelin
2018-03-27  5:34                   ` Sergey Organov
2018-03-12 22:41               ` Igor Djordjevic

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34e8d563-a035-b09e-e959-748f2b4f4b99@gmail.com \
    --to=igor.d.djordjevic@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jacob.keller@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sorganov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox